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

From: Ludovic Courtès <[email-masked]>
Date: Sat, 28 Mar 2020 13:20:08 +0100

Hi!

Christopher Baines <[email-masked]> skribis:

> Ludovic Courtès <[email-masked]> writes:
>
>> 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”.
>
> I had a go at this, but I think trying to copy the environment variables
> from the host Guix to the inferior one caused problems, at least this
> backtrace appears when calling open-inferior/container and I'm guessing
> it comes from the inferior guix.
>
> I think calling it environment-variables and having it be '() is OK, the
> only change I can see being made elsewhere is that
> open-inferior/container adds HOME=/tmp, and that's just to avoid issues
> with (guix profiles).
>
> Does that make sense?

Ah yes, defaulting to the empty list is even better.

Thanks,
Ludo’.