[lit] Fix shell commands with newlines #67898
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
In PR llvm#65242 (landed as 9e739fd), I claimed that RUN lines cannot contain newlines. Actually, they can after substitution expansion. More generally, a lit config file can define substitutions or preamble commands containing newlines. While both of those cases seem unlikely in practice, [D154987](https://reviews.llvm.org/D154987) proposes PYTHON directives where it seems very likely. Regardless of the use case, without this patch, such newlines break expansion of `%dbg(RUN: at line N)`, and the fix is simple.
|
@llvm/pr-subscribers-testing-tools ChangesIn PR #65242 (landed as 9e739fd), I claimed that RUN lines cannot contain newlines. Actually, they can after substitution expansion. More generally, a lit config file can define substitutions or preamble commands containing newlines. While both of those cases seem unlikely in practice, Regardless of the use case, without this patch, such newlines break expansion of Full diff: https://github.com/llvm/llvm-project/pull/67898.diff 8 Files Affected:
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 528694b07f03300..a6314e35c1a045c 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -55,7 +55,7 @@ def __init__(self, command, message):
#
# COMMAND that follows %dbg(ARG) is also captured. COMMAND can be
# empty as a result of conditinal substitution.
-kPdbgRegex = "%dbg\\(([^)'\"]*)\\)(.*)"
+kPdbgRegex = "%dbg\\(([^)'\"]*)\\)((?:.|\\n)*)"
def buildPdbgCommand(msg, cmd):
diff --git a/llvm/utils/lit/tests/Inputs/shtest-inject/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-inject/lit.cfg
index b3a86e73d21ac99..6454e22ce2145f7 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-inject/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-inject/lit.cfg
@@ -1,6 +1,7 @@
import lit
-preamble_commands = ['echo "THIS WAS"', 'echo "INJECTED"']
+# Check multiple commands, and check newlines.
+preamble_commands = ['echo "THIS WAS"', 'echo\n"INJECTED"']
config.name = "shtest-inject"
config.suffixes = [".txt"]
diff --git a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/lit.local.cfg b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/lit.local.cfg
index 4cc234df4fcaaf8..0bc85e213ec390f 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/lit.local.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/lit.local.cfg
@@ -1,3 +1,8 @@
import lit.formats
config.test_format = lit.formats.ShTest(execute_external=True)
+config.substitutions.append(("%{cmds-with-newlines}", """
+true &&
+echo abc |
+FileCheck %s
+"""))
diff --git a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/run-line-with-newline.txt b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/run-line-with-newline.txt
new file mode 100644
index 000000000000000..32be84d530419b0
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/run-line-with-newline.txt
@@ -0,0 +1,2 @@
+# RUN: %{cmds-with-newlines}
+# CHECK: bac
diff --git a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/lit.local.cfg b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/lit.local.cfg
index 3002a11ed23c5e9..960e5dc0e77ac95 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/lit.local.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/lit.local.cfg
@@ -1,3 +1,8 @@
import lit.formats
config.test_format = lit.formats.ShTest(execute_external=False)
+config.substitutions.append(("%{cmds-with-newlines}", """
+true &&
+echo abc |
+FileCheck %s
+"""))
diff --git a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/run-line-with-newline.txt b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/run-line-with-newline.txt
new file mode 100644
index 000000000000000..32be84d530419b0
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/run-line-with-newline.txt
@@ -0,0 +1,2 @@
+# RUN: %{cmds-with-newlines}
+# CHECK: bac
diff --git a/llvm/utils/lit/tests/shtest-inject.py b/llvm/utils/lit/tests/shtest-inject.py
index 3d34eb7161d462b..5ba9d2f81268f3d 100644
--- a/llvm/utils/lit/tests/shtest-inject.py
+++ b/llvm/utils/lit/tests/shtest-inject.py
@@ -14,7 +14,8 @@
# CHECK-TEST1-NEXT: # | THIS WAS
# CHECK-TEST1-NEXT: # `---{{-*}}
# CHECK-TEST1-NEXT: # preamble command line
-# CHECK-TEST1-NEXT: echo "INJECTED"
+# CHECK-TEST1-NEXT: echo
+# CHECK-TEST1-NEXT: "INJECTED"
# CHECK-TEST1-NEXT: # executed command: echo INJECTED
# CHECK-TEST1-NEXT: # .---command stdout{{-*}}
# CHECK-TEST1-NEXT: # | INJECTED
diff --git a/llvm/utils/lit/tests/shtest-run-at-line.py b/llvm/utils/lit/tests/shtest-run-at-line.py
index 18086f6fa10d650..fd729ddd8f27fba 100644
--- a/llvm/utils/lit/tests/shtest-run-at-line.py
+++ b/llvm/utils/lit/tests/shtest-run-at-line.py
@@ -7,7 +7,7 @@
# END.
-# CHECK: Testing: 6 tests
+# CHECK: Testing: 8 tests
# In the case of the external shell, we check for only RUN lines in stderr in
@@ -48,6 +48,15 @@
# CHECK-NOT: RUN
# CHECK: --
+# CHECK-LABEL: FAIL: shtest-run-at-line :: external-shell/run-line-with-newline.txt
+
+# CHECK: Command Output (stderr)
+# CHECK-NEXT: --
+# CHECK-NEXT: {{^}}RUN: at line 1: true &&
+# CHECK-NEXT: echo abc |
+# CHECK-NEXT: FileCheck {{.*}}
+# CHECK-NOT: RUN
+
# CHECK-LABEL: FAIL: shtest-run-at-line :: internal-shell/basic.txt
@@ -87,3 +96,16 @@
# CHECK-NEXT: # executed command: echo 'foo baz'
# CHECK-NEXT: # executed command: FileCheck {{.*}}
# CHECK-NOT: RUN
+
+# CHECK-LABEL: FAIL: shtest-run-at-line :: internal-shell/run-line-with-newline.txt
+
+# CHECK: Command Output (stdout)
+# CHECK-NEXT: --
+# CHECK-NEXT: # RUN: at line 1
+# CHECK-NEXT: true &&
+# CHECK-NEXT: echo abc |
+# CHECK-NEXT: FileCheck {{.*}}
+# CHECK-NEXT: # executed command: true
+# CHECK-NEXT: # executed command: echo abc
+# CHECK-NEXT: # executed command: FileCheck {{.*}}
+# CHECK-NOT: RUN
|
jdenny-ornl
requested review from
MaskRay,
yln,
Endilll,
jh7370,
asavonic,
mstorsjo and
banach-space
RoboTux
reviewed
Oct 2, 2023
| @@ -0,0 +1,2 @@ | |||
| # RUN: %{cmds-with-newlines} | |||
| # CHECK: bac | |||
There was a problem hiding this comment.
Why bac and not abc? Are you trying to make the test fail?
There was a problem hiding this comment.
Yes, that was the point. I've pushed a change to make that clearer. Thanks.
| @@ -0,0 +1,2 @@ | |||
| # RUN: %{cmds-with-newlines} | |||
| # CHECK: bac | |||
|
Thanks for the review. |
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.
In PR #65242 (landed as 9e739fd), I claimed that RUN lines cannot contain newlines. Actually, they can after substitution expansion. More generally, a lit config file can define substitutions or preamble commands containing newlines. While both of those cases seem unlikely in practice,
D154987 proposes PYTHON directives where it seems very likely.
Regardless of the use case, without this patch, such newlines break expansion of
%dbg(RUN: at line N), and the fix is simple.