[clang] [test] Skip a test that sets PATH= on Windows #95096
Conversation
|
CC @linehill |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Martin Storsjö (mstorsjo) ChangesThe same has been done in a couple other existing tests, that also are skipped on Windows (e.g. ld-path.c). Some tests that really do want to test setting the path on Windows does it differently, see e.g. ps4-ps5-linker-win.c. Since a65771f, the spirv-toolchain.cl test does one test where PATH is set. Setting PATH does work in some build configurations - however, if built with e.g. llvm-mingw, the built Clang executable depends on libc++.dll (and libunwind.dll) which are found in PATH. If the PATH is overridden, the newly built Clang executable no longer can run. Full diff: https://github.com/llvm/llvm-project/pull/95096.diff 1 Files Affected:
diff --git a/clang/test/Driver/spirv-toolchain.cl b/clang/test/Driver/spirv-toolchain.cl
index de818177cb19f..2c9f9db80cad6 100644
--- a/clang/test/Driver/spirv-toolchain.cl
+++ b/clang/test/Driver/spirv-toolchain.cl
@@ -1,3 +1,8 @@
+/// One test uses the PATH environment variable; on Windows, we may need to retain
+/// the original path for the built Clang binary to be able to execute (as it is
+/// used for locating dependent DLLs).
+// UNSUPPORTED: system-windows
+
// Check object emission.
// RUN: %clang -### --target=spirv64 -x cl -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s
// RUN: %clang -### --target=spirv64 %s 2>&1 | FileCheck --check-prefix=SPV64 %s
|
|
Could the single test with the PATH manipulation be moved to a separate test file which is disabled on Windows? That way we could keep running the other tests on Windows. |
That's certainly an option, too. |
mstorsjo
force-pushed
the
clang-test-path-windows
branch
from
1608c82 to
8e5e09f
Compare
| // RUN: mkdir -p %t/versioned | ||
| // RUN: touch %t/versioned/llvm-spirv-%llvm-version-major \ | ||
| // RUN: && chmod +x %t/versioned/llvm-spirv-%llvm-version-major | ||
| // RUN: env "PATH=%t/versioned" %clang -### --target=spirv64 -x cl -c %s 2>&1 \ |
There was a problem hiding this comment.
Instead of creating a file, consider %if !system-windows
There was a problem hiding this comment.
Ok, updated the PR to use %if !system-windows instead - tested that it does seem to work as expected here.
The same has been done in a couple other existing tests, that also are skipped on Windows (e.g. ld-path.c). Some tests that really do want to test setting the path on Windows does it differently, see e.g. ps4-ps5-linker-win.c. Since a65771f, the spirv-toolchain.cl test does one test where PATH is set. Setting PATH does work in some build configurations - however, if built with e.g. llvm-mingw, the built Clang executable depends on libc++.dll (and libunwind.dll) which are found in PATH. If the PATH is overridden, the newly built Clang executable no longer can run. Split the test that requires setting PATH to a separate file, and mark it as unsupported on Windows.
mstorsjo
force-pushed
the
clang-test-path-windows
branch
from
8e5e09f to
11b5778
Compare
)" This reverts commit f5a93c5.
)" This reverts commit f5a93c5.
The same has been done in a couple other existing tests, that also are skipped on Windows (e.g. ld-path.c). Some tests that really do want to test setting the path on Windows does it differently, see e.g. ps4-ps5-linker-win.c.
Since a65771f, the spirv-toolchain.cl test does one test where PATH is set. Setting PATH does work in some build configurations - however, if built with e.g. llvm-mingw, the built Clang executable depends on libc++.dll (and libunwind.dll) which are found in PATH. If the PATH is overridden, the newly built Clang executable no longer can run.