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