Spec URL: http://people.freedesktop.org/~david/udisks2.spec SRPM URL: http://people.freedesktop.org/~david/udisks2-1.90.0-0.git20111122.fc16.src.rpm Description: udisks provides a daemon, D-Bus API and command line tools for managing disks and storage devices. This is udisks, version 2. It is parallel installable with the current udisks 1.x series (shipped in the udisks and udisks-devel packages). This enables users such as KDE or GNOME to smoothly migrate to udisks2 at their own pace.
You should probably refer to 'udisks2' in the %description of the main package, as well as the subpackage descriptions. Or maybe just add 'This package contains version 2 of udisks.' for the main package. %defattr(-,root,root,-) is no longer needed There have been requests before to move the bash completion to /etc/bash-completion.d (https://bugzilla.redhat.com/show_bug.cgi?id=584569). Something to consider, maybe. Other than that, the spec file looks fine at first glance. Will do a formal review soon.
Building in mock reveals a missing BuildRequires: gobject-introspection-devel After adding that line, it builds ok. rpmlint output: rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/udisks2-*.rpm udisks2.src: W: spelling-error %description -l en_US udisks -> disks, u disks, Saudis udisks2.src:98: E: hardcoded-library-path in /lib/udisks2/udisksd udisks2.src: W: file-size-mismatch udisks-1.90.0.git20111122.tar.bz2 = 584775, http://people.freedesktop.org/~david/udisks-1.90.0.git20111122.tar.bz2 = 640107 udisks2.x86_64: W: unexpanded-macro dependency dbus >= %{dbus_version} %{dbus_version} udisks2.x86_64: W: spelling-error %description -l en_US udisks -> disks, u disks, Saudis udisks2.x86_64: E: executable-sourced-script /etc/profile.d/udisksctl-bash-completion.sh 0755L udisks2.x86_64: E: non-standard-dir-perm /var/lib/udisks2 0700L udisks2.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/org.freedesktop.UDisks2.conf udisks2.x86_64: W: no-manual-page-for-binary umount.udisks2 3 packages and 0 specfiles checked; 3 errors, 6 warnings. Some things to fix here: - The hardcoded library path is fine, but you need to add a %dir /lib/udisks2 to own the directory. - The file size mismatch should be investigated and fixed. - You need to define dbus_version - Should make /etc/profile.d/udisksctl-bash-completion.sh non-executable (and, as already mentioned, consider moving it) - You should probably add a comment in the file list explaining the permissions of /var/lib/udisks2
Review checklist: package name: ok spec file name: ok packaging guidelines: ok, apart from whats mentioned in the previous comment license: ok license field: ok license file: ok spec language: ok spec readable: ok upstream sources: see the rpmlint complaint above buildable: ok, module missing BR excludearch: ok buildrequires: see initial comment, needs gobject-introspection-devel locale handling: ok ldconfig: ok system libs: ok relocatable: ok dir ownership: see above, must own /lib/udisks2 duplicate files: ok permissions: ok. note that defattr() is no longer needed macro use: ok permissible content: ok large docs: ok %doc content: ok headers: ok static libs: ok shared libs: ok devel deps: ok libtool archives: ok gui apps: ok file ownership: ok utf8 filenames: ok
Hey, thanks for the review. I've uploaded new files http://people.freedesktop.org/~david/udisks2.spec http://people.freedesktop.org/~david/udisks2-1.90.0-0.git20111128.fc16.src.rpm Here's a unified diff of the spec file changes made: http://people.freedesktop.org/~david/udisks2.spec.diff Inline comments: (In reply to comment #1) > You should probably refer to 'udisks2' in the %description of the main package, > as well as the subpackage descriptions. Or maybe just add 'This package > contains version 2 of udisks.' for the main package. I've added "This package is for the udisks 2.x series" to all package descriptions. > %defattr(-,root,root,-) is no longer needed Nuked. > There have been requests before to move the bash completion to > /etc/bash-completion.d (https://bugzilla.redhat.com/show_bug.cgi?id=584569). > Something to consider, maybe. OK, don't really agree but not worth fighting. Changed upstream. (In reply to comment #2) > Building in mock reveals a missing > > BuildRequires: gobject-introspection-devel > > After adding that line, it builds ok. Fixed. > rpmlint output: > > rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/udisks2-*.rpm > udisks2.src: W: spelling-error %description -l en_US udisks -> disks, u disks, > Saudis > udisks2.src:98: E: hardcoded-library-path in /lib/udisks2/udisksd > udisks2.src: W: file-size-mismatch udisks-1.90.0.git20111122.tar.bz2 = 584775, > http://people.freedesktop.org/~david/udisks-1.90.0.git20111122.tar.bz2 = 640107 > udisks2.x86_64: W: unexpanded-macro dependency dbus >= %{dbus_version} > %{dbus_version} > udisks2.x86_64: W: spelling-error %description -l en_US udisks -> disks, u > disks, Saudis > udisks2.x86_64: E: executable-sourced-script > /etc/profile.d/udisksctl-bash-completion.sh 0755L > udisks2.x86_64: E: non-standard-dir-perm /var/lib/udisks2 0700L > udisks2.x86_64: W: non-conffile-in-etc > /etc/dbus-1/system.d/org.freedesktop.UDisks2.conf > udisks2.x86_64: W: no-manual-page-for-binary umount.udisks2 > 3 packages and 0 specfiles checked; 3 errors, 6 warnings. > > Some things to fix here: > > - The hardcoded library path is fine, but you need to add a > %dir /lib/udisks2 to own the directory. Done. > - The file size mismatch should be investigated and fixed. I did a new tarball with the same name but forgot to put it online. > - You need to define dbus_version Done. > - Should make /etc/profile.d/udisksctl-bash-completion.sh non-executable > (and, as already mentioned, consider moving it) Done upstream, see above. > - You should probably add a comment in the file list explaining > the permissions of /var/lib/udisks2 Added. (In reply to comment #3) > Review checklist: > > package name: ok > spec file name: ok > packaging guidelines: ok, apart from whats mentioned in the previous comment > license: ok > license field: ok > license file: ok > spec language: ok > spec readable: ok > upstream sources: see the rpmlint complaint above Should be fixed. > buildable: ok, module missing BR Fixed. > excludearch: ok > buildrequires: see initial comment, needs gobject-introspection-devel Fixed. > locale handling: ok > ldconfig: ok > system libs: ok > relocatable: ok > dir ownership: see above, must own /lib/udisks2 Fixed. > duplicate files: ok > permissions: ok. note that defattr() is no longer needed Fixed. > macro use: ok > permissible content: ok > large docs: ok > %doc content: ok > headers: ok > static libs: ok > shared libs: ok > devel deps: ok > libtool archives: ok > gui apps: ok > file ownership: ok > utf8 filenames: ok Thanks for the review.
Looks all good now, approved.
New Package SCM Request ======================= Package Name: udisks2 Short Description: Disk Manager Owners: davidz Branches: f17 InitialCC:
Git done (by process-git-requests). No need to request f17, ==devel.
Thanks, packages built: http://koji.fedoraproject.org/koji/taskinfo?taskID=3548581