Subject: Re: [bug#34638] [PATCH v2 2/4] linux-container: Add 'start-child-in-container'.

From: Ludovic Courtès <[email-masked]>
Date: Thu, 26 Mar 2020 10:28:18 +0100

Christopher Baines <[email-masked]> skribis:

> This new procedure is similar to open-pipe* in (ice-9 popen), but using
> run-container from (gnu build linux-container).
>
> * gnu/build/linux-container.scm (start-child-in-container): New procedure.

[...]

> +(define* (start-child-in-container command
> +                                   #:key read? write?
> +                                   (root 'temporary)
> +                                   (mounts '())
> +                                   (namespaces %namespaces)
> +                                   (host-uids 1)
> +                                   (extra-environment-variables '()))

Please add a docstring.  :-)

I’d change (extra-environment-variables '()) to:

  (environment-variables (environ))

I always find it too hard to reason about “extra” thing; it’s just more
convenient as an interface to specify the whole thing rather than a list
of “extras”.

> +    (apply execlp command))

To provide a correct argv[0] by default, you should probably change it
to:

  (match command
    ((program arguments ...)
     (execlp program program arguments)))

(That’ll also address a comment of yours in one of the subsequent
patches.)

Could you add a test to ‘tests/containers.scm’?

Thanks,
Ludo’.