[bug#32358] Add pcscd service

[bug#32358] Add pcscd service

[Arun Isaac]

[-- Attachment #1: Type: text/plain, Size: 128 bytes --]


This patchset adds pcscd service. It is required for gpg to detect and
use security tokens such as the Nitrokey and the Gnuk.


[-- Attachment #2: 0001-gnu-pcsc-lite-Link-to-USB-drivers-from-ccid.patch --]
[-- Type: text/x-patch, Size: 3453 bytes --]

From 3fe4571e27afcecd5cfb72059ece22969fd6f72a Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Thu, 2 Aug 2018 05:24:37 +0530
Subject: [PATCH 1/2] gnu: pcsc-lite: Link to USB drivers from ccid.

* gnu/packages/security-token.scm (pcsc-lite-bootstrap): New variable.
(ccid)[inputs]: Remove pcsc-lite.
[native-inputs]: Add pcsc-lite-bootstrap.
(pcsc-lite)[inputs]: Add ccid.
[arguments]: Set --enable-usbdropdir configure flag, linking to USB drivers
from ccid.
---
 gnu/packages/security-token.scm | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/security-token.scm b/gnu/packages/security-token.scm
index 7fdcaaf1e..424f4626e 100644
--- a/gnu/packages/security-token.scm
+++ b/gnu/packages/security-token.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2017, 2018 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -29,6 +30,7 @@
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix git-download)
+  #:use-module (guix utils)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system glib-or-gtk)
   #:use-module (gnu packages autotools)
@@ -75,11 +77,11 @@
                (("/bin/echo") (which "echo")))
              #t)))))
     (native-inputs
-     `(("perl" ,perl)
+     `(("pcsc-lite-bootstrap" ,pcsc-lite-bootstrap) ; only required for headers
+       ("perl" ,perl)
        ("pkg-config" ,pkg-config)))
     (inputs
-     `(("libusb" ,libusb)
-       ("pcsc-lite" ,pcsc-lite)))
+     `(("libusb" ,libusb)))
     (home-page "https://ccid.apdu.fr/")
     (synopsis "PC/SC driver for USB smart card devices")
     (description
@@ -169,13 +171,16 @@ the low-level development kit for the Yubico YubiKey authentication device.")
                 "1jc9ws5ra6v3plwraqixin0w0wfxj64drahrbkyrrwzghqjjc9ss"))))
     (build-system gnu-build-system)
     (arguments
-     `(#:configure-flags '("--enable-usbdropdir=/var/lib/pcsc/drivers"
-                           "--disable-libsystemd")))
+     `(#:configure-flags
+       (list (string-append "--enable-usbdropdir="
+                            (assoc-ref %build-inputs "ccid") "/pcsc/drivers")
+             "--disable-libsystemd")))
     (native-inputs
      `(("perl" ,perl)                   ; for pod2man
        ("pkg-config" ,pkg-config)))
     (inputs
-     `(("libudev" ,eudev)))
+     `(("ccid" ,ccid)
+       ("libudev" ,eudev)))
     (home-page "https://pcsclite.apdu.fr/")
     (synopsis "Middleware to access a smart card using PC/SC")
     (description
@@ -186,6 +191,16 @@ from a client application and provide access to the desired reader.")
                    license:isc                  ; src/strlcat.c src/strlcpy.c
                    license:gpl3+))))            ; src/spy/*
 
+(define pcsc-lite-bootstrap
+  (package
+    (inherit pcsc-lite)
+    (name "pcsc-lite-bootstrap")
+    (inputs
+     `(("libudev" ,eudev)))
+    (arguments
+     (substitute-keyword-arguments (package-arguments pcsc-lite)
+       ((#:configure-flags _) '(list "--disable-libsystemd"))))))
+
 (define-public ykclient
   (package
     (name "ykclient")
-- 
2.18.0


[-- Attachment #3: 0002-gnu-services-Add-pcscd-service.patch --]
[-- Type: text/x-patch, Size: 4960 bytes --]

From d565f247fc34680bf39c2618ea0ff05c229c2b54 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Thu, 2 Aug 2018 05:32:56 +0530
Subject: [PATCH 2/2] gnu: services: Add pcscd service.

* gnu/services/security-token.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
* doc/guix.texi (Miscellaneous Services): Document the service.
---
 doc/guix.texi                   | 28 +++++++++++++++
 gnu/local.mk                    |  1 +
 gnu/services/security-token.scm | 63 +++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 gnu/services/security-token.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index 080b091b3..7e5212a66 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20326,6 +20326,34 @@ An association list specifies kernel parameters and their values.
 @end table
 @end deftp
 
+@cindex pcscd
+@subsubheading PC/SC Smart Card Daemon Service
+
+The @code{(gnu services security-token)} module provides the following service
+to run @command{pcscd}, the PC/SC Smart Card Daemon.  @command{pcscd} is the
+daemon program for pcsc-lite and the MuscleCard framework. It is a resource
+manager that coordinates communications with smart card readers, smart cards
+and cryptographic tokens that are connected to the system.
+
+@defvr {Scheme Variable} pcscd-service-type
+Service type for the @command{pcscd} service.  Its value must be a
+@code{pcscd-configuration} object.  To run pcscd in the default
+configuration, instantiate it as:
+
+@example
+(service pcscd-service-type)
+@end example
+@end defvr
+
+@deftp {Data Type} pcscd-configuration
+The data type representing the configuration of @command{pcscd}.
+
+@table @asis
+@item @code{pcsc-lite} (default: @code{pcsc-lite})
+The pcsc-lite package that provides pcscd.
+@end table
+@end deftp
+
 @cindex lirc
 @subsubheading Lirc Service
 
diff --git a/gnu/local.mk b/gnu/local.mk
index d1f9a193b..c637f0954 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -485,6 +485,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/monitoring.scm			\
   %D%/services/networking.scm			\
   %D%/services/nfs.scm			\
+  %D%/services/security-token.scm		\
   %D%/services/shepherd.scm			\
   %D%/services/sound.scm			\
   %D%/services/herd.scm				\
diff --git a/gnu/services/security-token.scm b/gnu/services/security-token.scm
new file mode 100644
index 000000000..888c92fb1
--- /dev/null
+++ b/gnu/services/security-token.scm
@@ -0,0 +1,63 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services security-token)
+  #:use-module (gnu services)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu packages admin)
+  #:use-module (gnu packages security-token)
+  #:use-module (gnu system shadow)
+  #:use-module (guix gexp)
+  #:use-module (guix modules)
+  #:use-module (guix records)
+  #:use-module (ice-9 match)
+  #:export (pcscd-configuration
+            pcscd-configuration?
+            pcscd-service-type))
+
+;;;
+;;; PC/SC Smart Card Daemon
+;;;
+
+(define-record-type* <pcscd-configuration>
+  pcscd-configuration make-pcscd-configuration pcscd-configuration?
+  (pcsc-lite pcscd-configuration-package
+             (default pcsc-lite)))
+
+(define pcscd-shepherd-service
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite)
+     (with-imported-modules (source-module-closure
+                             '((gnu build shepherd)))
+       (shepherd-service
+        (documentation "PC/SC Smart Card Daemon")
+        (provision '(pcscd))
+        (modules '((gnu build shepherd)))
+        (start #~(make-forkexec-constructor
+                  (list #$(file-append pcsc-lite "/sbin/pcscd") "-f")))
+        (stop #~(make-kill-destructor)))))))
+
+(define pcscd-service-type
+  (service-type
+   (name 'pcscd)
+   (description
+    "Run @command{pcscd}, the PC/SC smart card daemon.")
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             (compose list pcscd-shepherd-service))))
+   (default-value (pcscd-configuration))))
-- 
2.18.0

[bug#32358] Add pcscd service

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 3633 bytes --]

Hi Arun,

Thank you for submitting a patch!  I've been wanting something like this
for a long time, so I'm very happy to see it.  I'll test it out and let
you know if it works for me.

Arun Isaac <arunisaac@systemreboot.net> writes:

> diff --git a/gnu/packages/security-token.scm b/gnu/packages/security-token.scm
> index 7fdcaaf1e..424f4626e 100644
> --- a/gnu/packages/security-token.scm
> +++ b/gnu/packages/security-token.scm
>
> [...]
>
> @@ -169,13 +171,16 @@ the low-level development kit for the Yubico YubiKey authentication device.")
>                  "1jc9ws5ra6v3plwraqixin0w0wfxj64drahrbkyrrwzghqjjc9ss"))))
>      (build-system gnu-build-system)
>      (arguments
> -     `(#:configure-flags '("--enable-usbdropdir=/var/lib/pcsc/drivers"
> -                           "--disable-libsystemd")))
> +     `(#:configure-flags
> +       (list (string-append "--enable-usbdropdir="
> +                            (assoc-ref %build-inputs "ccid") "/pcsc/drivers")
> +             "--disable-libsystemd")))

If someone was relying on the previous behavior (in which pcsc-lite
would look in /var/lib/pcsc/drivers for drivers), this would break for
them if they were using anything other than the ccid drivers, right?
However, I understand that previous Guix email threads have talked about
adding a symlink there which points to ccid's drivers (I do that today,
myself), so I can see why you would want to make this change.

> @@ -186,6 +191,16 @@ from a client application and provide access to the desired reader.")
>                     license:isc                  ; src/strlcat.c src/strlcpy.c
>                     license:gpl3+))))            ; src/spy/*
>  
> +(define pcsc-lite-bootstrap
> +  (package
> +    (inherit pcsc-lite)
> +    (name "pcsc-lite-bootstrap")
> +    (inputs
> +     `(("libudev" ,eudev)))
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments pcsc-lite)
> +       ((#:configure-flags _) '(list "--disable-libsystemd"))))))
> +

What problem does this pcsc-lite-bootstrap package solve?  It looks like
you added pcsc-lite-bootstrap in order to avoid a cyclic dependency
between ccid and pcsc-lite.  That seems fair, but is there any other
reason that I'm missing?

> Subject: [PATCH 2/2] gnu: services: Add pcscd service.

Do you know what the intent behind the --enable-usbdropdir option is in
(1) the pcsc-lite configure script and (2) the ccid configure script?  I
checked the README in the source for both of these packages, but I
couldn't quite understand the intended use.  It wasn't clear to me if
the usbdropdir directory is where the ccid/pcsc-lite expect to _find_
drivers that the user makes available, or if this directory is where the
packages will _install_ their own drivers (or both, perhaps).

If the usbdropdir is intended to house additional drivers that the user
can "drop" in and use at runtime, then users of pcsc-lite/ccid might
reasonably expect to be able to easily add additional drivers to the
default configuration.  If that's the case, then we might want to think
about how we can give a user the ability to configure additional
drivers.  For example, if we gave the pcsc service configuration a field
like "usbdropdir-drivers" (e.g. a list of packages or file-like
objects), we could arrange for the union of those drivers to be placed
into the usbdropdir for pcsc/ccid to use.

Of course, I might be totally misunderstanding the intended use of the
--enable-usbdropdir configuration option.  If that's the case, please
let me know.

What do you think?

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32358] Add pcscd service

[Arun Isaac]

> What problem does this pcsc-lite-bootstrap package solve?  It looks like
> you added pcsc-lite-bootstrap in order to avoid a cyclic dependency
> between ccid and pcsc-lite.  That seems fair, but is there any other
> reason that I'm missing?

No, there is no other reason. I only created pcsc-lite-bootstrap to
avoid a circular dependence between ccid and pcsc-lite.

> Do you know what the intent behind the --enable-usbdropdir option is in
> (1) the pcsc-lite configure script and (2) the ccid configure script?  I
> checked the README in the source for both of these packages, but I
> couldn't quite understand the intended use.  It wasn't clear to me if
> the usbdropdir directory is where the ccid/pcsc-lite expect to _find_
> drivers that the user makes available, or if this directory is where the
> packages will _install_ their own drivers (or both, perhaps).

I thought about this too. As far as I understood, the usbdropdir of the
ccid package is where it installs its drivers, and the usbdropdir of the
pcsc-lite package is where it expects to find drivers.

> If the usbdropdir is intended to house additional drivers that the user
> can "drop" in and use at runtime, then users of pcsc-lite/ccid might
> reasonably expect to be able to easily add additional drivers to the
> default configuration.  If that's the case, then we might want to think
> about how we can give a user the ability to configure additional
> drivers.  For example, if we gave the pcsc service configuration a field
> like "usbdropdir-drivers" (e.g. a list of packages or file-like
> objects), we could arrange for the union of those drivers to be placed
> into the usbdropdir for pcsc/ccid to use.

Good idea. I'll add a usb-drivers configuration field and send an
updated patchset.

[bug#32358] Add pcscd service

[Arun Isaac]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]


Please find attached the updated patches.


[-- Attachment #2: 0001-gnu-services-Add-pcscd-service.patch --]
[-- Type: text/x-patch, Size: 5795 bytes --]

From 87133250c4d485988b2d29ec82d005befe3b5731 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Thu, 2 Aug 2018 05:32:56 +0530
Subject: [PATCH 1/2] gnu: services: Add pcscd service.

* gnu/services/security-token.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
* doc/guix.texi (Miscellaneous Services): Document the service.
---
 doc/guix.texi                   | 31 +++++++++++++
 gnu/local.mk                    |  1 +
 gnu/services/security-token.scm | 80 +++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)
 create mode 100644 gnu/services/security-token.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index 080b091b3..595cf7bf2 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20326,6 +20326,37 @@ An association list specifies kernel parameters and their values.
 @end table
 @end deftp
 
+@cindex pcscd
+@subsubheading PC/SC Smart Card Daemon Service
+
+The @code{(gnu services security-token)} module provides the following service
+to run @command{pcscd}, the PC/SC Smart Card Daemon.  @command{pcscd} is the
+daemon program for pcsc-lite and the MuscleCard framework. It is a resource
+manager that coordinates communications with smart card readers, smart cards
+and cryptographic tokens that are connected to the system.
+
+@defvr {Scheme Variable} pcscd-service-type
+Service type for the @command{pcscd} service.  Its value must be a
+@code{pcscd-configuration} object.  To run pcscd in the default
+configuration, instantiate it as:
+
+@example
+(service pcscd-service-type)
+@end example
+@end defvr
+
+@deftp {Data Type} pcscd-configuration
+The data type representing the configuration of @command{pcscd}.
+
+@table @asis
+@item @code{pcsc-lite} (default: @code{pcsc-lite})
+The pcsc-lite package that provides pcscd.
+@item @code{usb-drivers} (default: @code{(list ccid)})
+List of packages that provide USB drivers to pcscd. Drivers are expected to be
+under @file{pcsc/drivers} in the store directory of the package.
+@end table
+@end deftp
+
 @cindex lirc
 @subsubheading Lirc Service
 
diff --git a/gnu/local.mk b/gnu/local.mk
index c54a29720..8f7d56702 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -485,6 +485,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/monitoring.scm			\
   %D%/services/networking.scm			\
   %D%/services/nfs.scm			\
+  %D%/services/security-token.scm		\
   %D%/services/shepherd.scm			\
   %D%/services/sound.scm			\
   %D%/services/herd.scm				\
diff --git a/gnu/services/security-token.scm b/gnu/services/security-token.scm
new file mode 100644
index 000000000..8cda69c83
--- /dev/null
+++ b/gnu/services/security-token.scm
@@ -0,0 +1,80 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services security-token)
+  #:use-module (gnu services)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu packages admin)
+  #:use-module (gnu packages security-token)
+  #:use-module (gnu system shadow)
+  #:use-module (guix gexp)
+  #:use-module (guix modules)
+  #:use-module (guix records)
+  #:use-module (ice-9 match)
+  #:use-module (srfi srfi-26)
+  #:export (pcscd-configuration
+            pcscd-configuration?
+            pcscd-service-type))
+
+;;;
+;;; PC/SC Smart Card Daemon
+;;;
+
+(define-record-type* <pcscd-configuration>
+  pcscd-configuration make-pcscd-configuration pcscd-configuration?
+  (pcsc-lite pcscd-configuration-pcsc-lite
+             (default pcsc-lite))
+  (usb-drivers pcscd-configuration-usb-drivers
+               (default (list ccid))))
+
+(define pcscd-shepherd-service
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite)
+     (with-imported-modules (source-module-closure
+                             '((gnu build shepherd)))
+       (shepherd-service
+        (documentation "PC/SC Smart Card Daemon")
+        (provision '(pcscd))
+        (modules '((gnu build shepherd)))
+        (start #~(make-forkexec-constructor
+                  (list #$(file-append pcsc-lite "/sbin/pcscd") "-f")))
+        (stop #~(make-kill-destructor)))))))
+
+(define pcscd-activation
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite usb-drivers)
+     #~(begin
+         (use-modules (guix build utils))
+         (mkdir-p "/var/lib")
+         (symlink #$(directory-union
+                     "pcsc"
+                     (map (cut file-append <> "/pcsc")
+                          usb-drivers))
+                  "/var/lib/pcsc")))))
+
+(define pcscd-service-type
+  (service-type
+   (name 'pcscd)
+   (description
+    "Run @command{pcscd}, the PC/SC smart card daemon.")
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             (compose list pcscd-shepherd-service))
+          (service-extension activation-service-type
+                             pcscd-activation)))
+   (default-value (pcscd-configuration))))
-- 
2.18.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gnu-ccid-Move-pcsc-lite-from-inputs-to-native-inputs.patch --]
[-- Type: text/x-patch, Size: 1205 bytes --]

From 9e6f8943a9da4e786436c3e568a5628c28fdda48 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Thu, 9 Aug 2018 19:19:03 +0530
Subject: [PATCH 2/2] gnu: ccid: Move pcsc-lite from inputs to native-inputs.

pcsc-lite only provides the headers to build ccid. So, it is sufficient that
it be a native-input.

* gnu/packages/security-token.scm (ccid)[inputs]: Move pcsc-lite to ...
[native-inputs]: ... here.
---
 gnu/packages/security-token.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/security-token.scm b/gnu/packages/security-token.scm
index 7fdcaaf1e..ff0383d11 100644
--- a/gnu/packages/security-token.scm
+++ b/gnu/packages/security-token.scm
@@ -75,11 +75,11 @@
                (("/bin/echo") (which "echo")))
              #t)))))
     (native-inputs
-     `(("perl" ,perl)
+     `(("pcsc-lite" ,pcsc-lite) ; only required for headers
+       ("perl" ,perl)
        ("pkg-config" ,pkg-config)))
     (inputs
-     `(("libusb" ,libusb)
-       ("pcsc-lite" ,pcsc-lite)))
+     `(("libusb" ,libusb)))
     (home-page "https://ccid.apdu.fr/")
     (synopsis "PC/SC driver for USB smart card devices")
     (description
-- 
2.18.0

[bug#32358] Add pcscd service

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

Hi Arun,

Thank you for the update!  I'm having a little trouble testing this on
my system due to the following unrelated bug:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28144

However, I'll keep trying and let you know once I've tested it out.  For
now, here's my feedback.

Overall, this looks good to me.  Mainly, I'd just like to test it before
we commit it to master.  If you're feeling up to it, it would be great
to add a system test, but it's not necessary, and we could add it later.

Arun Isaac <arunisaac@systemreboot.net> writes:

> +(define-module (gnu services security-token)
> +  #:use-module (gnu services)
> +  #:use-module (gnu services shepherd)
> +  #:use-module (gnu packages admin)
> +  #:use-module (gnu packages security-token)
> +  #:use-module (gnu system shadow)
> +  #:use-module (guix gexp)
> +  #:use-module (guix modules)
> +  #:use-module (guix records)
> +  #:use-module (ice-9 match)
> +  #:use-module (srfi srfi-26)
> +  #:export (pcscd-configuration
> +            pcscd-configuration?
> +            pcscd-service-type))

Shouldn't we also export pcscd-configuration-pcsc-lite and
pcscd-configuration-usb-drivers?  We allow users to set them when
constructing the record, so it seems reasonable to allow users to get
them, to.  After all, if a user wants to construct a new
<pcscd-configuration> from an existing (e.g., using "inherit"), they
might need to be able to get the original values (e.g., to add more
packages to the usb-drivers list).

> +(define pcscd-shepherd-service
> +  (match-lambda
> +    (($ <pcscd-configuration> pcsc-lite)
> +     (with-imported-modules (source-module-closure
> +                             '((gnu build shepherd)))
> +       (shepherd-service
> +        (documentation "PC/SC Smart Card Daemon")
> +        (provision '(pcscd))
> +        (modules '((gnu build shepherd)))
> +        (start #~(make-forkexec-constructor
> +                  (list #$(file-append pcsc-lite "/sbin/pcscd") "-f")))
> +        (stop #~(make-kill-destructor)))))))

Does this work as written?  The make-forkexec-constructor and
make-kill-destructor procedures are exported in (shepherd service), but
it doesn't look like that module will be used, since it isn't in the
modules list.  If it does work, then I don't understand how (shepherd
service) is getting used, so I'd be curious to know why it works!

> +(define pcscd-activation
> +  (match-lambda
> +    (($ <pcscd-configuration> pcsc-lite usb-drivers)
> +     #~(begin
> +         (use-modules (guix build utils))
> +         (mkdir-p "/var/lib")
> +         (symlink #$(directory-union
> +                     "pcsc"
> +                     (map (cut file-append <> "/pcsc")
> +                          usb-drivers))
> +                  "/var/lib/pcsc")))))

What happens if the symlink target already exists?  Will this crash the
init process, or will the system come online and just report an error?
Some people (such as myself) have already created this directory
manually, so the directory might exist if they forget to delete it.

> Subject: [PATCH 2/2] gnu: ccid: Move pcsc-lite from inputs to native-inputs.

Patch 2/2 looks good to me!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32358] Add pcscd service

[Arun Isaac]

> I'm having a little trouble testing this on my system due to the
> following unrelated bug:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28144
>
> However, I'll keep trying and let you know once I've tested it out.

Sure, no problem.

> If you're feeling up to it, it would be great to add a system test,
> but it's not necessary, and we could add it later.

I'm ok with adding a system test right now. But, what kind of test? Can
you elaborate on any ideas you have?

> Shouldn't we also export pcscd-configuration-pcsc-lite and
> pcscd-configuration-usb-drivers?

Sure, will do.

>> +(define pcscd-shepherd-service
>> +  (match-lambda
>> +    (($ <pcscd-configuration> pcsc-lite)
>> +     (with-imported-modules (source-module-closure
>> +                             '((gnu build shepherd)))
>> +       (shepherd-service
>> +        (documentation "PC/SC Smart Card Daemon")
>> +        (provision '(pcscd))
>> +        (modules '((gnu build shepherd)))
>> +        (start #~(make-forkexec-constructor
>> +                  (list #$(file-append pcsc-lite "/sbin/pcscd") "-f")))
>> +        (stop #~(make-kill-destructor)))))))
>
> Does this work as written?  The make-forkexec-constructor and
> make-kill-destructor procedures are exported in (shepherd service), but
> it doesn't look like that module will be used, since it isn't in the
> modules list.  If it does work, then I don't understand how (shepherd
> service) is getting used, so I'd be curious to know why it works!

Yes, the service does work. But, I don't really know why. I copied this
bit of code from some other service and modified it incrementally until
it did what I wanted. :-P So, I'm not super-clear what exactly is
happening here.

>> +(define pcscd-activation
>> +  (match-lambda
>> +    (($ <pcscd-configuration> pcsc-lite usb-drivers)
>> +     #~(begin
>> +         (use-modules (guix build utils))
>> +         (mkdir-p "/var/lib")
>> +         (symlink #$(directory-union
>> +                     "pcsc"
>> +                     (map (cut file-append <> "/pcsc")
>> +                          usb-drivers))
>> +                  "/var/lib/pcsc")))))
>
> What happens if the symlink target already exists?  Will this crash the
> init process, or will the system come online and just report an error?
> Some people (such as myself) have already created this directory
> manually, so the directory might exist if they forget to delete it.

When the symlink already exists, the system reconfigures properly, but
reports an error. You will have to delete your existing /var/lib/pcsc
symlink before reconfiguring.

>> Subject: [PATCH 2/2] gnu: ccid: Move pcsc-lite from inputs to native-inputs.
>
> Patch 2/2 looks good to me!

I pushed this patch alone to master.

[bug#32358] Add pcscd service

[Arun Isaac]

Please find attached an updated patch. I have used invoke instead of
make-forkexec-constructor. I have also used (call-with-input-file
"/var/run/pcscd/pcscd.pid" read) so that shepherd knows the correct
PID. I hope I have followed your discussion correctly. Let me know if
this patch is satisfactory.

I am working on a simple test for this service. I will hopefully be done
and send you a patch for the same within 24 hours.

[bug#32358] Add pcscd service

[Arun Isaac]

[-- Attachment #1: Type: text/plain, Size: 58 bytes --]


Sorry, I forgot to attach the patch in my earlier mail.


[-- Attachment #2: 0001-gnu-services-Add-pcscd-service.patch --]
[-- Type: text/x-patch, Size: 5972 bytes --]

From 17b4763615e40d2ac821021ccbd451ad9ac0d10d Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Thu, 2 Aug 2018 05:32:56 +0530
Subject: [PATCH] gnu: services: Add pcscd service.

* gnu/services/security-token.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
* doc/guix.texi (Miscellaneous Services): Document the service.
---
 doc/guix.texi                   | 31 ++++++++++++
 gnu/local.mk                    |  1 +
 gnu/services/security-token.scm | 84 +++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+)
 create mode 100644 gnu/services/security-token.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index a9bb6d864..36d857082 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20331,6 +20331,37 @@ An association list specifies kernel parameters and their values.
 @end table
 @end deftp
 
+@cindex pcscd
+@subsubheading PC/SC Smart Card Daemon Service
+
+The @code{(gnu services security-token)} module provides the following service
+to run @command{pcscd}, the PC/SC Smart Card Daemon.  @command{pcscd} is the
+daemon program for pcsc-lite and the MuscleCard framework. It is a resource
+manager that coordinates communications with smart card readers, smart cards
+and cryptographic tokens that are connected to the system.
+
+@defvr {Scheme Variable} pcscd-service-type
+Service type for the @command{pcscd} service.  Its value must be a
+@code{pcscd-configuration} object.  To run pcscd in the default
+configuration, instantiate it as:
+
+@example
+(service pcscd-service-type)
+@end example
+@end defvr
+
+@deftp {Data Type} pcscd-configuration
+The data type representing the configuration of @command{pcscd}.
+
+@table @asis
+@item @code{pcsc-lite} (default: @code{pcsc-lite})
+The pcsc-lite package that provides pcscd.
+@item @code{usb-drivers} (default: @code{(list ccid)})
+List of packages that provide USB drivers to pcscd. Drivers are expected to be
+under @file{pcsc/drivers} in the store directory of the package.
+@end table
+@end deftp
+
 @cindex lirc
 @subsubheading Lirc Service
 
diff --git a/gnu/local.mk b/gnu/local.mk
index dee36f8b4..29c05dfc9 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -486,6 +486,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/monitoring.scm			\
   %D%/services/networking.scm			\
   %D%/services/nfs.scm			\
+  %D%/services/security-token.scm		\
   %D%/services/shepherd.scm			\
   %D%/services/sound.scm			\
   %D%/services/herd.scm				\
diff --git a/gnu/services/security-token.scm b/gnu/services/security-token.scm
new file mode 100644
index 000000000..7e7ea54a5
--- /dev/null
+++ b/gnu/services/security-token.scm
@@ -0,0 +1,84 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services security-token)
+  #:use-module (gnu services)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu packages admin)
+  #:use-module (gnu packages security-token)
+  #:use-module (gnu system shadow)
+  #:use-module (guix gexp)
+  #:use-module (guix modules)
+  #:use-module (guix records)
+  #:use-module (ice-9 match)
+  #:use-module (srfi srfi-26)
+  #:export (pcscd-configuration
+            pcscd-configuration?
+            pcscd-configuration-pcsc-lite
+            pcscd-configuration-usb-drivers
+            pcscd-service-type))
+
+;;;
+;;; PC/SC Smart Card Daemon
+;;;
+
+(define-record-type* <pcscd-configuration>
+  pcscd-configuration make-pcscd-configuration pcscd-configuration?
+  (pcsc-lite pcscd-configuration-pcsc-lite
+             (default pcsc-lite))
+  (usb-drivers pcscd-configuration-usb-drivers
+               (default (list ccid))))
+
+(define pcscd-shepherd-service
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite)
+     (with-imported-modules (source-module-closure
+                             '((gnu build shepherd)))
+       (shepherd-service
+        (documentation "PC/SC Smart Card Daemon")
+        (provision '(pcscd))
+        (requirement '(syslogd))
+        (modules '((gnu build shepherd)))
+        (start #~(lambda _
+                   (invoke #$(file-append pcsc-lite "/sbin/pcscd"))
+                   (call-with-input-file "/var/run/pcscd/pcscd.pid" read)))
+        (stop #~(make-kill-destructor)))))))
+
+(define pcscd-activation
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite usb-drivers)
+     #~(begin
+         (use-modules (guix build utils))
+         (mkdir-p "/var/lib")
+         (symlink #$(directory-union
+                     "pcsc"
+                     (map (cut file-append <> "/pcsc")
+                          usb-drivers))
+                  "/var/lib/pcsc")))))
+
+(define pcscd-service-type
+  (service-type
+   (name 'pcscd)
+   (description
+    "Run @command{pcscd}, the PC/SC smart card daemon.")
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             (compose list pcscd-shepherd-service))
+          (service-extension activation-service-type
+                             pcscd-activation)))
+   (default-value (pcscd-configuration))))
-- 
2.18.0

[bug#32358] Add pcscd service

[Arun Isaac]

[-- Attachment #1: Type: text/plain, Size: 81 bytes --]


I have finished work on the tests too. Please find attached the updated
patch.


[-- Attachment #2: 0001-gnu-services-Add-pcscd-service.patch --]
[-- Type: text/x-patch, Size: 9015 bytes --]

From 58af4b276dfc34af6dc27f213e26110ca9e4ffcb Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Thu, 2 Aug 2018 05:32:56 +0530
Subject: [PATCH] gnu: services: Add pcscd service.

* gnu/services/security-token.scm: New file.
* gnu/tests/security-token.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Register new files.
* doc/guix.texi (Miscellaneous Services): Document the service.
---
 doc/guix.texi                   | 31 ++++++++++++
 gnu/local.mk                    |  2 +
 gnu/services/security-token.scm | 84 +++++++++++++++++++++++++++++++++
 gnu/tests/security-token.scm    | 70 +++++++++++++++++++++++++++
 4 files changed, 187 insertions(+)
 create mode 100644 gnu/services/security-token.scm
 create mode 100644 gnu/tests/security-token.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index 1cc73eb93..560daf5e8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20331,6 +20331,37 @@ An association list specifies kernel parameters and their values.
 @end table
 @end deftp
 
+@cindex pcscd
+@subsubheading PC/SC Smart Card Daemon Service
+
+The @code{(gnu services security-token)} module provides the following service
+to run @command{pcscd}, the PC/SC Smart Card Daemon.  @command{pcscd} is the
+daemon program for pcsc-lite and the MuscleCard framework. It is a resource
+manager that coordinates communications with smart card readers, smart cards
+and cryptographic tokens that are connected to the system.
+
+@defvr {Scheme Variable} pcscd-service-type
+Service type for the @command{pcscd} service.  Its value must be a
+@code{pcscd-configuration} object.  To run pcscd in the default
+configuration, instantiate it as:
+
+@example
+(service pcscd-service-type)
+@end example
+@end defvr
+
+@deftp {Data Type} pcscd-configuration
+The data type representing the configuration of @command{pcscd}.
+
+@table @asis
+@item @code{pcsc-lite} (default: @code{pcsc-lite})
+The pcsc-lite package that provides pcscd.
+@item @code{usb-drivers} (default: @code{(list ccid)})
+List of packages that provide USB drivers to pcscd. Drivers are expected to be
+under @file{pcsc/drivers} in the store directory of the package.
+@end table
+@end deftp
+
 @cindex lirc
 @subsubheading Lirc Service
 
diff --git a/gnu/local.mk b/gnu/local.mk
index 53120db7f..d6eb140ed 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -486,6 +486,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/monitoring.scm			\
   %D%/services/networking.scm			\
   %D%/services/nfs.scm			\
+  %D%/services/security-token.scm		\
   %D%/services/shepherd.scm			\
   %D%/services/sound.scm			\
   %D%/services/herd.scm				\
@@ -540,6 +541,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/tests/messaging.scm			\
   %D%/tests/networking.scm			\
   %D%/tests/rsync.scm				\
+  %D%/tests/security-token.scm			\
   %D%/tests/ssh.scm				\
   %D%/tests/version-control.scm			\
   %D%/tests/virtualization.scm			\
diff --git a/gnu/services/security-token.scm b/gnu/services/security-token.scm
new file mode 100644
index 000000000..7e7ea54a5
--- /dev/null
+++ b/gnu/services/security-token.scm
@@ -0,0 +1,84 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services security-token)
+  #:use-module (gnu services)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu packages admin)
+  #:use-module (gnu packages security-token)
+  #:use-module (gnu system shadow)
+  #:use-module (guix gexp)
+  #:use-module (guix modules)
+  #:use-module (guix records)
+  #:use-module (ice-9 match)
+  #:use-module (srfi srfi-26)
+  #:export (pcscd-configuration
+            pcscd-configuration?
+            pcscd-configuration-pcsc-lite
+            pcscd-configuration-usb-drivers
+            pcscd-service-type))
+
+;;;
+;;; PC/SC Smart Card Daemon
+;;;
+
+(define-record-type* <pcscd-configuration>
+  pcscd-configuration make-pcscd-configuration pcscd-configuration?
+  (pcsc-lite pcscd-configuration-pcsc-lite
+             (default pcsc-lite))
+  (usb-drivers pcscd-configuration-usb-drivers
+               (default (list ccid))))
+
+(define pcscd-shepherd-service
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite)
+     (with-imported-modules (source-module-closure
+                             '((gnu build shepherd)))
+       (shepherd-service
+        (documentation "PC/SC Smart Card Daemon")
+        (provision '(pcscd))
+        (requirement '(syslogd))
+        (modules '((gnu build shepherd)))
+        (start #~(lambda _
+                   (invoke #$(file-append pcsc-lite "/sbin/pcscd"))
+                   (call-with-input-file "/var/run/pcscd/pcscd.pid" read)))
+        (stop #~(make-kill-destructor)))))))
+
+(define pcscd-activation
+  (match-lambda
+    (($ <pcscd-configuration> pcsc-lite usb-drivers)
+     #~(begin
+         (use-modules (guix build utils))
+         (mkdir-p "/var/lib")
+         (symlink #$(directory-union
+                     "pcsc"
+                     (map (cut file-append <> "/pcsc")
+                          usb-drivers))
+                  "/var/lib/pcsc")))))
+
+(define pcscd-service-type
+  (service-type
+   (name 'pcscd)
+   (description
+    "Run @command{pcscd}, the PC/SC smart card daemon.")
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             (compose list pcscd-shepherd-service))
+          (service-extension activation-service-type
+                             pcscd-activation)))
+   (default-value (pcscd-configuration))))
diff --git a/gnu/tests/security-token.scm b/gnu/tests/security-token.scm
new file mode 100644
index 000000000..fb520face
--- /dev/null
+++ b/gnu/tests/security-token.scm
@@ -0,0 +1,70 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu tests security-token)
+  #:use-module (gnu tests)
+  #:use-module (gnu system vm)
+  #:use-module (gnu services)
+  #:use-module (gnu services security-token)
+  #:use-module (guix gexp)
+  #:export (%test-pcscd))
+
+(define %pcscd-os
+  (simple-operating-system
+   (service pcscd-service-type)))
+
+(define* (run-pcscd-test)
+  "Run tests of 'pcscd-service-type'."
+  (define os
+    (marionette-operating-system
+     %pcscd-os
+     #:imported-modules '((gnu services herd))))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (srfi srfi-64)
+                       (gnu build marionette))
+          (define marionette
+            (make-marionette (list #$(virtual-machine os))))
+
+          (mkdir #$output)
+          (chdir #$output)
+
+          (test-begin "pcscd")
+
+          (test-assert "pcscd is alive"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd))
+                (live-service-running
+                 (find (lambda (live)
+                         (memq 'pcscd (live-service-provision live)))
+                       (current-services))))
+             marionette))
+
+          (test-end)
+          (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
+
+  (gexp->derivation "pcscd" test))
+
+(define %test-pcscd
+  (system-test
+   (name "pcscd")
+   (description "Test a running pcscd daemon.")
+   (value (run-pcscd-test))))
-- 
2.18.0

[bug#32358] Add pcscd service

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 4583 bytes --]

Hi Arun,

It turns out that when we run pcscd in the foreground with the -f
option, it won't emit messages to syslog.  Instead, it emits messages to
stderr, and those messages will not be stored in logs, as explained in
the following bug report:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30939

To ensure users can easily find the messages, I think we should avoid
using the "-f" option.

In addition, pcscd logs its PID to /var/run/pcscd/pcscd.pid.  To ensure
that Shepherd can still tell if the service is alive even when we do not
run it in the foreground, we should invoke make-forkexec-constructor
with the #:pid-file keyword argument.

Could you make those last couple changes?  Everything else looks great!

Arun Isaac <arunisaac@systemreboot.net> writes:

>> I'm having a little trouble testing this on my system due to the
>> following unrelated bug:
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28144
>>
>> However, I'll keep trying and let you know once I've tested it out.
>
> Sure, no problem.

I was successful in testing it.  The service works for me!

> I'm ok with adding a system test right now. But, what kind of test? Can
> you elaborate on any ideas you have?

It would be good to have a system test that verifies that pcscd has
successfully started.  Even such a simple test would be useful, since it
would catch a certain class of problems.  There are a lot of existing
examples in the gnu/tests directory.  I recently added a test like this
for the tor service, which you can find here (I haven't committed it to
master yet):

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32346

>>> +(define pcscd-shepherd-service
>>> +  (match-lambda
>>> +    (($ <pcscd-configuration> pcsc-lite)
>>> +     (with-imported-modules (source-module-closure
>>> +                             '((gnu build shepherd)))
>>> +       (shepherd-service
>>> +        (documentation "PC/SC Smart Card Daemon")
>>> +        (provision '(pcscd))
>>> +        (modules '((gnu build shepherd)))
>>> +        (start #~(make-forkexec-constructor
>>> +                  (list #$(file-append pcsc-lite "/sbin/pcscd") "-f")))
>>> +        (stop #~(make-kill-destructor)))))))
>>
>> Does this work as written?  The make-forkexec-constructor and
>> make-kill-destructor procedures are exported in (shepherd service), but
>> it doesn't look like that module will be used, since it isn't in the
>> modules list.  If it does work, then I don't understand how (shepherd
>> service) is getting used, so I'd be curious to know why it works!
>
> Yes, the service does work. But, I don't really know why. I copied this
> bit of code from some other service and modified it incrementally until
> it did what I wanted. :-P So, I'm not super-clear what exactly is
> happening here.

I've looked into this.  The reason it works is because the "start"
field's g-expression is expanded into the Shepherd's configuration file
(see: (guix) Shepherd Services), which is evaluated in a context where
bindings from the (shepherd service) module are available (see:
(shepherd) Invoking shepherd).  Therefore, the "start" field's
g-expression can use procedures from (shepherd service), such as
make-forkexec-constructor, regardless of what is listed in the "modules"
field.

>>> +(define pcscd-activation
>>> +  (match-lambda
>>> +    (($ <pcscd-configuration> pcsc-lite usb-drivers)
>>> +     #~(begin
>>> +         (use-modules (guix build utils))
>>> +         (mkdir-p "/var/lib")
>>> +         (symlink #$(directory-union
>>> +                     "pcsc"
>>> +                     (map (cut file-append <> "/pcsc")
>>> +                          usb-drivers))
>>> +                  "/var/lib/pcsc")))))
>>
>> What happens if the symlink target already exists?  Will this crash the
>> init process, or will the system come online and just report an error?
>> Some people (such as myself) have already created this directory
>> manually, so the directory might exist if they forget to delete it.
>
> When the symlink already exists, the system reconfigures properly, but
> reports an error. You will have to delete your existing /var/lib/pcsc
> symlink before reconfiguring.

OK.  As long as there's a useful error message, that's good!

>>> Subject: [PATCH 2/2] gnu: ccid: Move pcsc-lite from inputs to native-inputs.
>>
>> Patch 2/2 looks good to me!
>
> I pushed this patch alone to master.

Great!  Thank you.  I look forward to getting the service itself into
master, also!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32358] Add pcscd service

[Clément Lassieur]

Hi all!

Chris Marusich <cmmarusich@gmail.com> writes:

> Hi Arun,
>
> It turns out that when we run pcscd in the foreground with the -f
> option, it won't emit messages to syslog.  Instead, it emits messages to
> stderr, and those messages will not be stored in logs, as explained in
> the following bug report:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30939
>
> To ensure users can easily find the messages, I think we should avoid
> using the "-f" option.
>
> In addition, pcscd logs its PID to /var/run/pcscd/pcscd.pid.  To ensure
> that Shepherd can still tell if the service is alive even when we do not
> run it in the foreground, we should invoke make-forkexec-constructor
> with the #:pid-file keyword argument.

Without the '-f' option, pcscd would return immediately, and thus
calling MAKE-FORKEXEC-CONSTRUCTOR wouldn't make sense.  Instead, you
could just INVOKE it.  See the 'prosody', 'postgres' and 'nginx'
services for examples.  And you could stop it by sending SIGTERM to the
PID in /var/run/pcscd/pcscd.pid I guess.

Also, the systemd unit files I've seen use the '--auto-exit' and
'--hotplug' options.  Do you know if they make sense for us?

Clément

[bug#32358] Add pcscd service

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 3243 bytes --]

Hi Clément!

Clément Lassieur <clement@lassieur.org> writes:

> Hi all!
>
> Chris Marusich <cmmarusich@gmail.com> writes:
>
>> Hi Arun,
>>
>> It turns out that when we run pcscd in the foreground with the -f
>> option, it won't emit messages to syslog.  Instead, it emits messages to
>> stderr, and those messages will not be stored in logs, as explained in
>> the following bug report:
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30939
>>
>> To ensure users can easily find the messages, I think we should avoid
>> using the "-f" option.
>>
>> In addition, pcscd logs its PID to /var/run/pcscd/pcscd.pid.  To ensure
>> that Shepherd can still tell if the service is alive even when we do not
>> run it in the foreground, we should invoke make-forkexec-constructor
>> with the #:pid-file keyword argument.
>
> Without the '-f' option, pcscd would return immediately, and thus
> calling MAKE-FORKEXEC-CONSTRUCTOR wouldn't make sense.  Instead, you
> could just INVOKE it.  See the 'prosody', 'postgres' and 'nginx'
> services for examples.  And you could stop it by sending SIGTERM to the
> PID in /var/run/pcscd/pcscd.pid I guess.

Thank you for mentioning this.  You make a good point.  I'm not yet
familiar with all the different ways Shepherd manages its herd of
processes.  For example, I know Shepherd looks for the #:pid-file when
first starting the process to determine whether it started up
successfully.  However, I don't know if Shepherd uses that PID file for
anything else while managing the process later on.

In any case, the procedure make-forkexec-constructor seems to hide a lot
of the nitty gritty details (e.g., closing file descriptors), so I think
it would be better to use it instead of invoking pcscd on our own if we
don't have to.

Perhaps we can do the following:

* Run pcscd in the foreground (with -f).
* Use make-forkexec-constructor.
* Tell Shepherd about the PID file via the #:pid-file option to
  make-forkexec-constructor.
* Redirect pcscd's stdout/stderr to /var/log/pcscd via the #:log-file
  option to make-forkexec-constructor.

That way, we can still use the convenient make-forkexec-constructor
procedure, Shepherd will still be able to manage the process, and people
will be able to find the logs more easily.

What do you think?

> Also, the systemd unit files I've seen use the '--auto-exit' and
> '--hotplug' options.  Do you know if they make sense for us?

These options are interesting, but I don't think they're useful for us
at this time.

Judging by the manual, it looks like the "--auto-exit" option is
intended to be used when a mechanism exists for automatically starting
pcscd on demand (maybe when a user plugs in a security token?), so you
don't need to keep it running all the time.  We don't have a mechanism
like that at this time.

As for the "--hotplug" option, it looks like it's intended to be used to
tell a running pcscd to re-read its configuration file.  However, since
that configuration file is an immutable file in the store, it isn't ever
going to change.

Maybe in the future we can think of helpful ways to use these options,
but for now I think it's OK to omit them.

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32358] Add pcscd service

[Clément Lassieur]

Chris Marusich <cmmarusich@gmail.com> writes:

> Thank you for mentioning this.  You make a good point.  I'm not yet
> familiar with all the different ways Shepherd manages its herd of
> processes.  For example, I know Shepherd looks for the #:pid-file when
> first starting the process to determine whether it started up
> successfully.

When using MAKE-FORKEXEC-CONSTRUCTOR, yes.  And it makes sense only for
blocking processes, because they are 'dumb', they don't return
information about whether they have succesfully started or not.
However, 'pcscd' is smart, it does return '1' if it failed to start, and
'0' if it started successfully.  So, all the logic (the pid check and
everything) is done within 'pcscd' itself, no need for the Shepherd to
do it.

(start (pid-of smart-nonblocking-process)) == (start (make-forkexec-constructor dumb-blocking-process))

> However, I don't know if Shepherd uses that PID file for anything else
> while managing the process later on.

No, it doesn't use the #:PID-FILE argument for anything else.  However,
it does use the return value of the start procedure as a PID file to
automatically respawn the service if it's an integer.  So you just need
to return (call-with-input-file pid-file read) instead of #t if you want
automatic respawning, as in
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=fdbca05d78d091bfc075e54c9fb90125262eadf0.

> In any case, the procedure make-forkexec-constructor seems to hide a lot
> of the nitty gritty details (e.g., closing file descriptors), so I think
> it would be better to use it instead of invoking pcscd on our own if we
> don't have to.

All this stuff is done by 'pcscd' when run without '-f', using
MAKE-FORKEXEC-CONSTRUCTOR doesn't help in any way.  Moreover, we do lose
the 'syslog' feature if we use it.

> Perhaps we can do the following:
>
> * Run pcscd in the foreground (with -f).
> * Use make-forkexec-constructor.
> * Tell Shepherd about the PID file via the #:pid-file option to
>   make-forkexec-constructor.
> * Redirect pcscd's stdout/stderr to /var/log/pcscd via the #:log-file
>   option to make-forkexec-constructor.
>
> That way, we can still use the convenient make-forkexec-constructor
> procedure, Shepherd will still be able to manage the process, and people
> will be able to find the logs more easily.

> What do you think?

I think it's better to use syslog ;-)  By the way, 'syslogd' should be
added as a requirement I think.

>> Also, the systemd unit files I've seen use the '--auto-exit' and
>> '--hotplug' options.  Do you know if they make sense for us?

[...]

> Maybe in the future we can think of helpful ways to use these options,
> but for now I think it's OK to omit them.

Ok!

Clément

[bug#32358] Add pcscd service

[Clément Lassieur]

Clément Lassieur <clement@lassieur.org> writes:

>> However, I don't know if Shepherd uses that PID file for anything else
>> while managing the process later on.
>
> No, it doesn't use the #:PID-FILE argument for anything else.  However,
> it does use the return value of the start procedure as a PID file to
> automatically respawn the service if it's an integer.  So you just need
> to return (call-with-input-file pid-file read) instead of #t if you want
> automatic respawning, as in
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=fdbca05d78d091bfc075e54c9fb90125262eadf0.

Note that the 'running' slot would then have the PID value, and
'make-kill-destructor' would work, so no need to change the stop
procedure.

[bug#32358] Add pcscd service

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

Hi Clément and Arun,

Clément, your explanation makes sense.  With that information, I now
understand why it isn't necessary to use make-forkexec-constructor in
this case.  Thank you for taking the time to explain it in detail!

Arun Isaac <arunisaac@systemreboot.net> writes:

> I have finished work on the tests too. Please find attached the updated
> patch.

Awesome!

> +(define* (run-pcscd-test)
> +  "Run tests of 'pcscd-service-type'."
> +  (define os
> +    (marionette-operating-system
> +     %pcscd-os
> +     #:imported-modules '((gnu services herd))))

If you use the #:requirements keyword argument to
marionette-operating-system, you can guarantee that the marionette
service will start up after the pcscd service.  This would be good in
order to avoid the possibility that you might marionette-eval something
before the pcscd service is started.

With that small change, if the system test passes, I think this is good
to go to master.  Thank you for taking the time to work on this and
iterate through the feedback!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[bug#32358] Add pcscd service

[Clément Lassieur]

Hi Chris and Arun,

Chris Marusich <cmmarusich@gmail.com> writes:

> Hi Clément and Arun,
>
> Clément, your explanation makes sense.  With that information, I now
> understand why it isn't necessary to use make-forkexec-constructor in
> this case.  Thank you for taking the time to explain it in detail!

Thank you all for your work!

Clément

bug#32358: Add pcscd service

[Arun Isaac]

Hi Clement and Chris,

I have pushed the pcscd service to master. Thank you both for your
feedback and review!

Regards,
Arun.

[bug#32358] About commit "Avoid assertion violations in maybe_produce_line_number"

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

If you reached this debbugs thread from this commit:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f0b8e64fb7720a9376bde80cc59fe37b0df83b9d,
the commit message actually has a typo. That commit fixes debbugs#32258:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32258
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 591 bytes --]