bug#60566: [PATCH] environment: Fix '--emulate-fhs' option overriding $PATH.

[John Kehayias via Bug reports for GNU Guix <bug-guix@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1522 bytes --]

Hi Ludo’,

On Sat, Jan 07, 2023 at 12:03 AM, Ludovic Courtès wrote:

> To be safe, you need to account for (getenv "PATH") returning #f, and
> not add a trailing colon in that case.
>

Ah, right. I think this would only happen if somehow unsetting PATH and preserving it? As 'guix shell' already sets PATH. Anyway, better to be safe here.

I tweaked this, though not sure if there is a more elegant way to construct the string than what I did (suggestions always welcome!).

> Other than that, I agree this is a valid change because that would be
> consistent with:
>
> $ PATH=/foo $(type -P guix) shell -E ^PATH$ -C coreutils -- env |grep ^PATH
> PATH=/gnu/store/pfl0lyqbs557khv7rw90bzp24qp2lqsn-profile/bin:/foo
>
> Perhaps you can add a line to test it in
> ‘tests/guix-environment-container.sh’?
>

I added two tests while I was at it: one to check that PATH has the FHS modification in the container and a second for this particular bug. For the second one I just used a test string added to PATH as the entire thing will differ already from inside/outside the container, FHS or not. I checked the tests pass here and removing '--emulate-fhs' causes the first to fail while removing the '--preserve' argument causes the second test to fail. I could separate the first out as a separate commit if that makes more sense, but I do think the current behavior is just wrong in overwriting all of PATH when '--emuate-fhs' is given.

New version attached, thanks for the suggestions!

John

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-environment-Fix-emulate-fhs-option-overriding-PATH.patch --]
[-- Type: text/x-patch; name=0001-environment-Fix-emulate-fhs-option-overriding-PATH.patch, Size: 3838 bytes --]

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.
---
 guix/scripts/environment.scm        |  8 +++++---
 tests/guix-environment-container.sh | 18 +++++++++++++++++-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index c7fd8fd340..19ba2f7bee 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -2,7 +2,7 @@
 ;;; Copyright © 2014, 2015, 2018 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015-2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Mike Gerwitz <mtg@gnu.org>
-;;; Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
+;;; Copyright © 2022, 2023 John Kehayias <john.kehayias@protonmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -475,10 +475,12 @@ (define* (launch-environment command profile manifest
      (catch 'system-error
        (lambda ()
          (when emulate-fhs?
-           ;; When running in a container with EMULATE-FHS?, override $PATH
+           ;; When running in a container with EMULATE-FHS?, augment $PATH
            ;; (optional, but to better match FHS expectations), and generate
            ;; /etc/ld.so.cache.
-           (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")))))
            (invoke "ldconfig" "-X"))
          (apply execlp program program args))
        (lambda _
diff --git a/tests/guix-environment-container.sh b/tests/guix-environment-container.sh
index 0306fc1744..198352c1e2 100644
--- a/tests/guix-environment-container.sh
+++ b/tests/guix-environment-container.sh
@@ -1,6 +1,6 @@
 # GNU Guix --- Functional package management for GNU
 # Copyright © 2015 David Thompson <davet@gnu.org>
-# Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
+# Copyright © 2022, 2023 John Kehayias <john.kehayias@protonmail.com>
 #
 # This file is part of GNU Guix.
 #
@@ -242,6 +242,22 @@ guix shell -CF --bootstrap guile-bootstrap glibc \
                            0
                            1))'
 
+# 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))'
+
+# 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))'
+
 # '--symlink' works.
 echo "TESTING SYMLINK IN CONTAINER"
 guix shell --bootstrap guile-bootstrap --container \
-- 
2.38.1