Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-1.src.rpm Description: A simple KDE network monitor that show rx/tx LEDs or numeric information about the transfer rate of any network interface in a system tray icon.
Hello, there is my first package to fedora extras :) I have 2 issues with it though: 1. how can I add a link to the menu ? 2. I can't compile it with QA_RPATHS=$[ 0x0001|0x0010 ]. Any Workaround ?
issue 1 : it automatically adds itself in Kmenu->Internet issue 2 : rpmbuilding through QA_RPATHS=$[ 0x0001|0x0010 ] rpmbuild -ba knetstats.spec Updated: Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-2.src.rpm
Not a formal review, but here's a few things I've noticed. 1. You should probably patch the SConstruct file to remove rpath from the environment. See 'env.KDEuse("environ rpath")' in the file. 2. Add -q to the %setup macro 3. Since you're using %{version} in the source tag, consider using the %{name} macro too. 4. Drop gcc-c++ from the buildrequires, see http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28CategoryExtras%29 5. Perhaps a matter of preference, but consider using the scons supplied with the source, ie drop the scons BR and use ./scons in the build section. 6. Use the %{?_smp_mflags} on the scons build line, to allow parallel building 7. The install process attempts to create files outside the buildroot: "mkdir: cannot create directory `/usr/share/doc/HTML/en/knetstats': Permission denied" This must be fixed. 8. The last changelog entry is missing its release number (im assuming it should have been -1) 9. To fix the desktop file, one solultion might be to delete the .desktop from the buildroot after scons install 'rm -f $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/knetstats.desktop', then use desktop-file-install to install it yourself. Remember to add desktop-file-utils as a BR
Thanks for such a reply, I have updated the spec file with respect to what have been documented above except: No. 7. Because I was unable to get scons root access/permissions while its execution. No. 9. Because It automatically adds itself to the kmenu->Internet. Do I have to use desktop-file-utils nevertheless ?
Updated: Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-3.src.rpm
Hi Chitlesh, I see (from google) that you're already active in other area's fo the Fedora project, good! As such I'm willing to sponsor you, but first you must show some more / better understanding of the FE packaging guidelines. There are 2 ways to show this better understanding: 1) You review a couple of packages from others see bug 163776 for a list of Review Requests that need a Reviewer, don't worry about not being competent enough todo a review, just add me to the CC-list and I'll watch over your back. 2) Create some more packages and put me in the CC for the Review Request Or (probably the best) a combination of these 2. Now some remarks related to this review: (In reply to comment #4) > Thanks for such a reply, > > I have updated the spec file with respect to what have been documented above except: > > No. 7. > > Because I was unable to get scons root access/permissions while its execution. > You really _must_ fix this, the trick is not to give scons root permission, but to change the build so that it doesnot try to install files outside of the buildroot > No. 9. > Because It automatically adds itself to the kmenu->Internet. Do I have to use > desktop-file-utils nevertheless ? Yes, and since it already puts the .desktop file under /usr/share/applicatiuons you should use the --delete-original option to desktop-file-install.
Couple of other points, it might be better to leave a blank line between different %changelog entries as it's easier on the eye and perhaps be a little more verbose as there's no mention of the other changes you made such as dropping gcc-c++ etc. I read somewhere on the wiki (although I can't find it at the mo), that patches should be named in the following way to avoid name collisions with other packages: name-version-description.patch For example: knetstats-1.5-rpathfix.patch It also advised against using the %{name} and %{version} macros here, particularly because the version indicates the version of the package this patch was introduced in, so for example if you were to upgrade to version 1.6, the patch would still be knetstats-1.5-rpathfix.patch, if it was still applicable. Anyway Hans will keep you right, he sponsored me too :)
NEEDSWORK: * use icon cache scriptlets, see "GTK+ icon cache" on http://www.fedoraproject.org/wiki/ScriptletSnippets * use "desktop-file-install" for .desktop file, see "desktop files" on http://www.fedoraproject.org/wiki/Packaging/Guidelines Since this is your first package submissino, it's a good idea to look over all of those ScriptletSnippets and Packaging Guidelines pages, if you haven't done so already.
------- Additional Comments From cgoorah AT yahoo com au 2006-06-11 08:19 EST ------- Updated: Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-4.src.rpm I have made the necessary modifications and read the guidelines as well :) Now I need to put that into action :) Nevertheless, I was unable to fix the mkdir -p /usr/share/doc/HTML/en/knetstats && cd /usr/share/doc/HTML/en/knetstats && rm -f common && ln -s ../common commonln: creating symbolic link `common' to `../common': Permission denied How can I change the build so that it doesnot try to install files outside of the buildroot ?
------- Additional Comments From j.w.r.degoede AT hhs nl 2006-06-11 16:08 EST ------- (In reply to comment #9) > Updated: > Spec URL: http://beta.glwb.info/knetstats/knetstats.spec > SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-4.src.rpm > > I have made the necessary modifications and read the guidelines as well :) > Now I need to put that into action :) > Okay, Some (semi) initial remarks: -You BuildRequire qt-devel and you also BuildRequire kdelibs-devel, however kdelibs-devel Requires qt-devel itself, so the BR qt-devel is redundant remove please. -update-desktop-database is deprecated, remove please as well as the belongen Requires(%post[un]) -as already mentioned in comment #8 you must properly update the icon cache in %post[un] see the wiki scriptlets page -don't call /sbin/ldconfig in %post[un] this package does not seem to contain any libs -instead of: ### rm -rf $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/%{name}.desktop desktop-file-install --vendor fedora \ --add-category Network \ --dir $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/ \ %{name}.desktop ### use: ### desktop-file-install --vendor fedora \ --add-category X-Fedora \ --add-category Network \ --delete-original \ --dir $RPM_BUILD_ROOT%{_datadir}/applications/ \ $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/%{name}.desktop ### Notice that the changed --dir, desktop files should be installed in %{_datadir}/applications/ nowadays. Also notice the --delete-original replacing the seperate rm command and last notice the additional "--add-category X-Fedora" param which all fedora packages should use. Also don't forget to update %files for the changed dir. > Nevertheless, I was unable to fix the > > mkdir -p /usr/share/doc/HTML/en/knetstats && cd /usr/share/doc/HTML/en/knetstats > && rm -f common && ln -s ../common commonln: creating symbolic link `common' to > `../common': Permission denied > > change the build so that it doesnot try to install files outside of the buildroot ? > How can I Okay, this is because of a dirty hack in upstreams sources, there are 2 possible fixes: 1) ignore the error (scons already does this) and add the following at the end of %install: ln -s ../common $RPM_BUILD_ROOT%{_datadir}/doc/HTML/en/knetstats/common 2) patch admin/kde.py to properly honor DESTDIR.
------- Additional Comments From cgoorah AT yahoo com au 2006-06-11 16:55 EST ------- Hello Hans, thanks for your remarks, > Some (semi) initial remarks: > -You BuildRequire qt-devel and you also BuildRequire kdelibs-devel, however > kdelibs-devel Requires qt-devel itself, so the BR qt-devel is redundant remove Removed. > please. > -update-desktop-database is deprecated, remove please as well as the belongen > Requires(%post[un]) > -as already mentioned in comment #8 you must properly update the icon cache in > %post[un] see the wiki scriptlets page > -don't call /sbin/ldconfig in %post[un] this package does not seem to contain > any libs Updated to %post touch --no-create %{_datadir}/icons/hicolor || : %postun touch --no-create %{_datadir}/icons/hicolor || : > -instead of: > ### > rm -rf $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/%{name}.desktop > > desktop-file-install --vendor fedora \ > --add-category Network \ > --dir $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/ \ > %{name}.desktop > ### > use: > ### > desktop-file-install --vendor fedora \ > --add-category X-Fedora \ > --add-category Network \ > --delete-original \ > --dir $RPM_BUILD_ROOT%{_datadir}/applications/ \ > $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/%{name}.desktop > ### > Notice that the changed --dir, desktop files should be installed in > %{_datadir}/applications/ nowadays. Also notice the --delete-original > replacing the seperate rm command and last notice the additional > "--add-category X-Fedora" param which all fedora packages should use. > Also don't forget to update %files for the changed dir. > done . > > > > Nevertheless, I was unable to fix the > > > > mkdir -p /usr/share/doc/HTML/en/knetstats && cd /usr/share/doc/HTML/en/knetstats > > && rm -f common && ln -s ../common commonln: creating symbolic link `common' to > > `../common': Permission denied > > > > change the build so that it doesnot try to install files outside of the > buildroot ? > > How can I > > Okay, this is because of a dirty hack in upstreams sources, there are 2 possible > fixes: > 1) ignore the error (scons already does this) and add the following at the end > of %install: > ln -s ../common $RPM_BUILD_ROOT%{_datadir}/doc/HTML/en/knetstats/common > 2) patch admin/kde.py to properly honor DESTDIR. > I was able to overcome that Permission Denied issue with %install rm -fr $RPM_BUILD_ROOT scons prefix=$RPM_BUILD_ROOT%{_prefix} install but with rpmlint -i knetstats-1.5-5.i386.rpm, Ive fallen on W: knetstats dangling-relative-symlink /usr/share/doc/HTML/en/knetstats/common ../common The relative symbolic link points nowhere. With f13's advice I've patched admin/kde.py accordingly. Again with f13's advise, I've included scons as BR. Updated: Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-5.src.rpm
------- Additional Comments From rdieter AT math unl edu 2006-06-11 18:23 EST ------- Re: comment #10 > -update-desktop-database is deprecated It certainly is not deprecated (what makes you say that?). If the app's .desktop file contains MimeType= entries, ScriptletSnippets are clear that use of update-desktop-database in %post/%postun is required, (though IMO, there shouldn't be need for additional Requires for this, just like the icon cache case).
------- Additional Comments From cgoorah AT yahoo com au 2006-06-12 01:42 EST ------- (In reply to comment #12) > Re: comment #10 > > -update-desktop-database is deprecated > > It certainly is not deprecated (what makes you say that?). Hans, you want to comment on this ? Anyone want to sponsor me for knetstats ? Hans ?
------- Additional Comments From j.w.r.degoede hhs nl 2006-06-12 02:22 EST ------- (In reply to comment #12) > Re: comment #10 > > -update-desktop-database is deprecated > > It certainly is not deprecated (what makes you say that?). > Sorry, deprected in deed is the wrong way to formulate this, what I meant is usually calling update-desktop-database is no longer needed unless: > the app's .desktop file contains MimeType= entries, ScriptletSnippets are > clear that use of update-desktop-database in %post/%postun is required. --- (In reply to comment #13) > (In reply to comment #12) > > Re: comment #10 > > > -update-desktop-database is deprecated > > > > It certainly is not deprecated (what makes you say that?). > > Hans, you want to comment on this ? > See above, the knetstats does not contain any MimeType's, so the call to update-desktop-database should be removed (and the Requires). > Anyone want to sponsor me for knetstats ? Hans ? > Quoting myself from comment #6: I see (from google) that you're already active in other area's of the Fedora project, good! As such I'm willing to sponsor you, but first you must show some more / better understanding of the FE packaging guidelines. There are 2 ways to show this better understanding: 1) You review a couple of packages from others see FE-NEW for a list of Review Requests that need a Reviewer, don't worry about not being competent enough todo a review, just add me to the CC-list and I'll watch over your back. 2) Create some more packages and put me in the CC for the Review Request So yes, I'm willing to sponsor you (eventually) provided that you show a somewhat better understanding of the packaging guidelines then you've done sofar. (Sofar is ok, you're still learning I understand!). So lets first get this package approved (but not yet imported as your not sponsored), and then I think it would be a good idea for you to package something else (maybe you already have) and get that reviewed too, usually after 2 packages most people are beginning to understand the guidelines good enough to get sponsored (good enough for my taste atleast).
Good job on restoring those lost comments! About the review, I've taken a quick glance at: Spec URL: http://beta.glwb.info/knetstats/knetstats.spec And you don't properly update the icon cache, the correctway is: touch --no-create %{_datadir}/icons/hicolor || : if [ -x %{_bindir}/gtk-update-icon-cache ]; then %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || : fi And yes you need the gtk-update-icon-cache for KDE-apps too, because you want the icon to show properly in the gnome applications menu. About the dangling symlink, that dir is provided by kdelibs: [hans@shalem ~]$ rpm -qf /usr/share/doc/HTML/en/common kdelibs-3.5.3-4.x86_64 Your package should automaticly require kdelibs because of .so dependencies, but in this case you could explicitly Require it, or better perhaps you could add a: Requires: /usr/share/doc/HTML/en/common Please post a new version with these things fixed and then I'll do a full review (and sponsor you once this package is approved).
Here you go :) Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-6.src.rpm
Re: comment #15 > And yes you need the gtk-update-icon-cache for KDE-apps too, because >you want the icon to show properly in the gnome applications menu. FYI, only the 'touch' is strictly required for proper function (and adherance to the fdo icon spec). gtk-update-icon-cache only improves gtk2's icon loading performance.
MUST: ===== * rpmlint output is clean * Package and spec file named appropriately * Packaged according to packaging guidelines * License (GPL) ok, license file included * spec file is legible and in Am. English. * Source matches upstream * Compiles and builds on devel-x86_64 * BR: ok * ocales properly handled * No shared libraries * Not relocatable * Package owns / or requires all dirs * No duplicate files & Permissions ok * %clean & macro usage OK * Contains code only * %doc does not affect runtime, and isn't large enough to warrent a sub package * no -devel package needed, no libs / .la files. * .desktop file as required and properly installed MUST fix: ========= * Source0 is wrong (my fault) it should be: Source0: http://dl.sf.net/sourceforge/%{name}/%{name}-%{version}.tar.bz2 * the common symlink is broken now due to your changes it now points to common, whereas it should point to ../common (In reply to comment #17) > Re: comment #15 > > And yes you need the gtk-update-icon-cache for KDE-apps too, because > >you want the icon to show properly in the gnome applications menu. > > FYI, only the 'touch' is strictly required for proper function (and adherance to > the fdo icon spec). gtk-update-icon-cache only improves gtk2's icon loading > performance. Nope, when you install a package while running gnome and it doesn't properly call gtk-update-icon-cache the icon doesnot show in the menu, atleast not during that session. I've made that mistake myself enough times to know this.
> Nope, when you install a package while running gnome and it doesn't properly > call gtk-update-icon-cache the icon doesnot show in the menu, atleast not > during that session. I've made that mistake myself enough times to > know this. Sounds more like a gtk2 bug that it doesn't validate it's icon cache.
(In reply to comment #18) Hello, > MUST fix: > ========= > * Source0 is wrong (my fault) it should be: > Source0: http://dl.sf.net/sourceforge/%{name}/%{name}-%{version}.tar.bz2 > * the common symlink is broken now due to your changes it now points to common, > whereas it should point to ../common > I've updated it and dropped the knetstats-1.5-kde.patch Spec URL: http://beta.glwb.info/knetstats/knetstats.spec SRPM URL: http://beta.glwb.info/knetstats/knetstats-1.5-7.src.rpm
All blockers fixed -> Approved! If you create an account in the account system and sign the CLA as described here: http://fedoraproject.org/wiki/Extras/Contributors Then I'll sponsor you, after which you can import the package into CVS as described on the same page. It could be that you already have an account since you contribute to other areas of Fedora, in that case add yourself to the cvsextras group.