Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: libdmapsharing - library for DAAP and DPAP
https://bugzilla.redhat.com/show_bug.cgi?id=483696
Summary: Review Request: libdmapsharing - library for DAAP and DPAP Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: mike@flyn.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://www.flyn.org/SRPMS/libdmapsharing.spec SRPM URL: http://www.flyn.org/SRPMS/libdmapsharing-1.9.0.1-1.fc10.src.rpm Description: libdmapsharing implements the DMAP protocols. This includes support for DAAP and DPAP.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
--- Comment #1 from W. Michael Petullo mike@flyn.org 2009-02-07 10:04:30 EDT --- Spec URL: http://www.flyn.org/SRPMS/libdmapsharing.spec SRPM URL: http://www.flyn.org/SRPMS/libdmapsharing-1.9.0.1-2.fc10.src.rpm Description: libdmapsharing implements the DMAP protocols. This includes support for DAAP and DPAP.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |bugs.michael@gmx.net Flag| |fedora-review?
--- Comment #2 from Michael Schwendt bugs.michael@gmx.net 2009-03-06 14:43:18 EDT --- * Noticed you're [involved] upstream.
* Hint: Run rpmlint not just on the src.rpm, but also on the built rpms.
Requires: glib2, libsoup, avahi-glib
Should really be dropped in favour of the automatic SONAME dependencies. However: You don't add the proper LDFLAGS to link with these libraries. libdmapsharing-1.9.so.1.0.9 contains undefined symbols! That's why you don't get rpmbuild's automatic SONAME deps.
%post /sbin/ldconfig
%postun /sbin/ldconfig
Prefer
%post -p /sbin/ldconfig %postun -p /sbin/ldconfig
to run this command directly instead of via /bin/sh.
* -devel pkg ought to "Requires: pkgconfig", because it stores a file in %_libdir/pkgconfig/ and because libdmapsharing API users will likely evaluate the pkg-config --cflags/--libs values for this library.
* %doc file "INSTALL" is irrelevant to RPM package users
* The installed pkg-config file contains a hardcoded /lib in libdir. This breaks on 64-bit archs. Use @libdir@ in the .pc.in template file.
* The major library version as part of the SONAME is intentional? (libdmapsharing-1.9.so.1.0.9)
* What "license issue" does the TODO file refer to?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
--- Comment #3 from W. Michael Petullo mike@flyn.org 2009-03-06 20:54:33 EDT --- New version:
Spec URL: http://www.flyn.org/SRPMS/libdmapsharing.spec SRPM URL: http://www.flyn.org/SRPMS/libdmapsharing-1.9.0.3-1.fc10.src.rpm
Requires: glib2, libsoup, avahi-glib
Should really be dropped in favour of the automatic SONAME dependencies. However: You don't add the proper LDFLAGS to link with these libraries. libdmapsharing-1.9.so.1.0.9 contains undefined symbols! That's why you don't get rpmbuild's automatic SONAME deps.
Fixed.
%post /sbin/ldconfig
%postun /sbin/ldconfig
Prefer
%post -p /sbin/ldconfig %postun -p /sbin/ldconfig
to run this command directly instead of via /bin/sh.
Fixed
- -devel pkg ought to "Requires: pkgconfig", because it stores a file in
%_libdir/pkgconfig/ and because libdmapsharing API users will likely evaluate the pkg-config --cflags/--libs values for this library.
Fixed.
- %doc file "INSTALL" is irrelevant to RPM package users
Fixed.
- The installed pkg-config file contains a hardcoded /lib in libdir. This
breaks on 64-bit archs. Use @libdir@ in the .pc.in template file.
Fixed.
- The major library version as part of the SONAME is intentional?
(libdmapsharing-1.9.so.1.0.9)
Fixed.
- What "license issue" does the TODO file refer to?
This issue is resolved. I have received permission directly from the rhythmbox/DAAP developers for a GPL to LGPL license change for their DAAP code. Previously, I had received second hand information from the old libdmapsharing maintainer. The AUTHORS file now documents this.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #4 from Michael Schwendt bugs.michael@gmx.net 2009-03-07 09:47:18 EDT --- Okay. libdmapsharing-1.9.0.3-1.fc10.src.rpm gets my APPROVAL.
[...]
Except (!) for one problem introduced in the .pc file, which you want to fix upstream and in pkg cvs. Here's the diff:
-Libs: -L${libdir} -ldmapsharing-@LIBDMAPSHARING_MAJORMINOR@ -Cflags: -I${includedir}/libdmapsharing-@LIBDMAPSHARING_MAJORMINOR@ +Libs: -L${libdir} -ldmapsharing +Cflags: -I${includedir}/libdmapsharing
The Cflags point to a non-existant directory now. The headers are found in /usr/include/libdmapsharing-1.9/libdmapsharing/ instead. As you want API users to include <libdmapsharing/foo.h> instead of <foo.h>, the previous Cflags line was correct.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
W. Michael Petullo mike@flyn.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #5 from W. Michael Petullo mike@flyn.org 2009-03-07 14:10:11 EDT --- New Package CVS Request ======================= Package Name: libdmapsharing Short Description: libdmapsharing implements the DMAP protocols. Owners: mikep Branches: F-9 F-10 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #6 from Kevin Fenzi kevin@tummy.com 2009-03-09 11:57:06 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=483696
W. Michael Petullo mike@flyn.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla