[Clang] Rewrite tests using subshells to set env variables #158446
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Aiden Grossman (boomanaiden154) ChangesNow that we have the %readfile substitution, we can rewrite these tests This does involve one invocation of bash so that we are using the system Full diff: https://github.com/llvm/llvm-project/pull/158446.diff 5 Files Affected:
diff --git a/clang/test/ClangScanDeps/pr61006.cppm b/clang/test/ClangScanDeps/pr61006.cppm
index f75edd38c81ba..f10bc1e673987 100644
--- a/clang/test/ClangScanDeps/pr61006.cppm
+++ b/clang/test/ClangScanDeps/pr61006.cppm
@@ -6,7 +6,8 @@
// RUN: mkdir -p %t
// RUN: split-file %s %t
//
-// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir` && \
+// RUN: %clang -print-resource-dir | tr -d '\n' > %t/resource-dir
+// RUN: env EXPECTED_RESOURCE_DIR=%{readfile:%t/resource-dir} && \
// RUN: ln -s %clang++ %t/clang++ && \
// RUN: sed "s|EXPECTED_RESOURCE_DIR|$EXPECTED_RESOURCE_DIR|g; s|DIR|%/t|g" %t/P1689.json.in > %t/P1689.json && \
// RUN: clang-scan-deps -compilation-database %t/P1689.json -format=p1689 | FileCheck %t/a.cpp -DPREFIX=%/t && \
diff --git a/clang/test/ClangScanDeps/resource_directory.c b/clang/test/ClangScanDeps/resource_directory.c
index 55d5d90bbcdea..6183e8aefacfa 100644
--- a/clang/test/ClangScanDeps/resource_directory.c
+++ b/clang/test/ClangScanDeps/resource_directory.c
@@ -12,14 +12,14 @@
// then verify `%clang-scan-deps` arrives at the same path by calling the
// `Driver::GetResourcesPath` function.
//
-// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir`
+// RUN: %clang -print-resource-dir | tr -d '\n' > %t/resource-dir
// RUN: sed -e "s|CLANG|%clang|g" -e "s|DIR|%/t|g" \
// RUN: %S/Inputs/resource_directory/cdb.json.template > %t/cdb_path.json
//
// RUN: clang-scan-deps -compilation-database %t/cdb_path.json --format experimental-full \
// RUN: --resource-dir-recipe modify-compiler-path > %t/result_path.json
// RUN: cat %t/result_path.json | sed 's:\\\\\?:/:g' \
-// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="$EXPECTED_RESOURCE_DIR"
+// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="%{readfile:%t/resource-dir}"
// CHECK-PATH: "-resource-dir"
// CHECK-PATH-NEXT: "[[EXPECTED_RESOURCE_DIR]]"
@@ -31,9 +31,8 @@
// Here we hard-code the expected path into `%t/compiler` and then verify
// `%clang-scan-deps` arrives at the path by actually running the executable.
//
-// RUN: EXPECTED_RESOURCE_DIR="/custom/compiler/resources"
// RUN: echo "#!/bin/sh" > %t/compiler
-// RUN: echo "echo '$EXPECTED_RESOURCE_DIR'" >> %t/compiler
+// RUN: echo "echo '/custom/compiler/resources'" >> %t/compiler
// RUN: chmod +x %t/compiler
// RUN: sed -e "s|CLANG|%/t/compiler|g" -e "s|DIR|%/t|g" \
// RUN: %S/Inputs/resource_directory/cdb.json.template > %t/cdb_invocation.json
@@ -41,6 +40,6 @@
// RUN: clang-scan-deps -compilation-database %t/cdb_invocation.json --format experimental-full \
// RUN: --resource-dir-recipe invoke-compiler > %t/result_invocation.json
// RUN: cat %t/result_invocation.json | sed 's:\\\\\?:/:g' \
-// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="$EXPECTED_RESOURCE_DIR"
+// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="/custom/compiler/resources"
// CHECK-INVOCATION: "-resource-dir"
// CHECK-INVOCATION-NEXT: "[[EXPECTED_RESOURCE_DIR]]"
diff --git a/clang/test/Driver/env.c b/clang/test/Driver/env.c
index b3345ae09ffef..68ded45385702 100644
--- a/clang/test/Driver/env.c
+++ b/clang/test/Driver/env.c
@@ -1,13 +1,14 @@
// Some assertions in this test use Linux style (/) file paths.
// UNSUPPORTED: system-windows
+// RUN: bash -c env | grep LD_LIBRARY_PATH | tr -d '\n' > /tmp/ld_library_path
// The PATH variable is heavily used when trying to find a linker.
-// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="$LD_LIBRARY_PATH" CLANG_NO_DEFAULT_CONFIG=1 \
+// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="%{readfile:/tmp/ld_library_path}" CLANG_NO_DEFAULT_CONFIG=1 \
// RUN: %clang %s -### -o %t.o --target=i386-unknown-linux \
// RUN: --sysroot=%S/Inputs/basic_linux_tree \
// RUN: --rtlib=platform --unwindlib=platform -no-pie \
// RUN: 2>&1 | FileCheck --check-prefix=CHECK-LD-32 %s
//
-// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="$LD_LIBRARY_PATH" CLANG_NO_DEFAULT_CONFIG=1 \
+// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="%{readfile:/tmp/ld_library_path}" CLANG_NO_DEFAULT_CONFIG=1 \
// RUN: %clang %s -### -o %t.o --target=i386-unknown-linux \
// RUN: --sysroot=%S/Inputs/basic_linux_tree \
// RUN: --rtlib=platform --unwindlib=platform -no-pie \
diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c
index b88c0f29f7f33..bb434482f90d4 100644
--- a/clang/test/Driver/program-path-priority.c
+++ b/clang/test/Driver/program-path-priority.c
@@ -87,8 +87,8 @@
/// <default-triple>-gcc has lowest priority so <triple>-gcc
/// on PATH beats default triple in program path
-// RUN: DEFAULT_TRIPLE=`%t/clang --version | grep "Target:" | cut -d ' ' -f2`
-// RUN: touch %t/$DEFAULT_TRIPLE-gcc && chmod +x %t/$DEFAULT_TRIPLE-gcc
+// RUN: %t/clang --version | grep "Target:" | cut -d ' ' -f2 > %t.default_triple
+// RUN: touch %t/%{readfile:%t.default_triple}-gcc && chmod +x %t/%{readfile:%t.default_triple}-gcc
// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_triple-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
@@ -101,7 +101,7 @@
// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc"
// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc"
-/// Pick "gcc" as a fallback. Don't pick $DEFAULT_TRIPLE-gcc.
+/// Pick "gcc" as a fallback. Don't pick DEFAULT_TRIPLE-gcc.
// RUN: rm %t/env/gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
@@ -110,9 +110,9 @@
/// -B paths are searched separately so default triple will win
/// if put in one of those even if other paths have higher priority names
// RUN: mkdir -p %t/prefix
-/// One of these will fail when $DEFAULT_TRIPLE == %target_triple
-// RUN: test -f %t/$DEFAULT_TRIPLE-gcc && \
-// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix || true
+/// One of these will fail when %{readfile:%t.default_triple} == %target_triple
+// RUN: test -f %t/%{readfile:%t.default_triple}-gcc && \
+// RUN: mv %t/%{readfile:%t.default_triple}-gcc %t/prefix || true
// RUN: test -f %t/%target_triple-gcc && \
// RUN: mv %t/%target_triple-gcc %t/prefix || true
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
@@ -123,8 +123,8 @@
// DEFAULT_TRIPLE_IN_PREFIX-NOT: notreal-none-elf-gcc"
/// Only if there is nothing in the prefix will we search other paths
-/// -f in case $DEFAULT_TRIPLE == %target_triple
-// RUN: rm -f %t/prefix/$DEFAULT_TRIPLE-gcc %t/prefix/%target_triple-gcc %t/prefix/gcc
+/// -f in case %{readfile:%t.default_triple} == %target_triple
+// RUN: rm -f %t/prefix/%{readfile:%t.default_triple}-gcc %t/prefix/%target_triple-gcc %t/prefix/gcc
// RUN: env "PATH=" %t/clang -### -canonical-prefixes --target=notreal-none-elf %s -B %t/prefix 2>&1 | \
// RUN: FileCheck --check-prefix=EMPTY_PREFIX_DIR1 %s
// EMPTY_PREFIX_DIR1: gcc"
diff --git a/clang/test/Modules/relative-resource-dir.m b/clang/test/Modules/relative-resource-dir.m
index 4f88150a2ba4c..96f2d8efc7860 100644
--- a/clang/test/Modules/relative-resource-dir.m
+++ b/clang/test/Modules/relative-resource-dir.m
@@ -1,9 +1,9 @@
// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
-// REQUIRES: shell
-// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir` && \
+// RUN: %clang -print-resource-dir | tr -d '\n' > %t.resource_dir
+// RUN: env EXPECTED_RESOURCE_DIR="%{readfile:%t.resource_dir}" && \
// RUN: mkdir -p %t && rm -rf %t/resource-dir && \
-// RUN: cp -R $EXPECTED_RESOURCE_DIR %t/resource-dir
+// RUN: cp -R %{readfile:%t.resource_dir} %t/resource-dir
// RUN: cd %t && %clang -cc1 -x objective-c -fmodules -fmodule-format=obj \
// RUN: -fimplicit-module-maps -fmodules-cache-path=%t.mcp \
// RUN: -fbuiltin-headers-in-system-modules \
|
Member
|
@llvm/pr-subscribers-clang-modules Author: Aiden Grossman (boomanaiden154) ChangesNow that we have the %readfile substitution, we can rewrite these tests This does involve one invocation of bash so that we are using the system Full diff: https://github.com/llvm/llvm-project/pull/158446.diff 5 Files Affected:
diff --git a/clang/test/ClangScanDeps/pr61006.cppm b/clang/test/ClangScanDeps/pr61006.cppm
index f75edd38c81ba..f10bc1e673987 100644
--- a/clang/test/ClangScanDeps/pr61006.cppm
+++ b/clang/test/ClangScanDeps/pr61006.cppm
@@ -6,7 +6,8 @@
// RUN: mkdir -p %t
// RUN: split-file %s %t
//
-// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir` && \
+// RUN: %clang -print-resource-dir | tr -d '\n' > %t/resource-dir
+// RUN: env EXPECTED_RESOURCE_DIR=%{readfile:%t/resource-dir} && \
// RUN: ln -s %clang++ %t/clang++ && \
// RUN: sed "s|EXPECTED_RESOURCE_DIR|$EXPECTED_RESOURCE_DIR|g; s|DIR|%/t|g" %t/P1689.json.in > %t/P1689.json && \
// RUN: clang-scan-deps -compilation-database %t/P1689.json -format=p1689 | FileCheck %t/a.cpp -DPREFIX=%/t && \
diff --git a/clang/test/ClangScanDeps/resource_directory.c b/clang/test/ClangScanDeps/resource_directory.c
index 55d5d90bbcdea..6183e8aefacfa 100644
--- a/clang/test/ClangScanDeps/resource_directory.c
+++ b/clang/test/ClangScanDeps/resource_directory.c
@@ -12,14 +12,14 @@
// then verify `%clang-scan-deps` arrives at the same path by calling the
// `Driver::GetResourcesPath` function.
//
-// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir`
+// RUN: %clang -print-resource-dir | tr -d '\n' > %t/resource-dir
// RUN: sed -e "s|CLANG|%clang|g" -e "s|DIR|%/t|g" \
// RUN: %S/Inputs/resource_directory/cdb.json.template > %t/cdb_path.json
//
// RUN: clang-scan-deps -compilation-database %t/cdb_path.json --format experimental-full \
// RUN: --resource-dir-recipe modify-compiler-path > %t/result_path.json
// RUN: cat %t/result_path.json | sed 's:\\\\\?:/:g' \
-// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="$EXPECTED_RESOURCE_DIR"
+// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="%{readfile:%t/resource-dir}"
// CHECK-PATH: "-resource-dir"
// CHECK-PATH-NEXT: "[[EXPECTED_RESOURCE_DIR]]"
@@ -31,9 +31,8 @@
// Here we hard-code the expected path into `%t/compiler` and then verify
// `%clang-scan-deps` arrives at the path by actually running the executable.
//
-// RUN: EXPECTED_RESOURCE_DIR="/custom/compiler/resources"
// RUN: echo "#!/bin/sh" > %t/compiler
-// RUN: echo "echo '$EXPECTED_RESOURCE_DIR'" >> %t/compiler
+// RUN: echo "echo '/custom/compiler/resources'" >> %t/compiler
// RUN: chmod +x %t/compiler
// RUN: sed -e "s|CLANG|%/t/compiler|g" -e "s|DIR|%/t|g" \
// RUN: %S/Inputs/resource_directory/cdb.json.template > %t/cdb_invocation.json
@@ -41,6 +40,6 @@
// RUN: clang-scan-deps -compilation-database %t/cdb_invocation.json --format experimental-full \
// RUN: --resource-dir-recipe invoke-compiler > %t/result_invocation.json
// RUN: cat %t/result_invocation.json | sed 's:\\\\\?:/:g' \
-// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="$EXPECTED_RESOURCE_DIR"
+// RUN: | FileCheck %s --check-prefix=CHECK-PATH -DEXPECTED_RESOURCE_DIR="/custom/compiler/resources"
// CHECK-INVOCATION: "-resource-dir"
// CHECK-INVOCATION-NEXT: "[[EXPECTED_RESOURCE_DIR]]"
diff --git a/clang/test/Driver/env.c b/clang/test/Driver/env.c
index b3345ae09ffef..68ded45385702 100644
--- a/clang/test/Driver/env.c
+++ b/clang/test/Driver/env.c
@@ -1,13 +1,14 @@
// Some assertions in this test use Linux style (/) file paths.
// UNSUPPORTED: system-windows
+// RUN: bash -c env | grep LD_LIBRARY_PATH | tr -d '\n' > /tmp/ld_library_path
// The PATH variable is heavily used when trying to find a linker.
-// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="$LD_LIBRARY_PATH" CLANG_NO_DEFAULT_CONFIG=1 \
+// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="%{readfile:/tmp/ld_library_path}" CLANG_NO_DEFAULT_CONFIG=1 \
// RUN: %clang %s -### -o %t.o --target=i386-unknown-linux \
// RUN: --sysroot=%S/Inputs/basic_linux_tree \
// RUN: --rtlib=platform --unwindlib=platform -no-pie \
// RUN: 2>&1 | FileCheck --check-prefix=CHECK-LD-32 %s
//
-// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="$LD_LIBRARY_PATH" CLANG_NO_DEFAULT_CONFIG=1 \
+// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="%{readfile:/tmp/ld_library_path}" CLANG_NO_DEFAULT_CONFIG=1 \
// RUN: %clang %s -### -o %t.o --target=i386-unknown-linux \
// RUN: --sysroot=%S/Inputs/basic_linux_tree \
// RUN: --rtlib=platform --unwindlib=platform -no-pie \
diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c
index b88c0f29f7f33..bb434482f90d4 100644
--- a/clang/test/Driver/program-path-priority.c
+++ b/clang/test/Driver/program-path-priority.c
@@ -87,8 +87,8 @@
/// <default-triple>-gcc has lowest priority so <triple>-gcc
/// on PATH beats default triple in program path
-// RUN: DEFAULT_TRIPLE=`%t/clang --version | grep "Target:" | cut -d ' ' -f2`
-// RUN: touch %t/$DEFAULT_TRIPLE-gcc && chmod +x %t/$DEFAULT_TRIPLE-gcc
+// RUN: %t/clang --version | grep "Target:" | cut -d ' ' -f2 > %t.default_triple
+// RUN: touch %t/%{readfile:%t.default_triple}-gcc && chmod +x %t/%{readfile:%t.default_triple}-gcc
// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_triple-gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
@@ -101,7 +101,7 @@
// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc"
// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc"
-/// Pick "gcc" as a fallback. Don't pick $DEFAULT_TRIPLE-gcc.
+/// Pick "gcc" as a fallback. Don't pick DEFAULT_TRIPLE-gcc.
// RUN: rm %t/env/gcc
// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
@@ -110,9 +110,9 @@
/// -B paths are searched separately so default triple will win
/// if put in one of those even if other paths have higher priority names
// RUN: mkdir -p %t/prefix
-/// One of these will fail when $DEFAULT_TRIPLE == %target_triple
-// RUN: test -f %t/$DEFAULT_TRIPLE-gcc && \
-// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix || true
+/// One of these will fail when %{readfile:%t.default_triple} == %target_triple
+// RUN: test -f %t/%{readfile:%t.default_triple}-gcc && \
+// RUN: mv %t/%{readfile:%t.default_triple}-gcc %t/prefix || true
// RUN: test -f %t/%target_triple-gcc && \
// RUN: mv %t/%target_triple-gcc %t/prefix || true
// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
@@ -123,8 +123,8 @@
// DEFAULT_TRIPLE_IN_PREFIX-NOT: notreal-none-elf-gcc"
/// Only if there is nothing in the prefix will we search other paths
-/// -f in case $DEFAULT_TRIPLE == %target_triple
-// RUN: rm -f %t/prefix/$DEFAULT_TRIPLE-gcc %t/prefix/%target_triple-gcc %t/prefix/gcc
+/// -f in case %{readfile:%t.default_triple} == %target_triple
+// RUN: rm -f %t/prefix/%{readfile:%t.default_triple}-gcc %t/prefix/%target_triple-gcc %t/prefix/gcc
// RUN: env "PATH=" %t/clang -### -canonical-prefixes --target=notreal-none-elf %s -B %t/prefix 2>&1 | \
// RUN: FileCheck --check-prefix=EMPTY_PREFIX_DIR1 %s
// EMPTY_PREFIX_DIR1: gcc"
diff --git a/clang/test/Modules/relative-resource-dir.m b/clang/test/Modules/relative-resource-dir.m
index 4f88150a2ba4c..96f2d8efc7860 100644
--- a/clang/test/Modules/relative-resource-dir.m
+++ b/clang/test/Modules/relative-resource-dir.m
@@ -1,9 +1,9 @@
// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
-// REQUIRES: shell
-// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir` && \
+// RUN: %clang -print-resource-dir | tr -d '\n' > %t.resource_dir
+// RUN: env EXPECTED_RESOURCE_DIR="%{readfile:%t.resource_dir}" && \
// RUN: mkdir -p %t && rm -rf %t/resource-dir && \
-// RUN: cp -R $EXPECTED_RESOURCE_DIR %t/resource-dir
+// RUN: cp -R %{readfile:%t.resource_dir} %t/resource-dir
// RUN: cd %t && %clang -cc1 -x objective-c -fmodules -fmodule-format=obj \
// RUN: -fimplicit-module-maps -fmodules-cache-path=%t.mcp \
// RUN: -fbuiltin-headers-in-system-modules \
|
ilovepi
approved these changes
Sep 15, 2025
Created using spr 1.3.6 [skip ci]
Created using spr 1.3.6 [skip ci]
cmtice
approved these changes
Sep 17, 2025
Created using spr 1.3.6 [skip ci]
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Sep 19, 2025
Now that we have the %readfile substitution, we can rewrite these tests that were using env variable subshells to write the output of the command into a file and then load it where it is needed using readfile. This does involve one invocation of bash so that we are using the system env binary, which does support redirection into a tool like grep. We already do this in one LLVM test. I'm not happy about needing that, but the more principled way to solve it involves reworking how in-process builtins work within lit. That is something we want to do eventually, but not something that I think should block this patch. Pull Request: llvm#158446
boomanaiden154
changed the base branch from
users/boomanaiden154/main.clang-rewrite-tests-using-subshells-to-set-env-variables
to
main
boomanaiden154
deleted the
users/boomanaiden154/clang-rewrite-tests-using-subshells-to-set-env-variables
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Sep 20, 2025
Now that we have the %readfile substitution, we can rewrite these tests that were using env variable subshells to write the output of the command into a file and then load it where it is needed using readfile. This does involve one invocation of bash so that we are using the system env binary, which does support redirection into a tool like grep. We already do this in one LLVM test. I'm not happy about needing that, but the more principled way to solve it involves reworking how in-process builtins work within lit. That is something we want to do eventually, but not something that I think should block this patch. Reviewers: cmtice, petrhosek, ilovepi Reviewed By: cmtice, ilovepi Pull Request: llvm/llvm-project#158446
| @@ -1,13 +1,14 @@ | |||
| // Some assertions in this test use Linux style (/) file paths. | |||
| // UNSUPPORTED: system-windows | |||
| // RUN: bash -c env | grep LD_LIBRARY_PATH | tr -d '\n' > /tmp/ld_library_path | |||
Collaborator
There was a problem hiding this comment.
What if several users run this lit test at the same time on the same machine? If unlucky they will all try to read/write to /tmp/ld_library_path at the same time? Or what If I've run the lit test at some point and created the file and then another user tries to create it on the same machine and can't overwrite it?
I've already seen
02:11:18 bash -c env | grep LD_LIBRARY_PATH | tr -d '\n' > /tmp/ld_library_path # RUN: at line 3
02:11:18 + bash -c env
02:11:18 + tr -d '\n'
02:11:18 /repo/llvm/build/tools/clang/test/Driver/Output/env.c.script: line 1: /tmp/ld_library_path: Permission denied
in some downstream buildbots that I imagine is due to these kind of problems.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that we have the %readfile substitution, we can rewrite these tests
that were using env variable subshells to write the output of the
command into a file and then load it where it is needed using readfile.
This does involve one invocation of bash so that we are using the system
env binary, which does support redirection into a tool like grep. We
already do this in one LLVM test. I'm not happy about needing that, but
the more principled way to solve it involves reworking how in-process
builtins work within lit. That is something we want to do eventually,
but not something that I think should block this patch.