lib/spack/env/cc: tolerate trailing / in elements of $PATH (spack#25733)

bernhardkaindl · haampie · web-flow · commit 4e4b199f1693 · 2021-09-08T12:09:07.000+02:00
Fixes removal of SPACK_ENV_PATH from PATH in the presence of trailing
slashes in the elements of PATH:

The compiler wrapper has to ensure that it is not called nested like
it would happen when gcc's collect2 uses PATH to call the linker ld,
or else the compilation fails.

To prevent nested calls, the compiler wrapper removes the elements
of SPACK_ENV_PATH from PATH.

Sadly, the autotest framework appends a slash to each element
of PATH when adding AUTOTEST_PATH to the PATH for the tests,
and some tests like those of GNU bison run cc inside the test.

Thus, ensure that PATH cleanup works even with trailing slashes.

This fixes the autotest suite of bison, compiling hundreds of
bison-generated test cases in a autotest-generated testsuite.

Co-authored-by: Harmen Stoppels &lt;harmenstoppels@gmail.com&gt;
diff --git a/lib/spack/env/cc b/lib/spack/env/cc
@@ -247,7 +247,7 @@ export PATH=""
 for dir in "${env_path[@]}"; do
     addpath=true
     for env_dir in "${spack_env_dirs[@]}"; do
-        if [[ "$dir" == "$env_dir" ]]; then
+        if [[ "${dir%%/}" == "$env_dir" ]]; then
             addpath=false
             break
         fi
@@ -616,6 +616,9 @@ if [[ $SPACK_TEST_COMMAND == dump-args ]]; then
     IFS="
 " && echo "${full_command[*]}"
     exit
+elif [[ $SPACK_TEST_COMMAND =~ dump-env-* ]]; then
+    var=${SPACK_TEST_COMMAND#dump-env-}
+    echo "$0: $var: ${!var}"
 elif [[ -n $SPACK_TEST_COMMAND ]]; then
     die "ERROR: Unknown test command"
 fi
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
@@ -141,6 +141,17 @@ def check_args(cc, args, expected):
         assert expected == cc_modified_args
 
 
+def check_env_var(executable, var, expected):
+    """Check environment variables updated by the passed compiler wrapper
+
+    This assumes that cc will print debug output when it's environment
+    contains SPACK_TEST_COMMAND=dump-env-<variable-to-debug>
+    """
+    with set_env(SPACK_TEST_COMMAND='dump-env-' + var):
+        output = executable(*test_args, output=str).strip()
+        assert output == executable.path + ': ' + var + ': ' + expected
+
+
 def dump_mode(cc, args):
     """Make cc dump the mode it detects, and return it."""
     with set_env(SPACK_TEST_COMMAND='dump-mode'):
@@ -274,6 +285,26 @@ def test_dep_include():
             test_args_without_paths)
 
 
+def test_system_path_cleanup():
+    """Ensure SPACK_ENV_PATH is removed from PATH, even with trailing /
+
+    The compiler wrapper has to ensure that it is not called nested
+    like it would happen when gcc's collect2 looks in PATH for ld.
+
+    To prevent nested calls, the compiler wrapper removes the elements
+    of SPACK_ENV_PATH from PATH. Autotest's generated testsuite appends
+    a / to each element of PATH when adding AUTOTEST_PATH.
+    Thus, ensure that PATH cleanup works even with trailing /.
+    """
+    system_path = '/bin:/usr/bin:/usr/local/bin'
+    cc_dir = os.path.dirname(cc.path)
+    with set_env(SPACK_ENV_PATH=cc_dir, SPACK_CC='true'):
+        with set_env(PATH=cc_dir + ':' + system_path):
+            check_env_var(cc, 'PATH', system_path)
+        with set_env(PATH=cc_dir + '/:' + system_path):
+            check_env_var(cc, 'PATH', system_path)
+
+
 def test_dep_lib():
     """Ensure a single dependency RPATH is added."""
     with set_env(SPACK_LINK_DIRS='x',
