Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 204140
Summary: | Review Request: libmtp - MTP client library | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Linus Walleij <triad> |
Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | panemade |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-10-03 10:11:09 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Linus Walleij
2006-08-25 20:30:06 UTC
New upstream version: Spec URL: http://www.df.lth.se/~triad/krad/fc/libmtp.spec SRPM URL: http://www.df.lth.se/~triad/krad/fc/libmtp-0.0.15-1.src.rpm {Not Official Reviewer} packaging looks ok. + Mockbuild is successfull for i386 FC6 + rpmlint on binary rpm is silent - rpmlint in SRPM is NOT silent rpmlint -iv ../SRPMS/libmtp-0.0.15-1.src.rpm I: libmtp checking W: libmtp mixed-use-of-spaces-and-tabs The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. + dist tag is present + Buildroot is correct + source URL is correct + BR is correct + License used is LGPL + License file COPYING is included + devel package is handled correctly + MD5 sum on tarball is matching upstream tarball 5d22b16cb7e6a117cdf489669821df09 libmtp-0.0.15.tar.gz + No duplicate files OK - Package name OK - Spec file matches base package name. OK - Meets Packaging Guidelines. OK - License (LGPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: 5d22b16cb7e6a117cdf489669821df09 libmtp-0.0.15.tar.gz 5d22b16cb7e6a117cdf489669821df09 libmtp-0.0.15.tar.gz.1 OK - Package compiles and builds on at least one arch. OK - BuildRequires correct OK - Spec has needed ldconfig in post and postun OK - Package owns all the directories it creates. OK - Package has no duplicate files in %files. See below - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Spec has consistant macro usage. OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Headers/static libs in -devel subpackage. OK - .pc files in -devel subpackage. OK - .so files in -devel subpackage. OK - -devel package Requires: %{name} = %{version}-%{release} OK - .la files are removed. OK - Package doesn't own any directories other packages own. See below - No rpmlint output. SHOULD Items: OK - Should include License or ask upstream to include it. OK - Should build in mock. OK - Should have subpackages require base package with fully versioned depend. Issues: 1. 0.0.19 is now available. 2. Might change the defattr from: %defattr(-, root, root) to %defattr(-, root, root,-) 3. rpmlint says: W: libmtp no-documentation W: libmtp-examples no-documentation Can be ignored. W: libmtp mixed-use-of-spaces-and-tabs Should clean up spec to only use tabs or spaces, not both. MUST FIX: * *-devel ships a *.pc => Missing "Requires: pkgconfig" in *-devel BUG: * The *.pc being shipped should "Require: libusb" instead of hard-coding -lusb in Libs. SHOULD: * Use make DESTDIR=... install instead of %makeinstall > * The *.pc being shipped should "Require: libusb"
> instead of hard-coding -lusb in Libs.
It is likely that -lusb is only required for static linking, and if that is
the case (it appears so to me), then a better fix would be to remove -lusb
from Libs: and add:
Libs.private: -lusb
that way not all libmtp-dependant apps (using shared libs) will get needlessly
linked against libusb.
Fixed all the review problems, by changing upstream and by fixing the spec: Spec URL: http://www.df.lth.se/~triad/krad/fc/libmtp.spec SRPM URL: http://www.df.lth.se/~triad/krad/fc/libmtp-0.0.20-1.src.rpm libusb is supposed to be dynamically linked so I have added Requires: libusb to the .pc file and removed -lusb > libusb is supposed to be dynamically linked
I'd argue that
Libs.private: -lusb
should still be used, and both changes should be pushed upstream. This latter
change isn't relevant here, since the static libs aren't being included.
OK I've changed it in upstream, seeing the point. However it's not in Fedora Extras guidelines right now so shouldn't be holding up the review I hope. > OK I've changed it in upstream, seeing the point. Fabulous. > shouldn't be holding up the review I hope nope. Just make Kevin happy. (: All the issues above seem to have been solved. This package is APPROVED. Don't forget to close this bug with NEXTRELEASE once you have imported and built it. ok, this appears to be in owners.list and imported and built. Is anything keeping this review request from being closed now? Yes, sorry for forgetting about this. Imported and build fine. |