[lit] Add support for readfile to external shell #159431
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
|
@llvm/pr-subscribers-testing-tools Author: Aiden Grossman (boomanaiden154) ChangesThis patch adds support for the new lit %{readfile:<filename>} This code is designed to be temporary with us deleting it once Full diff: https://github.com/llvm/llvm-project/pull/159431.diff 4 Files Affected:
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 045472429b6e4..53eeb2f85b48b 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -2412,6 +2412,20 @@ def runOnce(
status, output, attempts=i + 1, max_allowed_attempts=attempts
)
+def _expandLateSubstitutionsExternal(commandLine):
+ filePaths = []
+ def _replaceReadFile(match):
+ filePath = match.group(1)
+ filePaths.append(filePath)
+ return "$(cat %s)" % filePath
+
+ commandLine = re.sub(r"%{readfile:([^}]*)}", _replaceReadFile, commandLine)
+ # Add test commands before the command to check if the file exists as
+ # cat inside a subshell will never return a non-zero exit code outside
+ # of the subshell.
+ for filePath in filePaths:
+ commandLine = "%s && test -e %s" % (commandLine, filePath)
+ return commandLine
def executeShTest(
test, litConfig, useExternalSh, extra_substitutions=[], preamble_commands=[]
@@ -2443,4 +2457,8 @@ def executeShTest(
recursion_limit=test.config.recursiveExpansionLimit,
)
+ if useExternalSh:
+ for index, command in enumerate(script):
+ script[index] = _expandLateSubstitutionsExternal(command)
+
return _runShTest(test, litConfig, useExternalSh, script, tmpBase)
diff --git a/llvm/utils/lit/tests/Inputs/shtest-readfile/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-readfile/lit.cfg
index cf453e1ea786f..ee496674fdb62 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-readfile/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-readfile/lit.cfg
@@ -1,7 +1,19 @@
+import os
+
import lit.formats
+import lit.util
config.name = "shtest-readfile"
config.suffixes = [".txt"]
-config.test_format = lit.formats.ShTest(execute_external=False)
+lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
+use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
+config.test_format = lit.formats.ShTest(execute_external=not use_lit_shell)
config.test_source_root = None
config.test_exec_root = None
+
+# If we are testing with the external shell, remove the fake-externals from
+# PATH so that we use mkdir in the tests.
+if not use_lit_shell:
+ path_parts = config.environment["PATH"].split(os.path.pathsep)
+ path_parts = [path_part for path_part in path_parts if "fake-externals" not in path_part]
+ config.environment["PATH"] = os.path.pathsep.join(path_parts)
diff --git a/llvm/utils/lit/tests/shtest-readfile-external.py b/llvm/utils/lit/tests/shtest-readfile-external.py
new file mode 100644
index 0000000000000..5825ad674ba05
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-readfile-external.py
@@ -0,0 +1,21 @@
+## Tests the readfile substitution.
+
+# RUN: env LIT_USE_INTERNAL_SHELL=0 not %{lit} -a -v %{inputs}/shtest-readfile | FileCheck -match-full-lines -DTEMP_PATH=%S/Inputs/shtest-readfile/Output %s
+
+# CHECK: -- Testing: 4 tests{{.*}}
+
+# CHECK-LABEL: FAIL: shtest-readfile :: absolute-paths.txt ({{[^)]*}})
+# CHECK: echo $(cat [[TEMP_PATH]]/absolute-paths.txt.tmp) && test -e /home/gha/llvm-project/build/utils/lit/tests/Inputs/shtest-readfile/Output/absolute-paths.txt.tmp {{.*}}
+# CHECK: + echo hello
+
+# CHECK-LABEL: FAIL: shtest-readfile :: file-does-not-exist.txt ({{[^)]*}})
+# CHECK: echo $(cat /file/does/not/exist) && test -e /file/does/not/exist {{.*}}
+# CHECK: cat: /file/does/not/exist: No such file or directory
+
+# CHECK-LABEL: FAIL: shtest-readfile :: relative-paths.txt ({{[^)]*}})
+# CHECK: echo $(cat rel_path_test_folder/test_file) && test -e rel_path_test_folder/test_file {{.*}}
+# CHECK: + echo hello
+
+# CHECK-LABEL: FAIL: shtest-readfile :: two-same-line.txt ({{[^)]*}})
+# CHECK: echo $(cat /home/gha/llvm-project/build/utils/lit/tests/Inputs/shtest-readfile/Output/two-same-line.txt.tmp.1) $(cat /home/gha/llvm-project/build/utils/lit/tests/Inputs/shtest-readfile/Output/two-same-line.txt.tmp.2) && test -e /home/gha/llvm-project/build/utils/lit/tests/Inputs/shtest-readfile/Output/two-same-line.txt.tmp.1 && test -e /home/gha/llvm-project/build/utils/lit/tests/Inputs/shtest-readfile/Output/two-same-line.txt.tmp.2 {{.*}}
+# CHECK: + echo hello bye
diff --git a/llvm/utils/lit/tests/shtest-readfile.py b/llvm/utils/lit/tests/shtest-readfile.py
index c25a643b4eff8..2200576d05394 100644
--- a/llvm/utils/lit/tests/shtest-readfile.py
+++ b/llvm/utils/lit/tests/shtest-readfile.py
@@ -1,6 +1,6 @@
## Tests the readfile substitution.
-# RUN: not %{lit} -a -v %{inputs}/shtest-readfile | FileCheck -match-full-lines -DTEMP_PATH=%S/Inputs/shtest-readfile/Output %s
+# RUN: env LIT_USE_INTERNAL_SHELL=1 not %{lit} -a -v %{inputs}/shtest-readfile | FileCheck -match-full-lines -DTEMP_PATH=%S/Inputs/shtest-readfile/Output %s
# CHECK: -- Testing: 4 tests{{.*}}
|
|
✅ With the latest revision this PR passed the Python code formatter. |
arichardson
reviewed
Sep 17, 2025
Created using spr 1.3.6 [skip ci]
boomanaiden154
changed the base branch from
users/boomanaiden154/main.lit-add-support-for-readfile-to-external-shell
to
main
cmtice
approved these changes
Sep 19, 2025
boomanaiden154
deleted the
users/boomanaiden154/lit-add-support-for-readfile-to-external-shell
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Sep 19, 2025
This patch adds support for the new lit %{readfile:<filename>}
substitution to the external shell. The implementation currently just
appends some test commands to ensure the file exists and uses a subshell
with cat. This is intended to enable running tests using the
substitution in the external shell before we fully switch over to the
internal shell.
This code is designed to be temporary with us deleting it once
everything has migrated over to the internal shell and we are able to
remove the external shell code paths.
Reviewers: petrhosek, cmtice, pogo59, ilovepi, arichardson
Reviewed By: cmtice
Pull Request: llvm/llvm-project#159431
schwitanski
pushed a commit
to RWTH-HPC/llvm-lit-mirror
that referenced
this pull request
Sep 20, 2025
This patch adds support for the new lit %{readfile:<filename>}
substitution to the external shell. The implementation currently just
appends some test commands to ensure the file exists and uses a subshell
with cat. This is intended to enable running tests using the
substitution in the external shell before we fully switch over to the
internal shell.
This code is designed to be temporary with us deleting it once
everything has migrated over to the internal shell and we are able to
remove the external shell code paths.
Reviewers: petrhosek, cmtice, pogo59, ilovepi, arichardson
Reviewed By: cmtice
Pull Request: llvm/llvm-project#159431
|
Hi, Is it only me that see I just failed for me like |
|
That looks suspiciously like a buffered output not being flushed properly somewhere, perhaps being flushed in some racy manner somewhere within the system implementation. I don't really have a concrete suggestion as to where though, nor can I be certain that is the issue. |
Yeah, I've seen this too in our builders. I'm of the same opinion as James that it seems like some buffering issue. I haven't had time to investigate. I'll hopefully get to it later today. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
This patch adds support for the new lit %{readfile:}
substitution to the external shell. The implementation currently just
appends some test commands to ensure the file exists and uses a subshell
with cat. This is intended to enable running tests using the
substitution in the external shell before we fully switch over to the
internal shell.
This code is designed to be temporary with us deleting it once
everything has migrated over to the internal shell and we are able to
remove the external shell code paths.