Bug 193187 - Review Request: pcsc-lite & ccid
Review Request: pcsc-lite & ccid
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: David Cantrell
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FC-ACCEPT 194551 195311
  Show dependency treegraph
 
Reported: 2006-05-25 17:53 EDT by Bob Relyea
Modified: 2013-01-09 20:24 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-17 15:28:32 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Bob Relyea 2006-05-25 17:53:15 EDT
Spec URL: http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pcsc-lite.spec
SRPM URL: http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pcsc-lite-1.3.1-1.src.rpm
Description: 
The purpose of PC/SC Lite is to provide a Windows(R) SCard interface
in a very small form factor for communicating to smartcards and
readers.  PC/SC Lite uses the same winscard API as used under
Windows(R).  This package includes the PC/SC Lite daemon, a resource
manager that coordinates communications with smart card readers and
smart cards that are connected to the system, as well as other command
line tools.

This package is moving from Extras to Core. Changes from the extra version include:
remove dependency on graphviz (verify that rebuild worked without it).
removed %(_dist) flag
moved libpcsclite.so from devel to libs.
make %config %config(noreplace) to make rpmlint happy (oh and it's the right thing to do;).

This package requires the ccid package to run, but not build and they should be reviewed together.

Spec URL: http://directory.fedora.redhat.com/built/rpm_review/rrelyea/ccid.spec
SRPM URL: http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pcsc-lite-1.0.1-1.src.rpm
Description:
Generic USB CCID (Chip/Smart Card Interface Devices) driver.

This package is also moving from extras to core. The only change to this file was removing the %(_dist) macro.
Comment 1 Bob Relyea 2006-05-25 17:55:17 EDT
The SRPM URL for ccid should have been: SRPM URL:
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/ccid-1.0.1-1.src.rpm
Comment 2 Jesse Keating 2006-05-25 18:10:23 EDT
Since it is already in Extras, what is the purpose of bringing it into Core?
Comment 3 Bob Relyea 2006-05-25 19:29:05 EDT
They are needed for the smart card login support for RHEL5.
Comment 4 Chris Ricker 2006-05-25 19:53:36 EDT
It being needed for RHEL has nothing to do with whether it should be in Fedora
Core or not, though.... Is there a plan to add smart card login support to
Fedora also, or just to RHEL?
Comment 5 Bob Relyea 2006-05-25 20:00:01 EDT
Yes. You will see smart card login support in fedora before you see it in RHEL.
Comment 6 Jesse Keating 2006-05-26 13:00:21 EDT
This still doesn't mean it has to be in Core.  Since it is already in extras,
lets skip to the step of getting approval for core by the technical lead.

Bill?
Comment 7 Ville Skyttä 2006-05-26 13:53:57 EDT
By the way, if you're interested in maintaining these packages in Extras, that
would be fine with me too.  Some comments/notes anyway:

> SRPM URL: [...] pcsc-lite-1.3.1-1.src.rpm

Ick, this would not upgrade the version currently in Extras.  Please bump the
release tag.

> This package is moving from Extras to Core. Changes from the extra version 
> include:

These should be mentioned in the package's %changelog, otherwise it'll look like
I'm responsible of the changes (some of which I consider regressions):

> moved libpcsclite.so from devel to libs.

Why?

> make %config %config(noreplace) to make rpmlint happy (oh and it's the right 
> thing to do;).

Partially disagreed.  The FE package has /etc/reader.conf marked as %ghost
%config.  Actually the right thing to do would be to remove the %config
altogether; this file is not something that anyone should go and modify, it's
deleted and recreated from the contents of /etc/reader.conf.d/*.conf on every
pcscd restart.  One could also argue that a better place for the generated file
would be somewhere in /var instead of /etc/reader.conf.

Additionally, I see the new package marks /etc/reader.conf.d/README a
%config(noreplace) file, which doesn't make sense to me.

One other thing: the build dependency on graphviz was removed, but the HAVE_DOT
= yes modification for doxygen's config is still in, that should probably be
removed.

> Generic USB CCID (Chip/Smart Card Interface Devices) driver.
> This package is also moving from extras to core. The only change to this file 
> was removing the %(_dist) macro.

Same problem here wrt. upgrading from the current FE version, please bump the
release tag.
Comment 8 Bill Nottingham 2006-05-26 15:36:53 EDT
(In reply to comment #6)
> This still doesn't mean it has to be in Core.  Since it is already in extras,
> lets skip to the step of getting approval for core by the technical lead.

Well, if you want smart card login support, I suspect that means some portion of
pam/gdm/etc is going to grow a direct dependency. If that's the case, it's fine
with me. Login support generally seems like a Core feature

Curious why .so moved from devel->lib.

Why regenerate the config file on startup? Aside from requiring write access
(/var/run/pcsc?), it seems wasteful. Can't it just read /etc/reader.conf.d/
directly?
Comment 9 Ville Skyttä 2006-05-26 16:48:04 EDT
Reading /etc/reader.conf.d/*.conf directly would be better, yes.  The current
implementation was submitted to me in PM (see Jul 31 2004 %changelog entry) and
later sent to and adopted by upstream almost as-is.  I bet upstream wouldn't
mind a patch that makes pcscd take care of things itself.
Comment 10 Bob Relyea 2006-05-26 19:09:52 EDT
I'll update the release number.

The 'noreplace' was just blindly following the Fedora package standards. I have
no problem reverting it. Obviously the README is not part of the actual config.
The whole /etc/reader.conf.d is really only used for serial readers (and other
non-hot pluggable readers), which are getting more an more rare. I have no
problem reverting that.

Is /usr/sbin/update-reader.conf really run every time you start pcscd, or just
when an appropriate reader package installs itself?

The move of libpcsclite.so is for some pkcs #11 modules which dlopen libpcsclite.so.

One other question Ville, why does pcsc-lite depend on ccid. pcsc-lite isn't
useful without some reader, but it does start without one, and one can envision
systems which have some other reader. It's not a big deal since most modern
readers are ccid readers, so ccid support should be part of the base anyway. I
didn't want to change that at this point without knowing the reason for it.
Comment 11 Bill Nottingham 2006-05-26 22:50:52 EDT
So, they're dlopening the unversioned .so symlink, and hoping it's the same ABI
they're built against? That seems fragile.
Comment 12 Ville Skyttä 2006-05-27 04:55:35 EDT
Yes, /usr/sbin/update-reader.conf is being run on every pcscd start, see start()
in /etc/init.d/pcscd.

pcsc-lite does not depend on ccid specifically.  It depends on pcsc-ifd-handler,
which is a "virtual" dependency, provided by packages that contain suitable
drivers.  In addition to ccid, in Extras it is provided by pcsc-lite-openct and
(should be provided) by ctapi-cyberjack-pcsc, as well as number of other, mainly
proprietary, driver packages elsewhere.
Comment 13 Jesse Keating 2006-05-27 12:56:16 EDT
A conf file in /usr/sbin just seems a bit um... not cool.  Is there any way to
put this somewhere else?  /etc/sysconfig/ ? /etc/ ?  /usr/share/ ?  Somewhere
that isn't /usr/sbin...
Comment 14 Ville Skyttä 2006-05-27 14:15:00 EDT
/usr/sbin/update-reader.conf is not a config file, it's a shell script which
creates /etc/reader.conf from  the contents of /etc/reader.conf.d/*.conf.  Yes,
I did nag to upstream about the potentially confusing name of the script...
Comment 15 Bob Relyea 2006-06-05 21:51:35 EDT
I've uploaded new packages:

Spec URL: http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pcsc-lite.spec
SRPM URL:
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pcsc-lite-1.3.1-2.src.rpm

Spec URL: http://directory.fedora.redhat.com/built/rpm_review/rrelyea/ccid.spec
SRPM URL:
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/ccid-1.0.1-2.src.rpm

Changes include:
- reverting the changes to the reader.conf.
- reverting the shared library change. pcsc-lite is supposed to be a pretty
stable API, but I'll leave the versioning in until I can verify with upstream
whether or not binary API continuity is part of the package requirements.
- incremented the version number with the appropriate comment.
- removed the sed command which turned on HAVE_DOT in the doxygen config file.
Comment 16 Bob Relyea 2006-06-07 15:10:33 EDT
I guess I should have said, please review these updates?
There are several packages that depend on these that are waiting in the pipeline.

bob
Comment 17 Ville Skyttä 2006-06-07 16:56:50 EDT
pcsc-lite compared to the latest FE package:
- libpcsclite.pc has moved from -devel to -libs.  That looks like a bug to me in
  the first place, and especially so because -devel still has a dependency on
  pkgconfig, and -libs not.
- %{_sysconfdir}/reader.conf.d/README is %ghost %config; I think both of those
  should be dropped and the file included without any special %foo's.  Or maybe
  make it %doc.
- %{_sysconfdir}/reader.conf is no longer %ghost; I mentioned removing %config
  in comment 7, but was %ghost removed for some specific reason too?

ccid compared to the latest FE package:
- Careful, macros are expanded in %changelog too.  Escape it like %%{?dist}
  (with two '%'s) or just say "disttag".
Comment 18 Bob Relyea 2006-06-08 18:32:35 EDT
Thanks for the quick review Ville.
I've now updated the packages (and removed some bottlenecks so I can turn them
around faster).

here's the diff between the CORE proposed packages and the current EXTRAS to aid
the review the only question I have is the %doc for reader.conf.d/README
correct, or does the %doc do some magic I'm not aware of:

--- /usr/src/redhat/SPECS/pcsc-lite.spec        2006-04-22 13:32:58.000000000 -0700
+++ ./pcsc-lite.spec    2006-06-08 09:02:14.253045000 -0700
@@ -1,6 +1,6 @@
 Name:           pcsc-lite
 Version:        1.3.1
-Release:        1%{?dist}
+Release:        2
 Summary:        PC/SC Lite smart card framework and applications

 Group:          System Environment/Daemons
@@ -13,7 +13,6 @@

 BuildRequires:  libusb-devel >= 0.1.7
 BuildRequires:  doxygen
-BuildRequires:  graphviz
 Requires(post): /sbin/chkconfig
 Requires(preun): /sbin/chkconfig
 Requires:       pcsc-ifd-handler
@@ -57,8 +56,6 @@
 %setup -q
 %patch0 -p0
 %patch1 -p1
-sed -i -e 's/^\(HAVE_DOT\s*=\s*\)/\1 YES/' doc/doxygen.conf.in
-

 %build
 %configure \
@@ -114,7 +111,7 @@
 %defattr(-,root,root,-)
 %doc AUTHORS ChangeLog* COPYING DRIVERS HELP README SECURITY TODO
 %dir %{_sysconfdir}/reader.conf.d/
-%{_sysconfdir}/reader.conf.d/README
+%doc %{_sysconfdir}/reader.conf.d/README
 %ghost %config %{_sysconfdir}/reader.conf
 %{_initrddir}/pcscd
 %{_bindir}/formaticc
@@ -142,6 +139,11 @@


 %changelog
+* Mon Jun 5 2006 Bob Relyea <rrelyea@redhat.com> - 1.3.1-2
+- Move to Fedora Core.
+- Remove dependency on graphviz.
+- Removed %%{_dist}
+
 * Sat Apr 22 2006 Ville Skyttä <ville.skytta at iki.fi> - 1.3.1-1
 - 1.3.1.

3c3
< Release:        2
---
> Release:        1%{?dist}
15a16
> BuildRequires:  graphviz
58a60,61
> sed -i -e 's/^\(HAVE_DOT\s*=\s*\)/\1 YES/' doc/doxygen.conf.in
>
114c117
< %doc %{_sysconfdir}/reader.conf.d/README
---
> %{_sysconfdir}/reader.conf.d/README
142,146d144
< * Mon Jun 5 2006 Bob Relyea <rrelyea@redhat.com> - 1.3.1-2
< - Move to Fedora Core.
< - Remove dependency on graphviz.
< - Removed %%{_dist}
<


--------------------------------
--- /usr/src/redhat/SPECS/ccid.spec     2006-04-22 13:33:57.000000000 -0700
+++ ./ccid.spec 2006-06-08 09:06:19.497820000 -0700
@@ -2,7 +2,7 @@

 Name:           ccid
 Version:        1.0.1
-Release:        1%{?dist}
+Release:        2
 Summary:        Generic USB CCID smart card reader driver

 Group:          System Environment/Libraries
@@ -61,6 +61,9 @@


 %changelog
+* Mon Jun  5 2006 Bob Relyea <rrelyea@redhat.com> - 1.0.1-2
+- Move to Fedora Core, removed %%{_dist}.
+
 * Sat Apr 22 2006 Ville Skyttä <ville.skytta at iki.fi> - 1.0.1-1
 - 1.0.1.
Comment 20 Bob Relyea 2006-06-14 15:00:41 EDT
Here's the full view

Smart Card Login Packages:

Base:
pam_pkcs11      (default)
coolkey         (default) depends on pcsc-lite-libs ifd-egate
ccid            (default) depends on pcsc-lite
ifd-egate       (default) depends on pcsc-lite
pcsc-lite-libs  (default)
pcsc-lite       (default)

Developement Libraries
pcsc-lite-devel depends on pcsc-lite-libs
coolkey-devel   depends on coolkey
Comment 21 Jesse Keating 2006-06-26 13:07:57 EDT
Added to dist-fc6, waiting for build into rawhide.
Comment 22 Jesse Keating 2006-07-17 15:28:32 EDT
Built into rawhide.

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