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