Hide Forgot
Spec URL: http://cwickert.fedorapeople.org/review/gtrayicon.spec SRPM URL: http://cwickert.fedorapeople.org/review/gtrayicon-1.1-1.fc12.src.rpm Description: Gtrayicon is a generic tray icon for GNOME. It defines two actions: activate and deactivate, that are alternated by clicking in the tray icon. You choose what commands to run on each action. You can also define each action's icon and a custom menu that is shown on left click. A sample menu file is included with the package. Check /usr/share/doc/gtrayicon-1.1/sample.menu for more info.
Christoph, you should avoid compressing the man pages manually as the compression format depends on the target system and rpm applies it automatically. I'd suggest to modify the %build and %install section like this: %build make %{?_smp_mflags} CFLAGS="%{optflags}" %install rm -rf %{buildroot} make install DESTDIR=%{buildroot} INSTALL='install -p' install -Dpm0644 Debian/%{name}.1 %{buildroot}/%{_mandir}/man1/%{name}.1
Oh, and I forgot to mention the %files section: %{_mandir}/man1/* :)
The licensing is also a bit unclear: - LICENSE contains GPLv3 - gtrayicon.glade refers to GPLv3+ - the SVG files link to GPLv2: <cc:License rdf:about="http://creativecommons.org/licenses/GPL/2.0/">
(In reply to comment #2) > Oh, and I forgot to mention the %files section: > %{_mandir}/man1/* I understand the rest of your points but not this one. What's the benefit of changing the line?
Maybe %{_mandir}/man1/* is a bit too general, %{_mandir}/man1/%{name}.1* is probably a better choice. I just wanted to ensure that all dependencies on a particular compression format (including the suffix) are avoided. The above variants will even work on targets with uncompressed man pages.
Do we have such targets in Fedora? Do we have any manpages not gzipped?
I don't know. :) But that's the point. I don't like to make too many assumptions if not necessary. If Fedora decided to change the compression format, the spec file would have to be adapted. Why not just avoid that by changing a few characters? However, I'm still familiarizing with Fedora packaging. If you say the explicit .gz suffix is fine then it is. To me at first glance, it's an inconsistency because the automatic compression is a transparent action and one should not rely on specific results.
Sorry, this slipped off my radar. I agree that what you say makes some sense. However I don't see a reason to change the spec because *if* one day Fedora really decided to switch to another compression method for manpages, this would require a mass rebuild anyway. Enough time to change the spec when really needed and not based on vague assumptions. In several years of using Linux I have never seen manpages *not* gzipped. My primary objective is not accidentally packaging uncompressed or even unwanted files, e.g. due to a bad Makefile. If you really think there should be no prefixes for manpages, please bring this suggestion up to the packaging committee on fedora-packaging-list.
(In reply to comment #3) > The licensing is also a bit unclear: > - LICENSE contains GPLv3 > - gtrayicon.glade refers to GPLv3+ results in GPLv3+ > - the SVG files link to GPLv2: Not sure if this is really intended, but if, this would result in "License: GPLv2 and GPLv3+" in the spec. Thanks for pointing this out.
(In reply to comment #8) > If you really think there should be no prefixes for manpages, please bring this > suggestion up to the packaging committee on fedora-packaging-list. No, it's not that important to me and I think it's also some kind of personal preference. As far as I noticed, a lot of Fedora packagers seem to prefer omitting the suffix, though.
$ rpmlint gtrayicon.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint gtrayicon-1.1-1.fc12.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint gtrayicon-1.1-1.fc10.x86_64.rpm gtrayicon-debuginfo-1.1-1.fc10.x86_64.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. MUSTs ----- OK: packaged is named according to the package naming guidelines OK: specfile name matches %{name}.spec OK: package seems to meet packaging guidelines NOT OK: license in specfile matches actual license and meets licensing guidelines OK: license file is included in %doc OK: specfile is written in AE OK: specfile is legible OK: sourcefile in the package is the same as provided in the mentioned source, md5sum fits OK: package compiles successfully OK: all build dependencies are listed in BuildRequires N/A: package handles locales properly there are no locales installed with this package N/A: call ldconfig in %post and %postun the installed binary is not linked against any library OK: package is not designed to be relocatable OK: package owns directorys it creates OK: does not list a file more than once in the %files listing OK: %files section includes %defattr and permissions are set properly OK: %clean section is there and contains rm -rf %{buildroot} OK: macros are consistently used OK: package contains code N/A: subpackage for large documentation files there are no large documentation files OK: program runs properly without files listed in %doc N/A: header files are in a -devel package there are no header files N/A: static libraries are in a -static package there are no static libs N/A: require pkgconfig if package contains a pkgconfig(.pc) there is no pkgconfig file N/A: put .so-files into -devel package if there are library files with suffix there isn't any library installed with this package N/A: devel package includes fully versioned dependency for the base package there is no devel package N/A: any libtool archives are removed there are no libtool archives NOT OK: contains desktop file if it is a GUI application this package actually provides GUI, but there is no desktop-file OK: package does not own any files or directories owned by other packages OK: buildroot is removed at beginning of %install N/A: filenames are encoded in UTF-8 not necessary since there are no non-ASCII filenames SHOULD ------ N/A: non-English translations for description and summary a localization is not available for this package OK: package builds in mock OK: package builds into binary rpms for all supported architectures OK: program runs N/A: subpackages contain fully versioned dependency for the base package there are no subpackages N/A: pkgconfig file is placed in a devel package there is no pkgconfig file N/A: require package providing a file instead of the file itself no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required Please update the License-Filed in specfile accordingly to what you wrote before to GPLv3+ and contact upstream regarding the license issue in the *.svg-files. There also is no licensing hint in the sourcefile (gtrayicon.c) itself, so you may want to suggest upstream to fix that. A desktop-file seems to be missing. This obviously is a GUI application, how is it intended to be started without an item in a menu? If necessary, please add the desktop-file. As Martin already said, since RPM applies compression for the manpage itself, you also should consider to apply his suggestion to modify the %build and %install section. Once these three issues are fixed or clarified I will approve this package.
(In reply to comment #11) > Please update the License-Filed in specfile accordingly to what you wrote > before > to GPLv3+ and contact upstream regarding the license issue in the *.svg-files. I have contacted the author of the program but got no reply so far. I think the licensing of the icons is different than the one from the program, because they were done by two different authors, who AFAIK published them as GPLv2+. So we could use GPLv3+ in the spec, but I'll wait for further clarification from upstream first. > There also is no licensing hint in the sourcefile (gtrayicon.c) itself, so you > may want to suggest upstream to fix that. Will do. > A desktop-file seems to be missing. This obviously is a GUI application, how is > it intended to be started without an item in a menu? If necessary, please add > the desktop-file. A desktop file is useless because the command to run gtraycon needs to be customized. Just calling grayicon doesn't work, you need to specify the commands for 'activate' and 'deactivate' and optionally a custom menu file. The resulting commandline should be added in the session-properties to autostart the trayicon, so there really is no need for a desktop file. > As Martin already said, since RPM applies compression for the manpage itself, > you also should consider to apply his suggestion to modify the %build and > %install section. Agreed. I will remove the compression statement and the suffix on the file. Nevertheless I think that Martin should bring up the suffix topic on fedora-packaging because it really makes sense what he said.
Reply from upstream: "Sure, I want to stick to GPLv3+, I just used gnome artwork because I found it was GPL too, but I messed with versions." So GPLv3+ for the whole thing is correct. I have also fixed the other issues (except from the desktop file). SPEC: http://cwickert.fedorapeople.org/review/gtrayicon.spec SRPM: http://cwickert.fedorapeople.org/review/gtrayicon-1.1-2.fc12.src.rpm
$ rpmlint gtrayicon.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint gtrayicon-1.1-2.fc11.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint gtrayicon-1.1-2.fc11.x86_64.rpm gtrayicon-debuginfo-1.1-2.fc11.x86_64.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. MUSTs ----- OK: packaged is named according to the package naming guidelines OK: specfile name matches %{name}.spec OK: package seems to meet packaging guidelines OK: license in specfile matches actual license and meets licensing guidelines OK: license file is included in %doc OK: specfile is written in AE OK: specfile is legible OK: sourcefile in the package is the same as provided in the mentioned source, md5sum fits OK: package compiles successfully OK: all build dependencies are listed in BuildRequires N/A: package handles locales properly there are no locales installed with this package N/A: call ldconfig in %post and %postun the installed binary is not linked against any library OK: package is not designed to be relocatable OK: package owns directorys it creates OK: does not list a file more than once in the %files listing OK: %files section includes %defattr and permissions are set properly OK: %clean section is there and contains rm -rf %{buildroot} OK: macros are consistently used OK: package contains code N/A: subpackage for large documentation files there are no large documentation files OK: program runs properly without files listed in %doc N/A: header files are in a -devel package there are no header files N/A: static libraries are in a -static package there are no static libs N/A: require pkgconfig if package contains a pkgconfig(.pc) there is no pkgconfig file N/A: put .so-files into -devel package if there are library files with suffix there isn't any library installed with this package N/A: devel package includes fully versioned dependency for the base package there is no devel package N/A: any libtool archives are removed there are no libtool archives OK: contains desktop file if it is a GUI application This is a GUI program whichs needs to be started with arguments from command line OK: package does not own any files or directories owned by other packages OK: buildroot is removed at beginning of %install N/A: filenames are encoded in UTF-8 not necessary since there are no non-ASCII filenames SHOULD ------ N/A: non-English translations for description and summary a localization is not available for this package OK: package builds in mock OK: package builds into binary rpms for all supported architectures OK: program runs N/A: subpackages contain fully versioned dependency for the base package there are no subpackages N/A: pkgconfig file is placed in a devel package there is no pkgconfig file N/A: require package providing a file instead of the file itself no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required APPROVED.
Thanks for the review! New Package CVS Request ======================= Package Name: gtrayicon Short Description: Generic tray icon for GNOME Owners: cwickert Branches: F-10 F-11 F-12 InitialCC:
cvs done.
gtrayicon-1.1-2.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/gtrayicon-1.1-2.fc12
gtrayicon-1.1-2.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/gtrayicon-1.1-2.fc11
gtrayicon-1.1-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/gtrayicon-1.1-2.fc10
gtrayicon-1.1-2.fc11 has been pushed to the Fedora 11 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update gtrayicon'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10565
gtrayicon-1.1-2.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update gtrayicon'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-10587
gtrayicon-1.1-2.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
gtrayicon-1.1-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.