Bug 483696
| Summary: | Review Request: libdmapsharing - library for DAAP and DPAP | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | W. Michael Petullo <mike> |
| Component: | Package Review | Assignee: | Michael Schwendt <bugs.michael> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | bugs.michael, fedora-package-review, notting |
| Target Milestone: | --- | Flags: | bugs.michael:
fedora-review+
kevin: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-03-11 01:03:05 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
W. Michael Petullo
2009-02-03 02:47:44 UTC
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. * 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? 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. 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.
New Package CVS Request ======================= Package Name: libdmapsharing Short Description: libdmapsharing implements the DMAP protocols. Owners: mikep Branches: F-9 F-10 InitialCC: cvs done. |