[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#60566: [PATCH] environment: Fix '--emulate-fhs' option overriding $P
From: |
John Kehayias |
Subject: |
bug#60566: [PATCH] environment: Fix '--emulate-fhs' option overriding $PATH. |
Date: |
Sun, 15 Jan 2023 23:05:13 +0000 |
Hi Ludo’,
On Sat, Jan 14, 2023 at 03:41 PM, Ludovic Courtès wrote:
> Hi John,
>
> John Kehayias <john.kehayias@protonmail.com> skribis:
>
> [...]
>
>> - (setenv "PATH" "/bin:/usr/bin:/sbin:/usr/sbin")
>> + (setenv "PATH" (string-append "/bin:/usr/bin:/sbin:/usr/sbin"
>> + (when (getenv "PATH")
>> + (string-append ":" (getenv
>> "PATH")))))
>
> Remember that ‘when’ returns *unspecified* when the condition is false,
> so you’d get a type error here when PATH is undefined.
>
> Instead write: (if (getenv "PATH") … "").
>
Ah yes, my Common Lisp showing through and relying on nil instead. Fixed and
thanks!
>> +# Test that $PATH inside the container has FHS directories.
>> +guix shell -CF --bootstrap guile-bootstrap \
>> + -- guile -c '(exit (if (string-contains (getenv "PATH")
>> + "/bin:/usr/bin:/sbin:/usr/sbin")
>> + 0
>> + 1))'
>
> Even (exit (string=? (getenv "PATH") "/bin:/usr/bin:/sbin:/usr/sbin")).
>
With this patch PATH now gets the FHS directories in addition to what it
normally has (like the profile's bin directory). While slightly redundant, this
seems to be better than clobbering it. Anyway, so we can't check that the PATH
is completely equal here.
>> +# Make sure '--preserve' is honored for $PATH, which the '--emulate-fhs'
>> +# option will modify. We can't (easily) check the whole $PATH as it will
>> +# differ inside and outside the container, so just check for an added
>> string.
>> +PATH=this-is-a-test:$PATH guix shell -CF --bootstrap guile-bootstrap -E
>> PATH \
>> + -- guile -c '(exit (if (string-contains (getenv "PATH")
>> + "this-is-a-test")
>> + 0
>> + 1))'
>
> It might be slightly more concise with ‘env’:
>
> PATH=/foo $(type -P guix) shell -E ^PATH$ -C coreutils -- env |grep
> ^PATH=.*:/foo
>
> (I think ‘--bootstrap’ doesn’t buy us much here because we have to
> download/build ‘glibc-for-fhs’ anyway. ‘--bootstrap’ and
> ‘guile-bootstrap’ are particularly useful for testse that can run
> without network access and without building tons of stuff, as in
> ‘tests/guix-environment.sh’ for instance.)
>
Ah, thanks, that is nicer if we can just use coreutils. I rewrote the previous
test to use that as well. Probably some other tests here could use that
simplification, but outside of the scope here.
(Side note that 'type' in zsh works differently, one could use 'whence' there
or even the built-in 'which'. For the tests we are running with bash or bash
compliant here, so it is not a problem.)
> Otherwise LGTM, thanks!
>
> Ludo’.
Thanks again for your careful review! Pushed as
3bfbfa2946aebb7f68c8027ae80f272f6915c94f and closing this issue.
Thanks also for jman for reporting.
John