Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-1.ta.1.fc4.src.rpm Description: GNOME Interface for YUM
your spec file is autogenerated. look at http://fedoraproject.org/wiki/PackagingGuidelines and edit your file by hand.
(In reply to comment #1) > your spec file is autogenerated. > look at http://fedoraproject.org/wiki/PackagingGuidelines and edit your file by > hand. I modified gnome-yum.spec and create new src.rpm... Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-2.src.rpm
This is not a full review but a couple of comments: - The consolehelper symlinks don't belong to %post/%postun scriptlets, package them along with other content in %install - You should add dependency on usermode(-gtk) for the above - GConf2, vte, gnome-vfs2 dependencies are probably automatically detected by rpmbuild, if so they should be removed as redundant - Requires: rpm is silly - you must have rpm installed to install an rpm anyway - Adding 3rd party repositories containing known legally problematic packages is not allowed, the fedora-repositories subpackage and any references to those repositories needs to be removed. - The for-loop for copying the docs is redundant, just use %doc ABOUT-NLS AUTHORS README... in %files section - I'd suggest using %find_lang macro instead of the current {_datadir}/locale/*/LC_MESSAGES/gnome-yum.mo in %files
> - Requires: rpm is silly - you must have rpm installed to install an rpm anyway not necessarily; chroot installations do not need rpm
- Okay. I remove all repositories from source package and I insert %find_lang macro to the spec file. - I can't solve the consolehelper symlinks with your solving and I don't modify this problem in the end. (error message: RPM build errors: Symlink points to BuildRoot: /usr/bin/gnome-yum => /var/tmp/gnome-yum-0.1.2-3-root-root/usr/bin/consolehelper) Here is the next release: Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-3.src.rpm
The URL for your spec is pointing to an old version, could you update it please?
- Missing BR: gtkhtml2-devel (In reply to comment #3) > - Requires: rpm is silly - you must have rpm installed to install an rpm anyway Strictly speaking you need to have rpm-libs and a front-end. What's interesting is that I don't see any path from gnome-yum to rpm-libs. Am I missing something obvious?
BuildArch: i386 an reason for that? what about x86_64 ?
(In reply to comment #6) > The URL for your spec is pointing to an old version, could you update it please? Use 'Refresh' button... I used it too. (In reply to comment #8) > BuildArch: i386 > an reason for that? what about x86_64 ? Huh! Truly. I remove it. Here is the 4th release: Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-4.src.rpm
(In reply to comment #9) > (In reply to comment #6) > > The URL for your spec is pointing to an old version, could you update it please? > > Use 'Refresh' button... I used it too. I doubt that was the problem, since it was the first time I visited. But I see the right version now, so thanks.
(In reply to comment #5) > - I can't solve the consolehelper symlinks with your solving and I don't modify > this problem in the end. > (error message: > RPM build errors: > Symlink points to BuildRoot: /usr/bin/gnome-yum => > /var/tmp/gnome-yum-0.1.2-3-root-root/usr/bin/consolehelper) There are two ways of doing this currently implemented in Extras yumex in Extras has the symlinks created inside the Makefile in the source. netgo in Extras uses a construction in the specfile in the %install section: take a look at both the src.rpm for both yumex and netgo and choose a method that best suits you. Don't forget to add Requires: usermode to make sure consolehelper is installed on the system as a dependancy. -jef
(In reply to comment #11) I resolved this problem in the next release: Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-5.src.rpm
Missing build requires: gtkhtml2-devel (config fails without it) (trying srpm from comment #12) rpmlint isn't so happy either: E: gnome-yum no-changelogname-tag E: gnome-yum zero-length /usr/share/doc/gnome-yum-0.1.2/README W: gnome-yum no-dependency-on usermode-consoleonly E: gnome-yum non-executable-script /usr/share/gnome-yum/gyum-query.sh 0644 E: gnome-yum zero-length /usr/share/doc/gnome-yum-0.1.2/NEWS W: gnome-yum non-conffile-in-etc /etc/pam.d/gnome-yum E: gnome-yum zero-length /usr/share/doc/gnome-yum-0.1.2/TODO W: gnome-yum non-conffile-in-etc /etc/security/console.apps/gnome-yum Changelog is just completely missing .. shouldn't be :-) Empty file warnings are easy to fix, just leave them out :-) The non-executable, maybe a root,755,root %attr for it? The warning about the non conf file can be ignored, since it is a config file I see the spec file uses: [ -n "$RPM_BUILD_ROOT" -a "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT This was a standard (a good while ago) but the current guidelines say to just use: rm -rf $RPM_BUILD_ROOT If no one else has stepped up for review yet, i'll be willing too, changing blocker to FE-REVIEW. Fix the issues mentioned in the comments (including this one :-)) and once thats done i'll do the full review
It would also be ok to do: %{_datadir}/pixmaps/gnome-yum in the files list, no need to type out all the file names really, and otherwise you wouldn't own the directory for the config file mentioned above, try putting %config(noreplace) before them (pam.d/gnome-yum and console.apps/gnome-yum), which is proper thing to do with config files also it might be usefull to put gettext in the build requires, while i've never seen a live fedora system without, mock does need to know about it else it might trip over it
Desktop file should be parsed with desktop-file-install, adding the Fedora category, and add desktop-file-utils to the buildrequires Lastly there's no need for the: [ ! -f Makefile ] || make distclean line is there? If gnome-yum does its releases properly it should never be shipped with a Makefile
I repair (Comment #13-#15) bugs (exception rpmlint warning: 'W: gnome-yum no-dependency-on usermode-consoleonly') and create a new package: Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-6.src.rpm
Few last comments before i can do a complete formal review checklist: - Its not Buildrequires but: BuildRequires: (notice the capital R) - %changelog doesn't have to include the upstream (source) changes, but the changes you made to the package, ie: * Sat Jan 14 2006 András Tóth <toth_bandi.net> - 0.1.2-6 - Fixed desktop file vendor - Owned complete datadir - Changed clean section to be fedora compliant - find_lang can use %{name} macro, doesn't need to hard-code 'gnome-yum' And so on .. Your expected to add such a changelog entry for every 'release' of the package you make (ie up the version or release field) describing what you changed in the specfile. Its not required to list what changed in 'gnome-yum' its self,only what you changed of the package / specfile - attr for gyum-query.sh doesn't have to go thru a defattr, you could just do: %attr(0755,root,root) %{_datadir}/gnome-yum/gyum-query.sh - desktop file install misses: --add-category X-Fedora - Missing BuildRequires entry: libgnomeui-devel Thats it i think, great progress so far! Once these final issues are resolved i'll give it the final formal run thru
Last 2 small comments: - No ; needed after the done (in the for doc ... loop), also no ; needed after the rm -f line - Could you please add: %dir %{_datadir}/pixmaps/gnome-yum before the: %{_datadir}/pixmaps/gnome-yum line? Thats all i could find :-)
Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-7.src.rpm
Thanks that solves all the issues reported However it looks like you modified the source.tar.bz2 file? MD5SUM now missmatches md5sum: 43e4f678a9243c92297f0929e7d87efe (original from sf.net) gnome-yum-0.1.2.tar.bz2 bc105a3711409e8bdd66522caa59b316 (from src.rpm) gnome-yum-0.1.2.tar.bz2 I presume you've done this to include the X-Fedora category? Might be easier to use the original source and in your spec file do: desktop-file-install --vendor fedora --delete-original \ --dir $RPM_BUILD_ROOT%{_datadir}/applications \ --add-category X-Fedora \ $RPM_BUILD_ROOT%{_datadir}/applications/gnome-yum.desktop It is a strict policy for Fedora Extra's that source tarbal must match the original one (checked with md5sum) so unfortunatly i can't accept this package yet. Please either update the upstream tarbal, or use the original one and add the --add-category X-Fedora to the desktop-file-install Other then that its looking great, and builds cleanly in mock too. I will give it the formal review once the source tarbal issue is resolved.
I need modify the next files in the source package: ./configure.in and ./configure => remove gtkhtml2-devel checking src/callbacks.c => remove one line: #include <libgtkhtml/gtkhtml.h> ./README,./TODO => to be not empty ./gnome-yum.spec => fix format bugs ./desktop.in => Add fedora category If it is necessary I make a new release (v0.1.3). It is OK?
Making a 1.3.0 release would be preferable then yes Or alternativly you could make a patch file and include it in the spec/rpm that makes the changes you mentioned, but since your able to change this upstream, making a new release would be easier yes :-) As soon as you have that ready and with a new .spec / .src.rpm i'll give it the formal review. Thanks for the effort so far!
Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec SRPM Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.3-1.src.rpm
Looks good, the formal review: MUST review items: - Builds cleanly on FC5 devel. - rpmlint has no output / complaints - Source included matches upsteam source (md5sum) - Package name meets guidelines - spec file name is in %{name}.spec format - Licence (GPL) is fedora extra's compatible & is included in spec - Spec file is in (american) english - Does not list buildrequires that are excepted in the package guidelines - All build dependencies are listed - No ldconfig needed - All files have proper permissions - Package is not relocatable - No duplicate files in %files section - No missing files in %files section - Has a proper %clean section with rm -rf $RPM_BUILD_ROOT - Uses macro's described in PackagingGuidelines - No entries in %doc that are required for standard program operation - No -devel package needed - No directory-ownerships needed Should items: - Includes upstream licence file (COPYING) - No insane scriplets - No unnescesarry requires rpmlint's only warning is: W: gnome-yum no-dependency-on usermode-consoleonly Mock builds cleanly for fc-devel Please read: - http://fedoraproject.org/wiki/Extras/NewPackageProcess and - http://fedoraproject.org/wiki/Extras/Contributors and - http://fedoraproject.org/wiki/Extras/UsingCvsFaq Carefully for the next steps FE-APPROVED
Any particular reason why this package is not yet imported and built ?
Most likely because I just sponsored the packager this past weekend.
Next time please add a _valid_ email address in CVS owners.list, where "valid" means it is used in your bugzilla account!
Why the toth_bandi.net isn't valid e-mail address?
Okay. I tested this address and really it isn't working. The problem is the mail system of sourceforge.net. I am waiting any times and if it is necessary I shall change e-mail address...
> Why the toth_bandi.net isn't valid e-mail address? You added a different address to owners.list: https://www.redhat.com/archives/fedora-extras-commits/2006-April/msg00137.html https://www.redhat.com/archives/fedora-extras-commits/2006-April/msg00138.html
package is in the repos. Clsoing bug. Please remember to close package reviews once they have been imported into cvs etc etc