unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Chris Marusich <cmmarusich@gmail.com>
To: Arun Isaac <arunisaac@systemreboot.net>
Cc: 32358@debbugs.gnu.org
Subject: [bug#32358] Add pcscd service
Date: Sun, 12 Aug 2018 15:26:47 -0700	[thread overview]
Message-ID: <87sh3jkyqg.fsf@gmail.com> (raw)
In-Reply-To: <b19c2167.AL4AABKe2YsAAAAAAAAAAAQxnJwAAAACwQwAAAAAAAW9WABbb-8t@mailjet.com> (Arun Isaac's message of "Sun, 12 Aug 2018 13:55:52 +0530")

[-- 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 --]

  parent reply	other threads:[~2018-08-12 22:28 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03 10:23 [bug#32358] Add pcscd service Arun Isaac
2018-08-04  4:15 ` Chris Marusich
2018-08-04 12:42   ` Arun Isaac
2018-08-09 14:25     ` Arun Isaac
2018-08-10  6:56       ` Chris Marusich
2018-08-12  8:25         ` Arun Isaac
2018-08-13 21:17           ` Arun Isaac
2018-08-13 21:24             ` Arun Isaac
2018-08-14  9:00               ` Arun Isaac
     [not found]         ` <b19c2167.AL4AABKe2YsAAAAAAAAAAAQxnJwAAAACwQwAAAAAAAW9WABbb-8t@mailjet.com>
2018-08-12 22:26           ` Chris Marusich [this message]
2018-08-12 23:31             ` Clément Lassieur
2018-08-13  7:18               ` Chris Marusich
2018-08-13 16:21                 ` Clément Lassieur
2018-08-13 16:36                   ` Clément Lassieur
2018-08-15  5:55                   ` Chris Marusich
2018-08-15 19:00                     ` Clément Lassieur
2018-08-15 21:24                       ` bug#32358: " Arun Isaac
2018-08-06 14:36 ` [bug#32358] About commit "Avoid assertion violations in maybe_produce_line_number" Kaushal Modi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sh3jkyqg.fsf@gmail.com \
    --to=cmmarusich@gmail.com \
    --cc=32358@debbugs.gnu.org \
    --cc=arunisaac@systemreboot.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).