Bug 811330 - Review Request: pcsc-cyberjack - driver for ReinerSCT cyberJack chipcart readers
Review Request: pcsc-cyberjack - driver for ReinerSCT cyberJack chipcart readers
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ngo Than
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks: 901764
  Show dependency treegraph
 
Reported: 2012-04-10 14:02 EDT by Patrick C. F. Ernzer
Modified: 2015-12-03 23:07 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-03 23:07:06 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
than: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Patrick C. F. Ernzer 2012-04-10 14:02:42 EDT
Spec URL: http://pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack.spec
SRPM URL: http://pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack-3.99.5final.SP03-5.fc16.src.rpm
Description: 
This is the current driver for ReinerSCT chipcard readers

the vendor switched from ctapi to pcsc (dropping ctapi support completely as far as I can tell) and this causes some confusion, I think I got everything right but this package definitely needs more eyeballs.

Until fox is in Fedora (and I'm not sure the license exceptions will make this possible), the package needs building 'withGUI 0'. AFAICT the GUI components are just a nice to have but not a functional requirement.

I only use my reader very sparingly, in bug 767657 another user reported partial success.
Comment 1 Patrick C. F. Ernzer 2012-06-01 06:56:31 EDT
needed a small patch for F17 (#include <unistd.h> for cm_distri.cpp kudos to karsten)

new Spec URL: http://pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack-3.99.5final.SP03-6.fc17.src.rpm
Comment 3 Patrick C. F. Ernzer 2012-10-14 12:17:43 EDT
uploaded -8

http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack.spec
http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack-3.99.5final.SP03-8.fc17.src.rpm

now using systemd to do what 'service pcscd condrestart' did before

Than: anything else I should look at on the spec file front?
Comment 8 Ngo Than 2012-10-22 10:46:24 EDT
rpmlint outputs:

pcsc-cyberjack.src: W: spelling-error Summary(en_US) chipcard -> chip card, chip-card, chipboard
pcsc-cyberjack.src: W: spelling-error %description -l en_US chipcard -> chip card, chip-card, chipboard
pcsc-cyberjack.src: W: spelling-error %description -l en_US contactless -> con tactless, con-tactless, contact less
pcsc-cyberjack.x86_64: W: spelling-error Summary(en_US) chipcard -> chip card, chip-card, chipboard
pcsc-cyberjack.x86_64: W: spelling-error %description -l en_US chipcard -> chip card, chip-card, chipboard
pcsc-cyberjack.x86_64: W: spelling-error %description -l en_US contactless -> con tactless, con-tactless, contact less

 * i'm not sure if the descriptions are correct in en_US. Patrick, could you please check again?

pcsc-cyberjack.x86_64: W: conffile-without-noreplace-flag /etc/udev/rules.d/92-cyberjack.rules

 * please add %config(noreplace)

pcsc-cyberjack.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pcsc/drivers/libifd-cyberjack.bundle/Contents/Linux/libifd-cyberjack.so

 * it should be included in devel-package, or just remove it if there's no devel package

pcsc-cyberjack-cjflash.x86_64: W: no-documentation
pcsc-cyberjack-cjflash.x86_64: W: no-manual-page-for-binary cjflash

 * your package doesn't have man pages, please ask upstream to add man page in the future, it's only should fix.

pcsc-cyberjack-examples.x86_64: E: incorrect-fsf-address /usr/share/doc/pcsc-cyberjack-examples-3.99.5final.SP03/verifypin_ascii.c
pcsc-cyberjack-examples.x86_64: E: incorrect-fsf-address /usr/share/doc/pcsc-cyberjack-examples-3.99.5final.SP03/verifypin_fpin2.c
4 packages and 0 specfiles checked; 2 errors, 10 warnings

 * the Free Software Foundation address in this file seems to be outdated or
   misspelled.  Ask upstream to update the address, or if this is a license file,
   possibly the entire file with a new copy available from the FSF.
Comment 9 Patrick C. F. Ernzer 2012-10-22 13:05:01 EDT
Than,

thanks. Much appreciated. Seems rpmlint on my build box is way less strict than it should be.

(In reply to comment #8)
> rpmlint outputs:
[...]
> pcsc-cyberjack.src: W: spelling-error Summary(en_US) chipcard -> chip card,
> chip-card, chipboard

the next release of the spec file will use 'chip card'

[...]
> pcsc-cyberjack.src: W: spelling-error %description -l en_US contactless ->
> con tactless, con-tactless, contact less

after a dictionary lookup, I decided to use 'non-contact' for the next release

[...]
>  * i'm not sure if the descriptions are correct in en_US. Patrick, could you
> please check again?

You're right, they did need fixing.
> 
> pcsc-cyberjack.x86_64: W: conffile-without-noreplace-flag
> /etc/udev/rules.d/92-cyberjack.rules
> 
>  * please add %config(noreplace)

done
> 
> pcsc-cyberjack.x86_64: W: devel-file-in-non-devel-package
> /usr/lib64/pcsc/drivers/libifd-cyberjack.bundle/Contents/Linux/libifd-
> cyberjack.so
> 
>  * it should be included in devel-package, or just remove it if there's no
> devel package

This one I am not sure about, I _think_ the .so needs to remain in that location (compare pcsc-lite-ccid). But it will need someone more competent in PC/SC than me to confirm.

> pcsc-cyberjack-cjflash.x86_64: W: no-documentation
> pcsc-cyberjack-cjflash.x86_64: W: no-manual-page-for-binary cjflash
> 
>  * your package doesn't have man pages, please ask upstream to add man page
> in the future, it's only should fix.

I've asked upstream via their web form.

there is a small section about cjflash in the README.* files of the main package. As this sub-package depends on the main one, should I maybe add a README.Fedora to the sub-package that points to the main's README?

> pcsc-cyberjack-examples.x86_64: E: incorrect-fsf-address
> /usr/share/doc/pcsc-cyberjack-examples-3.99.5final.SP03/verifypin_ascii.c
> pcsc-cyberjack-examples.x86_64: E: incorrect-fsf-address
> /usr/share/doc/pcsc-cyberjack-examples-3.99.5final.SP03/verifypin_fpin2.c
> 4 packages and 0 specfiles checked; 2 errors, 10 warnings
> 
>  * the Free Software Foundation address in this file seems to be outdated or
[...]

Also asked this from upstream.

Do I remember correctly that I am NOT to patch these two files until upstream released an updated version or is it OK for me to patch the address in this version and then revert my patch once upstream fixed it?
Comment 10 Patrick C. F. Ernzer 2012-10-22 13:23:48 EDT
FWIW: those things I already did fix are now uploaded.
spec file at 
 http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack.spec

and packages in
 http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack-3.99.5final.SP03-9/
Comment 11 Kalev Lember 2012-10-22 13:29:51 EDT
(In reply to comment #9)
> > pcsc-cyberjack.src: W: spelling-error %description -l en_US contactless ->
> > con tactless, con-tactless, contact less
> 
> after a dictionary lookup, I decided to use 'non-contact' for the next
> release

The dictionary that rpmlint uses obviously just doesn't know the technical term. No need to fix every single rpmlint warning; we don't have to make all packages rpmlint clean. This warning is rpmlint saying "Hey, I've noticed a possible issue with your package, please check if this needs fixing."


> > pcsc-cyberjack.x86_64: W: conffile-without-noreplace-flag
> > /etc/udev/rules.d/92-cyberjack.rules
> > 
> >  * please add %config(noreplace)

I think this is wrong in two ways:
 a) the file isn't really meant to be modified by the user, and as such shouldn't be marked %config(noreplace);
 b) it should be installed in /usr/lib/udev/rules.d/ instead so that it's clear that it's a system file shipped by a package and not a config file that can be modified.

So in this case rpmlint caught a valid issue but the fix isn't adding %config(noreplace), but instead moving the file to another directory.


> > pcsc-cyberjack.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/pcsc/drivers/libifd-cyberjack.bundle/Contents/Linux/libifd-
> > cyberjack.so
> > 
> >  * it should be included in devel-package, or just remove it if there's no
> > devel package
> 
> This one I am not sure about, I _think_ the .so needs to remain in that
> location (compare pcsc-lite-ccid). But it will need someone more competent
> in PC/SC than me to confirm.

Yes. pcscd dlopens the .so file directly and it's needed for proper functioning. This .so file is a pcscd plugin. Another case of rpmlint warning about a possible issue but where we know better.


> >  * the Free Software Foundation address in this file seems to be outdated or
> [...]
> 
> Also asked this from upstream.
> 
> Do I remember correctly that I am NOT to patch these two files until
> upstream released an updated version or is it OK for me to patch the address
> in this version and then revert my patch once upstream fixed it?

Yes, never patch any license files. This is for upstream to change, not something a downstream packager should change. Notifying upstream and possibly sending them a patch that fixes up the license headers is the correct thing to do here.

See also https://bugzilla.redhat.com/show_bug.cgi?id=700095
Comment 12 Patrick C. F. Ernzer 2012-10-28 11:34:11 EDT
(In reply to comment #11)
> (In reply to comment #9)
[...]
> > after a dictionary lookup, I decided to use 'non-contact' for the next
> > release
> 
> The dictionary that rpmlint uses obviously just doesn't know the technical
> term.

Ah, I meant 'I looked in the dictionary at leo.org and found non-contact better'. Merriam-Webster also agrees that contactless does not exist. I presume 'contactless' is a straight translation of the German 'kontaktlos'.

[...]
>  a) the file isn't really meant to be modified by the user, and as such
> shouldn't be marked %config(noreplace);
>  b) it should be installed in /usr/lib/udev/rules.d/ instead so that it's
> clear that it's a system file shipped by a package and not a config file
> that can be modified.
[...]

Agreed and moved in -10

spec file at 
 http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack.spec

and mock build results in
  http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack-3.99.5final.SP03-9/fedora-17-x86_64/
and
  http://www.pcfe.net/pcsc-cyberjack-3.99.5final.SP03/pcsc-cyberjack-3.99.5final.SP03-9/fedora-18-x86_64/

Still unlear to me:
pcsc-cyberjack-cjflash has no docs; should I maybe add a README.Fedora to the cjflash sub-package that points to the main package's README?

PCFE
Comment 14 Ngo Than 2013-01-02 08:24:50 EST
> Still unlear to me:
> pcsc-cyberjack-cjflash has no docs; should I maybe add a README.Fedora to
> the cjflash sub-package that points to the main package's README?
>

it's not needed in this case.
Comment 15 Ngo Than 2013-01-02 08:44:30 EST
I don't see any further blockers, all looks fine now. This package is APPROVED. 

Don't forget to close this bug with NEXTRELEASE once you have imported and built
the package.
Comment 16 Patrick C. F. Ernzer 2013-01-02 11:09:23 EST
New Package SCM Request
=======================
Package Name: pcsc-cyberjack
Short Description: PC/SC driver for REINER SCT cyberjack USB chip card reader
Owners: pcfe
Branches: f17 f18
InitialCC:
Comment 17 Gwyn Ciesla 2013-01-02 11:25:58 EST
Git done (by process-git-requests).
Comment 18 Patrick C. F. Ernzer 2013-01-02 13:01:58 EST
package has been built for rawhide, f17-candidate and and f18-candidate
http://koji.fedoraproject.org/koji/packageinfo?packageID=15223
Comment 19 Patrick C. F. Ernzer 2013-01-21 04:27:51 EST
Package Change Request
======================
Package Name: pcsc-cyberjack
New Branches: el6
Owners: pcfe
InitialCC: 

In Bug 901764 a build for el6 was requested and a spec file patch submitted.
As expected, the package builds fine with "mock -r epel-6-x86_64 rebuild ..."
Comment 20 Robert Scheck 2013-02-23 12:20:19 EST
This Package Change Request is lacking fedora-cvs? no SCM action will ever
happen while this is still staying on fedora-cvs+.
Comment 21 Gwyn Ciesla 2013-03-14 11:47:52 EDT
Git done (by process-git-requests).
Comment 22 Patrick C. F. Ernzer 2014-06-29 12:16:39 EDT
Package Change Request
======================
Package Name: pcsc-cyberjack
New Branches: epel7
Owners: pcfe
Comment 23 Gwyn Ciesla 2014-06-30 08:04:42 EDT
Git done (by process-git-requests).
Comment 24 James Hogarth 2015-12-03 23:07:06 EST
Reclosing ticket in an effort to clean up the queue

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