[Shepherd 0.9.3] ‘system*’ replacement cannot be passed environment variables

  • Done
  • quality assurance status badge
Details
3 participants
  • Adam Faiz
  • Ludovic Courtès
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important
L
L
Ludovic Courtès wrote on 15 Dec 2022 23:47
(address . bug-guix@gnu.org)
87pmckl1xo.fsf@inria.fr
As we found out while debugging https://issues.guix.gnu.org/60010, the
Shepherd 0.9.3, with its ‘system*’ replacement (aka. ‘spawn-command’),
makes it very hard to spawn a command with different environment
variables.

The following options don’t work:

• Changing shepherd’s own environment variables with ‘setenv’ for
instance: ‘spawn-command’ calls ‘fork+exec-command’, whose default
#:environment-variables is provided by the
‘default-environment-variables’ parameter, which gets its default
value at when shepherd starts. There’s no environment variable
inheritance, contrary to the real ‘system*’.

• Parameterizing ‘default-environment-variables’:

(parameterize ((default-environment-variables …))
(system* …))

That won’t work because ‘spawn-command’ delegates to the process
monitoring fiber, which has a different dynamic state and thus
doesn’t see this change.

• Even a plain (set! default-environment-variables …) won’t work,
probably due to inlining within (shepherd services).

I think we’ll have to add a parameter to ‘spawn-command’ to specify
environment variables.

Ludo’.
A
A
Adam Faiz wrote on 22 Dec 2022 17:17
[Shepherd 0.9.3] ‘system*’ replacement cannot be passed environment variables
(address . 60106@debbugs.gnu.org)(address . ludovic.courtes@inria.fr)
7bff0337-5bfd-22d9-8cc2-b7912eef0f66@disroot.org
Toggle quote (4 lines)
> I think we’ll have to add a parameter to ‘spawn-command’ to specify
> environment variables.
>
> Ludo’.
If you do this, can you add an #:append? flag which adds environment
variables to the inherited environment instead of specifying the
variables declaratively? It can be #f by default.

It would be very useful for me using the shepherd as init on a foreign
distro, so I don't have to use the `env` command.
L
L
Ludovic Courtès wrote on 22 Dec 2022 17:47
Re: [Shepherd 0.9.3] ‘system*’ replacement cannot be passed environment variables
(name . Adam Faiz)(address . adam.faiz@disroot.org)(address . 60106@debbugs.gnu.org)
87k02j1j2u.fsf@inria.fr
Adam Faiz <adam.faiz@disroot.org> skribis:

Toggle quote (10 lines)
>> I think we’ll have to add a parameter to ‘spawn-command’ to specify
>> environment variables.
>> Ludo’.
> If you do this, can you add an #:append? flag which adds environment
> variables to the inherited environment instead of specifying the
> variables declaratively? It can be #f by default.
>
> It would be very useful for me using the shepherd as init on a foreign
> distro, so I don't have to use the `env` command.

You could always write something like:

#:environment-variables `("EXTRA_VARIABLE=something" ,@(environ))

to append ‘EXTRA_VARIABLE’ to those of the environment.

So I don’t think we need #:append.

Thanks for your feedback,
Ludo’.
L
L
Ludovic Courtès wrote on 17 Jan 2023 16:21
control message for bug #60106
(address . control@debbugs.gnu.org)
87lem1faqm.fsf@gnu.org
severity 60106 important
quit
L
L
Ludovic Courtès wrote on 4 Mar 2023 23:09
Re: bug#61803: [PATCH 0/3] [shepherd] improve race-free spawn+wait
(name . Ulf Herrman)(address . striness@tilde.club)
877cvwp3rm.fsf@gnu.org
Hi Ulf,

Ulf Herrman <striness@tilde.club> skribis:

Toggle quote (17 lines)
> From 51ee63ace6f3f52eb196c990664cc6b9af3d3683 Mon Sep 17 00:00:00 2001
> From: ulfvonbelow <striness@tilde.club>
> Date: Sat, 25 Feb 2023 00:46:27 -0600
> Subject: [PATCH 2/3] service: accept fork+exec-command argument list in
> monitor.
>
> Sometimes it's necessary to run startup / shutdown programs as a certain user,
> in a certain directory, with certain environment variables, etc. Shepherd
> currently provides a replacement for system* that won't race against the
> child process auto-reaper, but this lacks the flexibility Shepherd users are
> used to.
>
> * modules/shepherd/service.scm (process-monitor): treat command instead as
> argument list to fork+exec-command.
> (spawn-via-monitor): update to new convention.
> (fork+exec+wait-command): new procedure.

On this one I took a similar approach but chose to extend
‘spawn-command’ instead of introducing a new procedure—see commit
0f3276a9c3dafbef41b0aab88ba5dda1bb78dc99.

Another difference is explicitly listing keyword arguments so that their
default values are taken from the caller’s dynamic state and not from
that of the process monitoring fiber. This fixes

Let me know what you think!

Thanks,
Ludo’.
Closed
?