[libcxx-commits] [PATCH] D148324: [libcxx] [test] Prepend to PATH instead of overriding it

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo created this revision.
Herald added a subscriber: arichardson.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: libc++.
Herald added a reviewer: libc++.

On Windows, the PATH env variable is used for locating dynamically
linked librarys, akin to LD_LIBRARY_PATH on Linux.

The tests that run with a dynamically linked libc++ used "--env
PATH=%{lib}" in the test config. This had the unfortunate side effect
of making other tools available in PATH during the runtime of the tests;
in particular, it caused the "executor-has-no-bash" flag to be set for
all those Windows test configs (with the clang-cl static config being
the only one lacking it).

Thus, this increases the number of tests actually included in the
clang-cl dll and all mingw test configs by 9 tests.

The clang-cl static test configuration has been executing those tests
since the "--env PATH=%{lib}" was removed from that test config in
e78223e79efc886ef6f0ea5413deab3737d6d63b <https://reviews.llvm.org/rGe78223e79efc886ef6f0ea5413deab3737d6d63b>. (For mingw we haven't had a
need to split the test config between shared and static, which means
that the mingw static test config previously ran with --env PATH
needlessly.)

This increases the test coverage for patches like D146398 <https://reviews.llvm.org/D146398> which
can't be executed in the executor-has-no-bash configs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148324

Files:
  libcxx/test/configs/llvm-libc++-mingw.cfg.in
  libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
  libcxx/test/configs/llvm-libc++-shared-no-vcruntime-clangcl.cfg.in
  libcxx/utils/run.py


Index: libcxx/utils/run.py
===================================================================
--- libcxx/utils/run.py
+++ libcxx/utils/run.py
@@ -24,6 +24,7 @@
     parser.add_argument('--execdir', type=str, required=True)
     parser.add_argument('--codesign_identity', type=str, required=False, default=None)
     parser.add_argument('--env', type=str, nargs='*', required=False, default=dict())
+    parser.add_argument('--prepend_env', type=str, nargs='*', required=False, default=dict())
     parser.add_argument("command", nargs=argparse.ONE_OR_MORE)
     args = parser.parse_args()
     commandLine = args.command
@@ -43,6 +44,14 @@
 
     # Extract environment variables into a dictionary
     env = {k : v  for (k, v) in map(lambda s: s.split('=', 1), args.env)}
+
+    # Set environment variables where we prepend the given value to the
+    # existing environment variable.
+    for (k, v) in map(lambda s: s.split('=', 1), args.prepend_env):
+        if k in os.environ:
+            v = v + os.pathsep + os.environ[k]
+        env[k] = v
+
     if platform.system() == 'Windows':
         # Pass some extra variables through on Windows:
         # COMSPEC is needed for running subprocesses via std::system().
Index: libcxx/test/configs/llvm-libc++-shared-no-vcruntime-clangcl.cfg.in
===================================================================
--- libcxx/test/configs/llvm-libc++-shared-no-vcruntime-clangcl.cfg.in
+++ libcxx/test/configs/llvm-libc++-shared-no-vcruntime-clangcl.cfg.in
@@ -12,7 +12,7 @@
     '-nostdlib -L %{lib} -lc++ -lmsvcrt -lmsvcprt -loldnames'
 ))
 config.substitutions.append(('%{exec}',
-    '%{executor} --execdir %T --env PATH=%{lib} -- '
+    '%{executor} --execdir %T --prepend_env PATH=%{lib} -- '
 ))
 
 import os, site
Index: libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
===================================================================
--- libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
+++ libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
@@ -11,7 +11,7 @@
     '-nostdlib -L %{lib} -lc++ -lmsvcrt -lmsvcprt -loldnames'
 ))
 config.substitutions.append(('%{exec}',
-    '%{executor} --execdir %T --env PATH=%{lib} -- '
+    '%{executor} --execdir %T --prepend_env PATH=%{lib} -- '
 ))
 
 import os, site
Index: libcxx/test/configs/llvm-libc++-mingw.cfg.in
===================================================================
--- libcxx/test/configs/llvm-libc++-mingw.cfg.in
+++ libcxx/test/configs/llvm-libc++-mingw.cfg.in
@@ -11,7 +11,7 @@
     '-nostdlib++ -L %{lib} -lc++'
 ))
 config.substitutions.append(('%{exec}',
-    '%{executor} --execdir %T --env PATH=%{lib} -- '
+    '%{executor} --execdir %T --prepend_env PATH=%{lib} -- '
 ))
 
 import os, site


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148324.513530.patch
Type: text/x-patch
Size: 2738 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230414/74f15fc0/attachment.bin>