[llvm] [lit] Are all RUN lines skipped in windows cmd? (PR #65242)
Joel E. Denny via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 3 20:41:20 PDT 2023
https://github.com/jdenny-ornl created https://github.com/llvm/llvm-project/pull/65242:
Key issue
---------
When lit is configured to use windows `cmd` as an external shell, it appears that all RUN lines are effectively commented out. The problematic change landed in July 26, 2023, so it's surprising it hasn't come up yet. I'm afraid I don't have a windows setup, so I could use some help to verify what's happening.
Details
-------
In the case that lit is configured to use an external shell, D154280 (landed in 64d19542e78a on July 26, 2023) causes
`%dbg(RUN: at line N)` to be expanded in RUN lines early and in a manner that is specific to sh-like shells. As a result, later code in lit that expands it in a shell-specific manner is useless.
That sh-like expansion uses the `:` command as follows:
```
: 'RUN: at line N'; original-commands
```
In sh-like shells, the `:` command simply discards its arguments. However, in windows `cmd`, `:` indicates a goto label. That appears to effectively comment out the rest of the line, including the original commands from the RUN line.
I am not aware of any complaints about this change. Did I miss them? Are all tests still passing and so no one noticed? Lit's own test suite has some tests that normally fail if RUN lines don't execute. Is no one running lit's test suite with windows `cmd` as a lit external shell? Or is no one using windows `cmd` as a lit external shell at all anymore?
Another issue
-------------
D154280 doesn't implement `--per-test-coverage` for lit's internal shell.
Fix
---
This patch fixes the above problems by implementing `--per-test-coverage` before selecting the shell (internal or external) and by leaving `%dbg(RUN: at line N)` unexpanded. Thus, it is expanded later in a shell-specific manner, as before D154280.
I would like to understand whether windows `cmd` as a lit external shell is worthwhile to support anymore.
>From 26d5891879583b9addd2a6d4d7caf4241ba55b85 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.ornl at gmail.com>
Date: Thu, 31 Aug 2023 20:24:51 -0400
Subject: [PATCH] [lit] Are all RUN lines skipped in windows cmd?
Key issue
---------
When lit is configured to use windows `cmd` as an external shell, it
appears that all RUN lines are effectively commented out. The
problematic change landed in July 26, 2023, so it's surprising it
hasn't come up yet. I'm afraid I don't have a windows setup, so I
could use some help to verify what's happening.
Details
-------
In the case that lit is configured to use an external shell, D154280
(landed in 64d19542e78a on July 26, 2023) causes
`%dbg(RUN: at line N)` to be expanded in RUN lines early and in a
manner that is specific to sh-like shells. As a result, later code in
lit that expands it in a shell-specific manner is useless.
That sh-like expansion uses the `:` command as follows:
```
: 'RUN: at line N'; original-commands
```
In sh-like shells, the `:` command simply discards its arguments.
However, in windows `cmd`, `:` indicates a goto label. That appears
to effectively comment out the rest of the line, including the
original commands from the RUN line.
I am not aware of any complaints about this change. Did I miss them?
Are all tests still passing and so no one noticed? Lit's own test
suite has some tests that normally fail if RUN lines don't execute.
Is no one running lit's test suite with windows `cmd` as a lit
external shell? Or is no one using windows `cmd` as a lit external
shell at all anymore?
Another issue
-------------
D154280 doesn't implement `--per-test-coverage` for lit's internal
shell.
Fix
---
This patch fixes the above problems by implementing
`--per-test-coverage` before selecting the shell (internal or
external) and by leaving `%dbg(RUN: at line N)` unexpanded. Thus, it
is expanded later in a shell-specific manner, as before D154280.
I would like to understand whether windows `cmd` as a lit external
shell is worthwhile to support anymore.
---
llvm/utils/lit/lit/TestRunner.py | 44 ++++++++++++-------
.../per-test-coverage-by-lit-cfg/lit.cfg | 4 +-
.../per-test-coverage-by-lit-cfg.py | 5 ++-
.../tests/Inputs/per-test-coverage/lit.cfg | 5 ++-
.../per-test-coverage/per-test-coverage.py | 5 ++-
.../lit/tests/per-test-coverage-by-lit-cfg.py | 9 +++-
llvm/utils/lit/tests/per-test-coverage.py | 9 +++-
7 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 461cf63d6b9685..d564d6150a04a8 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1144,25 +1144,10 @@ def executeScriptInternal(test, litConfig, tmpBase, commands, cwd,
def executeScript(test, litConfig, tmpBase, commands, cwd):
bashPath = litConfig.getBashPath()
isWin32CMDEXE = litConfig.isWindows and not bashPath
- coverage_index = 0 # Counter for coverage file index
script = tmpBase + ".script"
if isWin32CMDEXE:
script += ".bat"
- # Set unique LLVM_PROFILE_FILE for each run command
- for j, ln in enumerate(commands):
- match = re.match(kPdbgRegex, ln)
- if match:
- command = match.group(2)
- commands[j] = match.expand(": '\\1'; \\2" if command else ": '\\1'")
- if litConfig.per_test_coverage:
- # Extract the test case name from the test object
- test_case_name = test.path_in_suite[-1]
- test_case_name = test_case_name.rsplit(".", 1)[0] # Remove the file extension
- llvm_profile_file = f"{test_case_name}{coverage_index}.profraw"
- commands[j] = f"export LLVM_PROFILE_FILE={llvm_profile_file} && {commands[j]}"
- coverage_index += 1
-
# Write script file
mode = "w"
open_kwargs = {}
@@ -2115,10 +2100,35 @@ def parseIntegratedTestScript(test, additional_parsers=[], require_script=True):
def _runShTest(test, litConfig, useExternalSh, script, tmpBase):
def runOnce(execdir):
+ # Set unique LLVM_PROFILE_FILE for each run command
+ if litConfig.per_test_coverage:
+ # Extract the test case name from the test object, and remove the
+ # file extension.
+ test_case_name = test.path_in_suite[-1]
+ test_case_name = test_case_name.rsplit(".", 1)[0]
+ coverage_index = 0 # Counter for coverage file index
+ scriptProf = []
+ for ln in script:
+ match = re.match(kPdbgRegex, ln)
+ if match:
+ dbg = match.group(1)
+ command = match.group(2)
+ else:
+ command = ln
+ profile = f"{test_case_name}{coverage_index}.profraw"
+ coverage_index += 1
+ command = f"export LLVM_PROFILE_FILE={profile} && {command}"
+ if match:
+ command = buildPdbgCommand(dbg, command)
+ scriptProf.append(command)
+ else:
+ scriptProf = script
+
if useExternalSh:
- res = executeScript(test, litConfig, tmpBase, script, execdir)
+ res = executeScript(test, litConfig, tmpBase, scriptProf, execdir)
else:
- res = executeScriptInternal(test, litConfig, tmpBase, script, execdir)
+ res = executeScriptInternal(test, litConfig, tmpBase, scriptProf,
+ execdir)
if isinstance(res, lit.Test.Result):
return res
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg
index 186c81ebd17283..710a9220464ac0 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/lit.cfg
@@ -3,6 +3,8 @@ import os
config.name = "per-test-coverage-by-lit-cfg"
config.suffixes = [".py"]
-config.test_format = lit.formats.ShTest(execute_external=True)
+config.test_format = lit.formats.ShTest(
+ execute_external=eval(lit_config.params.get("execute_external"))
+)
lit_config.per_test_coverage = True
config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
index 0abe00bdedb1f2..349bbafab2e1b6 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py
@@ -1,5 +1,6 @@
# Check that the environment variable is set correctly
-# RUN: %{python} %s | FileCheck %s
+# RUN: %{python} %s | FileCheck -DINDEX=0 %s
+# RUN: %{python} %s | FileCheck -DINDEX=1 %s
# Python script to read the environment variable
# and print its value
@@ -8,4 +9,4 @@
llvm_profile_file = os.environ.get('LLVM_PROFILE_FILE')
print(llvm_profile_file)
-# CHECK: per-test-coverage-by-lit-cfg0.profraw
+# CHECK: per-test-coverage-by-lit-cfg[[INDEX]].profraw
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg b/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg
index 6e7f4c0e3bc97d..dae6db561a65db 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage/lit.cfg
@@ -3,6 +3,7 @@ import os
config.name = "per-test-coverage"
config.suffixes = [".py"]
-config.test_format = lit.formats.ShTest(execute_external=True)
+config.test_format = lit.formats.ShTest(
+ execute_external=eval(lit_config.params.get("execute_external"))
+)
config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
-
diff --git a/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py b/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py
index 65e4aa69bff079..025ecb43fce0ec 100644
--- a/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py
+++ b/llvm/utils/lit/tests/Inputs/per-test-coverage/per-test-coverage.py
@@ -1,5 +1,6 @@
# Check that the environment variable is set correctly
-# RUN: %{python} %s | FileCheck %s
+# RUN: %{python} %s | FileCheck -DINDEX=0 %s
+# RUN: %{python} %s | FileCheck -DINDEX=1 %s
# Python script to read the environment variable
# and print its value
@@ -8,4 +9,4 @@
llvm_profile_file = os.environ.get('LLVM_PROFILE_FILE')
print(llvm_profile_file)
-# CHECK: per-test-coverage0.profraw
+# CHECK: per-test-coverage[[INDEX]].profraw
diff --git a/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py b/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
index 1cc3927dc89041..0b9ecf5346a70d 100644
--- a/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
+++ b/llvm/utils/lit/tests/per-test-coverage-by-lit-cfg.py
@@ -1,6 +1,11 @@
# Test if lit_config.per_test_coverage in lit.cfg sets individual test case coverage.
-# RUN: %{lit} -a -v %{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py \
-# RUN: | FileCheck -match-full-lines %s
+# RUN: %{lit} -a -v -Dexecute_external=False \
+# RUN: %{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py | \
+# RUN: FileCheck -match-full-lines %s
+#
+# RUN: %{lit} -a -v -Dexecute_external=True \
+# RUN: %{inputs}/per-test-coverage-by-lit-cfg/per-test-coverage-by-lit-cfg.py | \
+# RUN: FileCheck -match-full-lines %s
#
# CHECK: PASS: per-test-coverage-by-lit-cfg :: per-test-coverage-by-lit-cfg.py ({{[^)]*}})
diff --git a/llvm/utils/lit/tests/per-test-coverage.py b/llvm/utils/lit/tests/per-test-coverage.py
index 428712e4792576..c521189e4aeb83 100644
--- a/llvm/utils/lit/tests/per-test-coverage.py
+++ b/llvm/utils/lit/tests/per-test-coverage.py
@@ -1,6 +1,11 @@
# Test LLVM_PROFILE_FILE is set when --per-test-coverage is passed to command line.
-# RUN: %{lit} -a -v --per-test-coverage %{inputs}/per-test-coverage/per-test-coverage.py \
-# RUN: | FileCheck -match-full-lines %s
+# RUN: %{lit} -a -v --per-test-coverage -Dexecute_external=False \
+# RUN: %{inputs}/per-test-coverage/per-test-coverage.py | \
+# RUN: FileCheck -match-full-lines %s
+#
+# RUN: %{lit} -a -v --per-test-coverage -Dexecute_external=True \
+# RUN: %{inputs}/per-test-coverage/per-test-coverage.py | \
+# RUN: FileCheck -match-full-lines %s
#
# CHECK: PASS: per-test-coverage :: per-test-coverage.py ({{[^)]*}})
More information about the llvm-commits
mailing list