Spec URL: http://chkr.fedorapeple.org/review/dbus-sharp.spec SRPM URL: http://chkr.fedorapeople.org/review/dbus-sharp-0.7.0-1.fc17.src.rpm Description: D-Bus mono bindings for use with mono programs. Please note that this is a special re-review since there is already a package "dbus-sharp" in Fedora. The new "dbus-sharp" is a fork of "ndesk-dbus-sharp" which was a fork of the original "dbus-sharp". :-) The new package is not API-compatible to the original one and the license has changed as well. So technically it is not a simple update but a different package and so a re-review is required. Since the old "dbus-sharp" package is not used anymore by any other packages there should be no issue in updating the existing "dbus-sharp" package to the new version.
Spec URL: http://chkr.fedorapeople.org/review/dbus-sharp.spec
I'll be happy to review this one. :) I'm running some test builds now; and should have initial feedback for you later tonight. Regards.
Review of dbus-sharp-0.7.0-1: NEEDSWORK Just a couple of small problems (see the "NEEDS WORK" section below); but once those are fixed, this package looks good to go. =) === GOOD === These are items from the review checklist [2] which are being followed correctly. + rpmlint is clean on the source RPM. + Package is named according to the package naming guidelines, and the spec file is named accordingly ("%{name}.spec"). + Meets the packaging guidelines as outlined on the Wiki. + The license (MIT) is acceptable for Fedora, included as documentation (%doc), and the package's license matches that of upstream. + Spec file is legible, and written in American English. + The included sources match the source tarball given at Source0. (I checked this with SHA-256 checksums, as shown.) $ sha256sum dbus-sharp-0.7.0.tar.gz* 92529aef9063f477d1975947c6388c63d03234018f45d007c07716dd3e21dd41 dbus-sharp-0.7.0.tar.gz-fedora 92529aef9063f477d1975947c6388c63d03234018f45d007c07716dd3e21dd41 dbus-sharp-0.7.0.tar.gz-upstream + Succesfully builds in Mock for Rawhide on both i386 and x86_64, with appropriate Mono-specific ExcludeArch used. + Dependencies (both build- and runtime) look sane. + Package does not bundle duplicates or copies of system libraries. + File/directory ownership and permissions are handled correctly, with no duplicates of system stuff, and no unowned or orphaned directories within the package itself. + Macro usage is clear and consistent. + Included documentation (README and COPYING) does not appear to affect runtime. + pkconfig file is correctly shipped in a -devel subpackage; and that -devel subpackage correctly hardcodes a dependency on the main package: Requires: %name = %{epoch}:%{version}-%{release} + Package contains acceptable code. + All file names are valid UTF-8. === NEEDS WORK === These issue(s) are problematic and prevent the package from being approved. (1) rpmlint output is not clean on the binary RPMs: dbus-sharp.x86_64: E: no-binary dbus-sharp.x86_64: W: only-non-binary-in-usr-lib dbus-sharp-devel.x86_64: W: no-documentation 2 packages and 0 specfiles checked; 1 errors, 2 warnings. According to Packaging:Mono on the Wiki [1], it should install its files in the GAC in /usr/lib or /usr/lib/dbus-sharp, and *not* in an arch-specific location such as /usr/lib64. That should fix the first two complaints. The no-documentation warning is probably safe to ignore; but if you'd want to do so for clarity, you could opt to put a copy of the COPYING and/or README file in that too. (2) After compilation and installation to the buildroot, the DLL needs to be registered with gacutil. (This is also explained on the wiki page.) Add something like the following to your %install section: gacutil -i mono/dbus-sharp/dbus-sharp.dll -f -package dbus-sharp -root %{buildroot}/usr/lib (You may need to adjust the directory as necessary.) (3) The %defattr line in your %files section is not necessary with current RPM versions. Please remove it. (This particular one is a nitpick of mine. It's technically optional; but I feel the spec file would be cleaner with it removed.) === NOT APPLICABLE === These are items from the review checklist [2] that do not pertain to this particular package. ~ Package does not use locales, so %find_lang is not necessary. ~ Package does not store native shared libraries (with or without version suffixes), so ldconfig calls are also not necessary. There are also no static libraries. nor libtool archives. ~ Package is not designed to be relocatable. ~ Documentation is not very large, so putting it in its own -doc subpackage is also not necessary. ~ Package is not a GUI application, so no .desktop file is required. ~ No translations are available for Summary/Description. ~ No file-based dependencies are used. ~ Package contains no manual pages. === References === [1] http://fedoraproject.org/wiki/Packaging:Mono [2] http://fedoraproject.org/wiki/Packaging:ReviewGuidelines
Thank you very much for the review. (In reply to comment #3) > Review of dbus-sharp-0.7.0-1: NEEDSWORK > === NEEDS WORK === > These issue(s) are problematic and prevent the package from being approved. > > (1) rpmlint output is not clean on the binary RPMs: > > dbus-sharp.x86_64: E: no-binary > dbus-sharp.x86_64: W: only-non-binary-in-usr-lib > dbus-sharp-devel.x86_64: W: no-documentation > 2 packages and 0 specfiles checked; 1 errors, 2 warnings. > > According to Packaging:Mono on the Wiki [1], it should install its files in the > GAC in /usr/lib or /usr/lib/dbus-sharp, and *not* in an arch-specific location > such as /usr/lib64. That should fix the first two complaints. The > no-documentation warning is probably safe to ignore; but if you'd want to do so > for clarity, you could opt to put a copy of the COPYING and/or README file in > that too. Although the current packaging guidelines already refer to the new directory scheme, it is not yet implemented. Even the mono core packages still expect the old-style arch-independent paths. We are currently in the process of updating rawhide, but until this has finished, we still have to use the old paths. For reference: https://fedoraproject.org/wiki/User:Chkr/MonoMultiarchChanges https://fedorahosted.org/fpc/ticket/91 > dbus-sharp.x86_64: E: no-binary > dbus-sharp.x86_64: W: only-non-binary-in-usr-lib These two warnings are "normal" for mono packages (even if we would solely use /usr/lib), they are false positives caused by the fact that mono assemblies are not treated as binaries. > dbus-sharp-devel.x86_64: W: no-documentation I have not found an API documentation in the package, so I would rather leave it as it is than to package the README again just for keeping rpmlint quiet. ;-) > (2) After compilation and installation to the buildroot, the DLL needs to be > registered with gacutil. (This is also explained on the wiki page.) Add > something like the following to your %install section: > > gacutil -i mono/dbus-sharp/dbus-sharp.dll -f -package dbus-sharp -root > %{buildroot}/usr/lib > > (You may need to adjust the directory as necessary.) That's only necessary if it is not done automatically via make. In this case it happens in the "make install" step. To verify you can look into the binary package: rpm -qlp /tmp/dbus-sharp-0.7.0-1.fc17.i686.rpm /usr/lib/mono/dbus-sharp-1.0 /usr/lib/mono/dbus-sharp-1.0/dbus-sharp.dll /usr/lib/mono/gac/dbus-sharp /usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5 /usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5/dbus-sharp.dll /usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5/dbus-sharp.dll.config /usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5/dbus-sharp.dll.mdb [...] The assembly is already correctly installed/registered in the GAC. > (3) The %defattr line in your %files section is not necessary with current RPM > versions. Please remove it. (This particular one is a nitpick of mine. It's > technically optional; but I feel the spec file would be cleaner with it > removed.) Fixed. Here is now the updated package: Spec URL: http://chkr.fedorapeople.org/review/dbus-sharp.spec SRPM URL: http://chkr.fedorapeople.org/review/dbus-sharp-0.7.0-2.fc17.src.rpm
(In reply to comment #4) > Although the current packaging guidelines already refer to the new directory > scheme, it is not yet implemented. Even the mono core packages still expect the > old-style arch-independent paths. We are currently in the process of updating > rawhide, but until this has finished, we still have to use the old paths. > > For reference: > https://fedoraproject.org/wiki/User:Chkr/MonoMultiarchChanges > https://fedorahosted.org/fpc/ticket/91 Ah, I learned something new today. Excellent, and thank you for the pointers. ;) > These two warnings are "normal" for mono packages (even if we would solely use > /usr/lib), they are false positives caused by the fact that mono assemblies are > not treated as binaries. That's right - these are both false positives, now that you've explained the the multi-arch transition. So these can be safely ignored. > That's only necessary if it is not done automatically via make. In this case it > happens in the "make install" step. > > [...] > > The assembly is already correctly installed/registered in the GAC. > Aha - I had not noticed that at first. Thank you for the clarification. In that case, everything looks good to me with the updated spec/SRPM. dbus-sharp-0.7.0-2 is APPROVED. Thanks for packaging this! :)
gnome-do-plugins-0.8.4-3.fc16.1,banshee-community-extensions-2.2.0-1.fc16,banshee-2.2.1-1.fc16,dbus-sharp-glib-0.5.0-1.fc16,dbus-sharp-0.7.0-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/gnome-do-plugins-0.8.4-3.fc16.1,banshee-community-extensions-2.2.0-1.fc16,banshee-2.2.1-1.fc16,dbus-sharp-glib-0.5.0-1.fc16,dbus-sharp-0.7.0-3.fc16
Package gnome-do-plugins-0.8.4-3.fc16.1, banshee-community-extensions-2.2.0-1.fc16, banshee-2.2.1-1.fc16, dbus-sharp-glib-0.5.0-1.fc16, dbus-sharp-0.7.0-3.fc16: * should fix your issue, * was pushed to the Fedora 16 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing gnome-do-plugins-0.8.4-3.fc16.1 banshee-community-extensions-2.2.0-1.fc16 banshee-2.2.1-1.fc16 dbus-sharp-glib-0.5.0-1.fc16 dbus-sharp-0.7.0-3.fc16' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2011-16171/gnome-do-plugins-0.8.4-3.fc16.1,banshee-community-extensions-2.2.0-1.fc16,banshee-2.2.1-1.fc16,dbus-sharp-glib-0.5.0-1.fc16,dbus-sharp-0.7.0-3.fc16 then log in and leave karma (feedback).
gnome-do-plugins-0.8.4-3.fc16.1, banshee-community-extensions-2.2.0-1.fc16, banshee-2.2.1-1.fc16, dbus-sharp-glib-0.5.0-1.fc16, dbus-sharp-0.7.0-3.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report.