guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ludovic Courtès
Subject: [bug#34638] [PATCH v2 2/4] linux-container: Add 'start-child-in-container'.
Date: Thu, 26 Mar 2020 10:28:18 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)

Christopher Baines <address@hidden> 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’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]