Bug 643199 - Review Request: python-pymtp - A Pythonic wrapper around libmtp
Summary: Review Request: python-pymtp - A Pythonic wrapper around libmtp
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 628230 794971
TreeView+ depends on / blocked
 
Reported: 2010-10-14 21:47 UTC by Ville-Pekka Vainio
Modified: 2014-01-21 13:53 UTC (History)
5 users (show)

Fixed In Version: python-pymtp-0.0.4-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-02-24 10:32:43 UTC
lemenkov: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Ville-Pekka Vainio 2010-10-14 21:47:47 UTC
Spec URL: http://vpv.fedorapeople.org/packages/python-pymtp.spec
SRPM URL: http://vpv.fedorapeople.org/packages/python-pymtp-0.0.4-0.1.fc13.src.rpm
Description: A Pythonic wrapper around libmtp, allowing python applications to communicate with MTP devices.

Rpmlint only returns false spelling warnings.

A couple of things I'd like to be discussed in the review:
- This package is useless without libmtp being installed on the system, but just adding "Requires: libmtp" would technically be against the packaging guidelines. If that Requires is added anyway, will it always resolve to the correct architecture (32/64 bits)?
- This package is intended as a dependency for gPodder and includes a patch with libmtp compatibility changes done by the gPodder developers. Should there be a separate README.Fedora file pointing that out?

Comment 1 Ville-Pekka Vainio 2010-10-14 22:18:45 UTC
To answer my own question, based on discussions on #fedora-devel, depending on "libmtp" is probably ok. I could still probably test that the package works, if only an i686 version of libmtp is installed on an x86_64 system.

I looked into what Debian is doing with pymtp. They are shipping the same patch against pymtp.py as this package, but they have also patched the examples. I should probably just switch to using their patch, which is available at http://patch-tracker.debian.org/patch/series/view/pymtp/0.0.4-2/adapt-to-libmtp8.diff

Comment 2 Ville-Pekka Vainio 2010-10-15 10:24:11 UTC
Spec updated
New SRPM: http://vpv.fedorapeople.org/packages/python-pymtp-0.0.4-0.2.fc13.src.rpm

- Use Debian's patch, which also fixes the examples
- Add Requires: libmtp

rpmlint output:
python-pymtp.noarch: E: explicit-lib-dependency libmtp
python-pymtp.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/pymtp.py 0644L /usr/bin/env

The explicit dependency is necessary, if we want to make sure this package works when libmtp has not previously been installed. The module does not need to be executable, if __name__ == "__main__" is not used, the module needs to be imported in order for it to be used.

Comment 3 Ville-Pekka Vainio 2010-10-21 14:48:13 UTC
I tested this package on an x86_64 system with libmtp.i686 installed and it did not work. I see two options here. Either I have this package depending on "libmtp" (like it is now) and hope yum installs the correct arch version of libmtp and people don't already have the "wrong" version installed or I make this package arch-dependent. Opinions?

Comment 4 Ville-Pekka Vainio 2010-11-02 13:21:42 UTC
Spec updated
New SRPM: http://vpv.fedorapeople.org/packages/python-pymtp-0.0.4-0.3.fc14.src.rpm

I've made the package arch-specific based on discussions on the packaging list: http://lists.fedoraproject.org/pipermail/packaging/2010-October/007451.html
Now the package uses the following "trick" to depend on the arch-specific version of libmtp:

%global libmtp libmtp.so.8
%ifarch x86_64 ppc64 ia64 sparc64 s390x
%global mark64 ()(64bit)
%endif

Requires:       %{libmtp}%{?mark64}

I think this package is finally ready to be reviewed.

Comment 5 Peter Lemenkov 2011-01-12 15:03:49 UTC
I'll review it

Comment 6 Peter Lemenkov 2011-01-12 15:28:04 UTC
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is not silent, but all its messages can be safely ignored:

work ~: rpmlint Desktop/python-pymtp-0.0.4-0.3.fc15.*
python-pymtp.src: W: spelling-error Summary(en_US) libmtp -> libation, Liberty, librate
python-pymtp.src: W: spelling-error %description -l en_US libmtp -> libation, Liberty, librate
python-pymtp.x86_64: W: spelling-error Summary(en_US) libmtp -> libation, Liberty, librate
python-pymtp.x86_64: W: spelling-error %description -l en_US libmtp -> libation, Liberty, librate

^^^ These are false positives.

python-pymtp.x86_64: E: no-binary

^^^ This package contains only arch-independent data, but does depends on arch-dependent stuff. So we can't mark it as noarch.

2 packages and 0 specfiles checked; 1 errors, 4 warnings.
work ~: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines. The situation with ugly dependency on libmtp.so.8 is explained in the comments above.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv3 or later).
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum pymtp-latest.tar.bz2*
b60d18ffa107a3e2a50a259123f51d81cd097a21e974f12dae84b3215a535f8b  pymtp-latest.tar.bz2
b60d18ffa107a3e2a50a259123f51d81cd097a21e974f12dae84b3215a535f8b  pymtp-latest.tar.bz2.1
sulaco ~/rpmbuild/SOURCES: 

Unfortunately, no versioned sources provided by upstream.

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. Koji scratchbuild for Rawhide:

http://koji.fedoraproject.org/koji/taskinfo?taskID=2716996

+ All build dependencies are listed in BuildRequires.

0 No need to handle locales.
0 No shared library files in some of the dynamic linker's default paths.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files without a suffix (e.g. libfoo.so).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

I can't find any issues here, so this package 

APPROVED.



p.s. I would like you to review this package in return:

https://bugzilla.redhat.com/show_bug.cgi?id=652623

Comment 7 Ville-Pekka Vainio 2011-01-12 15:52:17 UTC
Thanks, Peter! There are a couple of gPodder people on the CC list, if you want to be co-maintainers, I could add you. Right now I'll just make myself as the owner.

New Package SCM Request
=======================
Package Name: python-pymtp
Short Description: A Pythonic wrapper around libmtp
Owners: vpv
Branches: f13 f14
InitialCC:

Comment 8 Jason Tibbitts 2011-01-13 17:24:39 UTC
Git done (by process-git-requests).

Comment 9 Christopher Meng 2013-12-31 04:38:51 UTC
Package Change Request
======================
Package Name: python-pymtp
New Branches: el6
Owners: cicku

Comment 10 Jens Petersen 2014-01-02 11:33:00 UTC
Can the Fedora owner please ack for epel?

Comment 11 Christopher Meng 2014-01-02 15:38:01 UTC
This package is not either in RHEL @base or in EPEL6, so why can't I do that?

Package Change Request
======================
Package Name: python-pymtp
New Branches: el6
Owners: cicku

Comment 12 Gwyn Ciesla 2014-01-02 15:40:28 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2014-01-03 03:27:41 UTC
python-pymtp-0.0.4-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/python-pymtp-0.0.4-1.el6

Comment 14 Fedora Update System 2014-01-19 19:05:29 UTC
python-pymtp-0.0.4-1.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 15 Christopher Meng 2014-01-20 03:37:23 UTC
Thanks Jon! Here again... -((* ^_^ *))-

Package Change Request
======================
Package Name: python-pymtp
New Branches: epel7
Owners: cicku

Comment 16 Gwyn Ciesla 2014-01-21 13:53:48 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.