[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#60566: [PATCH] environment: Fix '--emulate-fhs' option overriding $P
From: |
Ludovic Courtès |
Subject: |
bug#60566: [PATCH] environment: Fix '--emulate-fhs' option overriding $PATH. |
Date: |
Sat, 14 Jan 2023 15:41:15 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Hi John,
John Kehayias <john.kehayias@protonmail.com> skribis:
> From beb6f9255fc62fe52e237f82c7e953a21b7f82f4 Mon Sep 17 00:00:00 2001
> From: John Kehayias <john.kehayias@protonmail.com>
> Date: Thu, 5 Jan 2023 16:06:19 -0500
> Subject: [PATCH] * environment: Fix '--emulate-fhs' option overriding $PATH.
>
> Fixes <https://issues.guix.gnu.org/60566> where even if "--preserve='^PATH$'"
> was passed to 'guix shell' it would be replaced by just the FHS directories
> when '--emulate-fhs' was also set.
>
> * gnu/scripts/environment.scm (launch-environment): Add the FHS directories to
> $PATH rather than overriding $PATH completely.
> * tests/guix-environment-container.sh: Test that FHS directories are in $PATH
> in the container and that $PATH can be preserved.
[...]
> - (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") … "").
> +# 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")).
> +# 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.)
Otherwise LGTM, thanks!
Ludo’.