Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: pyscard - python module adding smart cards support.
https://bugzilla.redhat.com/show_bug.cgi?id=663102
Summary: Review Request: pyscard - python module adding smart cards support. Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: andrew.elwell@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://dl.dropbox.com/u/6594808/Fedora/pyscard.spec SRPM URL: http://dl.dropbox.com/u/6594808/Fedora/pyscard-1.6.12-1.fc14.src.rpm Description:
1st attempt at packaging for Fedora -- rpmbuild on my F14 box works ok, and I'm running the built rpm to try and get my touchatag reader working with RFIDIOt.
rpmlint on spec: 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint on src.rpm:
pyscard.src: W: spelling-error %description -l en_US smartcard -> smart card, smart-card, smartness 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
upstream uses 'smartcard' in his text.
please note this is my first package and I'm seeking a sponsor
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=663102
Andrew Elwell andrew.elwell@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
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=663102
--- Comment #1 from Andrew Elwell andrew.elwell@gmail.com 2010-12-14 13:31:32 EST --- Created attachment 468665 --> https://bugzilla.redhat.com/attachment.cgi?id=468665 self performed review of spec / package
OK - I did my own self-test review (attached) -- please can a real reviewer point out where I'm incorrect?
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=663102
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |steve.traylen@cern.ch
--- Comment #2 from Steve Traylen steve.traylen@cern.ch 2010-12-15 13:39:47 EST --- Hi Andrew,
This is looking pretty good and thorough for a first package.
Immediate things I notice, you are not compiling with correct compiler options, look at the example CFLAGS settings on the Python guidelines page and
http://fedoraproject.org/wiki/PackagingGuidelines#Compiler_flags
for a more general explanation.
Concerning smartcard vs smart-card just because upstream is using the spelling of smartcard does not mean your .spec file has to... Though in this case smartcard seems to be in pretty common usage to me.
More generally for obtaining sponsorship can continue to follow
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
and submit another package or two and provide some informal reviews of other reviews:
http://fedoraproject.org/PackageReviewStatus/
report back here with links to some informal package review bugs you have done.
When you review head this with "this is an informal review while I try to obtain package sponsorship" or something.
Steve
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=663102
--- Comment #3 from Andrew Elwell andrew.elwell@gmail.com 2010-12-15 19:09:03 EST --- Thanks. Updated srpm / spec are at http://dl.dropbox.com/u/6594808/Fedora/pyscard.spec and http://dl.dropbox.com/u/6594808/Fedora/pyscard-1.6.12-2.fc14.src.rpm
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=663102
--- Comment #4 from Andrew Elwell andrew.elwell@gmail.com 2010-12-16 05:47:14 EST --- informal review by myself of another python package: https://bugzilla.redhat.com/show_bug.cgi?id=611054
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=663102
--- Comment #5 from Andrew Elwell andrew.elwell@gmail.com 2011-01-12 18:01:13 EST --- Some further work: * informal review of bug #668588 (Python26-imaging) * Review Request for libfap (amateur radio APRS parser) in bug #669010
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=663102
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status Whiteboard| |BuildFails
--- Comment #6 from Jason Tibbitts tibbs@math.uh.edu 2011-01-13 22:12:28 EST --- I was going to take a look, but unfortunately the -2 package fails to build for me:
swigging smartcard/scard/scard.i to smartcard/scard/scard_wrap.c swig -python -outdir smartcard/scard -DPCSCLITE -o smartcard/scard/scard_wrap.c smartcard/scard/scard.i unable to execute swig: Permission denied error: command 'swig' failed with exit status 1
A missing dependency on swig, perhaps? You really should always do a koji scratch build or a local mock build to make sure you don't have any problems like this. Here's a scratch build showing the failure:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2720512
Please clear the Whiteboard field if providing a package which builds.
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=663102
Andrew Elwell andrew.elwell@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status Whiteboard|BuildFails |
--- Comment #7 from Andrew Elwell andrew.elwell@gmail.com 2011-01-14 03:22:33 EST --- Ooops yes. Mock build failed. Bumped to -3 and added missing BuildRequires.
$ mock --rebuild ~/rpmbuild/SRPMS/pyscard-1.6.12-3.fc14.src.rpm ... State Changed: build INFO: Done(/home/aelwell/rpmbuild/SRPMS/pyscard-1.6.12-3.fc14.src.rpm) Config(default) 1 minutes 21 seconds INFO: Results and/or logs in: /var/lib/mock/fedora-14-x86_64/result
looks a bit better, thanks.
updated spec and srpm at: http://dl.dropbox.com/u/6594808/Fedora/pyscard.spec http://dl.dropbox.com/u/6594808/Fedora/pyscard-1.6.12-3.fc14.src.rpm
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=663102
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |tibbs@math.uh.edu
--- Comment #8 from Jason Tibbitts tibbs@math.uh.edu 2011-01-19 17:59:25 EST --- I will work through a couple of your submissions, though it may take me a little while.
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=663102
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review?
--- Comment #9 from Jason Tibbitts tibbs@math.uh.edu 2011-01-20 13:24:17 EST --- This is quite a clean package. It builds fine and rpmlint is silent.
You can remove BuildRoot, %clean and the first line of %install. You're obviously not targeting el4 or el5 with this spec (because of the filter stuff) so you shouldn't need those bits either.
I found a few files which do not appear to have the same license.
smartcard/ClassLoader.py taken from the Python Cookbook. The provided URL indicates the "psf" license which I believe we call "Python", but I'm not certain.
smartcard/Observer.py smartcard/Synchronization.py taken from http://mindview.net/Books/TIPython; I didn't see a license at first glance.
smartcard/scard/pyscard-reader.h - I think this is BSD license, but you'll need to chase down the source of the code and verify. In any case, as it is compiled in with LGPL code it shouldn't change the final license but you must still verify that it is licensed and that license is compatible.
* source files match upstream. sha256sum: 0f70f8dd909497183bc1446fe7ea8b4498c7d1516269200f90cd0eec1fa6d630 pyscard-1.6.12.tar.gz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. ? license field matches the actual license. ? license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. * compiler flags are appropriate. * package builds in mock (rawhide, x86_64). * package installs properly. * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: pyscard-1.6.12-3.fc15.x86_64.rpm pyscard = 1.6.12-3.fc15 pyscard(x86-64) = 1.6.12-3.fc15 = pcsc-lite libpython2.7.so.1.0()(64bit) python(abi) = 2.7
* no bundled libraries that I can find. * no shared libraries are added to the regular linker search paths. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files. * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no static libraries. * no libtool .la files.
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=663102
--- Comment #10 from Andrew Elwell andrew.elwell@gmail.com 2011-01-21 15:23:03 EST --- Spec updated to remove buildroot et al. I have posted on upstream issue tracker (https://sourceforge.net/tracker/?group_id=196342&atid=957073 ) to ask if there has been any correspondence between upstream and the authors of those components.
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=663102
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) |
package-review@lists.fedoraproject.org