Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226638
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@redhat.com
--- Comment #2 from Tom "spot" Callaway tcallawa@redhat.com 2009-01-16 11:01:41 EDT --- Alright, lets pick this old merge review up, because I think we can beat it into shape.
The biggest item that I see here is that there is an embedded update "script". That would make a lot more sense to have it live as a Source file, especially since it is not using any rpm macros. It would also simplify the rpm spec file quite a bit.
There is the question as to whether this script (and the %pre copy) are still necessary in Fedora. If you think so, please keep them, if not, please remove them both from the package.
Please add an empty %build section.
Also, %dir %{_bindir}/xorg-x11-filesystem-upgrade is just wrong. That's a script, not a directory.
The last issue is that there seems to be fair bit of directory ownership duplication in the xorg stack.
/usr/lib/X11: filesystem, xorg-x11-filesystem
/usr/include/X11/: xorg-x11-filesystem, libfontenc-devel, libxkbfile-devel, libXdmcp-devel, libXfixes-devel, libICE-devel, libSM-devel, libXau-devel, libXt-devel, libXpm-devel, libXmu-devel, libXft-devel, libXv-devel, libXcursor-devel, libXvMC-devel, libXaw-devel, libXevie-devel, libXres-devel, libXfont-devel, libXcomposite-devel, libXrender-devel, libXdamage-devel, xorg-x11-xtrans-devel, libX11-devel, libXrandr-devel, xorg-x11-proto-devel
/usr/share/X11: xorg-x11-filesystem, xorg-x11-server-utils, xorg-x11-font-utils, xorg-x11-utils, imake, libX11, xkeyboard-config
If we don't need the upgrade script anymore, do we need this package anymore? Could we let filesystem own /usr/lib/X11 and /usr/share/X11, xorg-x11-proto-devel own /usr/include/X11 (and all those other dupes should Require: xorg-x11-proto-devel)