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=772608 --- Comment #12 from Gal Hammer <ghammer@xxxxxxxxxx> 2012-02-05 04:06:40 EST --- (In reply to comment #11) > First, I can see a glaring issue with your package, you are shipping GDM's > .src.rpm as part of your own source. That is probably not going to fly, even > though I can't really find anything in the packaging guidelines that prevents > you from doing it (closest would be > http://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built_binaries_or_libraries > but there's no mention about .src.rpm). I see some problems with your approach: > > - You are shipping a specific version of source that is already present in > fedora > - You will have to ship newer, static .src.rpm whenever GDM gets a newer > version > - You may end up fighting duplicated libraries > > The approach, in my opinion, would be to get in touch with the GDM packagers > and ask for your stuff to be included if it's not already. It is not entirely > apparent why requiring gdm-devel wouldn't work right now. The reason I include the gdm src.rpm file is because I didn't find another way to compile a gdm plugin outside the gdm's source tree. It is used only during compilation and linkage time and it is not shipped with the plugin itself. > [BLOCKER] The License field in the package spec file must match the actual > license - the COPYING file in the source tarball has GPLv3, you have GPLv2+ in > the spec file Fixed. > [BLOCKER] The sources used to build the package must match the upstream source, > as provided in the spec URL - You should take a look at > http://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control and > not use a self-hosted source tarball (Source0: > http://ghammer.fedorapeople.org/%{name}-%{version}.tar.bz2) As far as I understood this is allowed (http://fedoraproject.org/wiki/Packaging/SourceURL). > [BLOCKER] The package MUST successfully compile and build into binary rpms on > at least one primary architecture - I got this error when trying a mock build: > xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc16.x86_64. Probably a bug after fixes done after a review. I've updated a new version (http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.1-1.fc16.src.rpm) which compile on both 32 and 64 bits. > [BLOCKER] Each package must consistently use macros - I see a mix of $MACRO and > %{macro}, use one style and stick with. Also, if you don't plan on supporting > EPEL/RH, you should do away with the whole RPM_BUILD_ROOT [citation needed, > maybe?] I'm not sure I understand this. I checked with the gdm spec file (as an example) and it look pretty much like mine (using %macro expect for $RPM_BUILD_ROOT). > Additionally, it seems you include some python code in there, so, using > http://fedoraproject.org/wiki/Packaging:Python as guide: > > [BLOCKER] To build a package containing python2 files, you need to have > BuildRequires: python2-devel Why do I need to include it? My build machine doesn't have the python2-devel package installed. > Some additional suggestions: > - defattr is not needed anymore, if I recall "The %files section normally begins with a %defattr line which sets the default file permissions." (http://fedoraproject.org/wiki/How_to_create_an_RPM_package). Maybe it is time to review/update the manual... :-) > - you should name your different targets appropriately like > "ovirt-guest-agent-gdm-plugin" instead of just "gdm-plugin" The rpmbuild does that for me. The rpm file name is called: ovirt-guest-agent-gdm-plugin-1.0.1-1.fc16.x86_64. > - in your %pre step, you add a user rhevagent if it doesn't exist, but you are > also hardcoding the UID. You should leave that to the system to figure out and > just use -r for system users This is an assigned uid. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review