Spec URL: http://people.freedesktop.org/~david/polkit-gnome.spec SRPM URL: http://people.freedesktop.org/~david/polkit-gnome-0.92-0.git20090527.fc11.src.rpm Description: PolicyKit integration for the GNOME desktop. This depends on polkit, see bug 502919.
Created attachment 345821 [details] add build requires This spec adds enough build requires to make the package build.
rpmlint output [mclasen@planemask rpmbuild]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm polkit-gnome.src: W: non-standard-group Unspecified polkit-gnome.src: W: no-url-tag polkit-gnome.x86_64: W: non-standard-group Unspecified polkit-gnome.x86_64: W: no-url-tag polkit-gnome.x86_64: W: no-documentation polkit-gnome-debuginfo.x86_64: W: no-url-tag 3 packages and 0 specfiles checked; 0 errors, 6 warnings.
Created attachment 345829 [details] pendantic fixes This spec file has some more pedantic fixes. With this, rpmlint reports: 3 packages and 0 specfiles checked; 0 errors, 0 warnings. Basing the review on this.
checklist rpmlint: see above package name: matches tarball name, ok spec file name: ok packaging guidelines: ok license: ok license field: ok license file: ok, but should probably updated to be straight LGPL, since the daemon etc are gone spec file language: ok spec file legible: ok upstream sources: missing buildable: ok excludearch: n/a build deps: ok locales: ok shared libs: n/a relocatable: n/a directory ownership: ok duplicate files: ok permissions: ok %clean: ok macro use: ok content: ok large docs: n/a %doc content: ok header files: n/a static libs: n/a pc files: n/a shared libs: n/a devel deps: ok gui apps: ok file ownership: ok %install: ok utf8 filenames: ok Should drop the --vendor gnome in the desktop-file-install call
Created attachment 345932 [details] nuked --vendor Nuked the --vendor for desktop-file install. I've fixed the COPYING file in the upstream tree http://git.gnome.org/cgit/PolicyKit-gnome/commit/?id=96e92020e8f98b5b290215d9f6c7e02861c7bfd5 and this will be in 0.92 (won't build it anyway until 0.92 releases are out).
ok, approved. I assume the 0.92 releases will find a permanent upstream location, too...
New Package CVS Request ======================= Package Name: polkit-gnome Short Description: PolicyKit integration for the GNOME desktop Owners: davidz Branches: InitialCC:
CVS done.
I really think this package should Require gnome-session, especially as it's just for directory ownership. In this case duplicate ownership is perfectly ok as none of the packages really requires the other. If ths package required gnome-session we will have a *lot* of Gnome bits (GConf2, control-center, gnome-desktop, gnome-icon-theme, gnome-menus and their deps) on every livecd that contains a system-config tool or requies polkit-gnome somehow. Please reconsider this requirement.
(In reply to comment #9) > I really think this package should Require gnome-session, ^ Sorry, the word NOT was missing here. Some more comments: - License tag is wrong, should be LGPLv2+ instead of LGPLv2 - Why does %configure check for GConf? - "BuildRequires: gnome-doc-utils" seems wrong to me, AFAIKS only gtkdoc-check from gtk-doc is needed. - What is the use of " --enable-gtk-doc" if there are no docs? - package does not use parallel make, see https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make - Timestamps not preserved during make install, see https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps - This is the successor of PolicyKit-gnome, but the Provides and Obsoletes for it are missing. - HACKING and TODO are missing from %doc, possibly also NEWS, but this one is very outdated
- Why does %configure check for GConf? Because it does. Remember, this is a git snapshot, so don't expect perfect polish. - "BuildRequires: gnome-doc-utils" seems wrong to me, AFAIKS only gtkdoc-check from gtk-doc is needed. Well, thats what you think. Had you tried to build in mock, like I did, you'd see that gnome-doc-utils is needed for the build to succeed. - This is the successor of PolicyKit-gnome, but the Provides and Obsoletes for it are missing. You want us to break rawhide until all the porting is done ? Really ? - HACKING and TODO are missing from %doc, possibly also NEWS, but this one is very outdated Both of these are not useful at all in a non-devel package, I'd say.
(In reply to comment #11) > - "BuildRequires: gnome-doc-utils" seems wrong to me, AFAIKS only gtkdoc-check > from gtk-doc is needed. > > Well, thats what you think. Had you tried to build in mock, like I did, you'd > see that gnome-doc-utils is needed for the build to succeed. > > - This is the successor of PolicyKit-gnome, but the Provides and Obsoletes for > it are missing. > > You want us to break rawhide until all the porting is done ? Really ? No, but I want this to be in the spec, even if it's commented out, so the reviewer could verify it is correct. > - HACKING and TODO are missing from %doc, possibly also NEWS, but this one is > very outdated > > Both of these are not useful at all in a non-devel package, I'd say. As long as we have no devel package they should be in the base package I think. The main problem I see is that you rewrote the spec and based your review on the rewrite. This renders the review pretty useless because nobody will realize his own errors.
> The main problem I see is that you rewrote the spec and based your review on > the rewrite. This renders the review pretty useless because nobody will realize > his own errors. No, the main problem is that you have 'licked blood' now and go out of your way to find issues where there are none.
You call a wrong license tag and missing parallel build no problems?
The license tag was a minor oversight indeed. Thanks for finding that. I don't see any problem with parallel build. Have you verified that there are no race conditions in the package if you turn it on ? See, me neither...
(In reply to comment #15) > I don't see any problem with parallel build. Have you verified that there are > no race conditions in the package if you turn it on ? See, me neither... Well, that's one of the things a reviewer should check. ;)
Package Change Request ====================== Package Name: polkit-gnome New Branches: el7 Owners: leigh123linux
No such branch as yet.
Perhaps you could try epel7 but I am not sure if it is completely ready yet.
(In reply to Jens Petersen from comment #19) > Perhaps you could try epel7 but I am not sure if it is completely ready yet. Thanks thats listed at koji as a tag https://koji.fedoraproject.org/koji/buildtargetinfo?targetID=124 Package Change Request ====================== Package Name: polkit-gnome New Branches: epel7 Owners: leigh123linux
Git done (by process-git-requests).