llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-testing-tools

Author: Aiden Grossman (boomanaiden154)

<details>
<summary>Changes</summary>

This was deprecated in LLVM 23 and slated for removal in LLVM 24. Now
that we have branched and everything has been updated to not set
execute_external, we can remove it.


---
Full diff: https://github.com/llvm/llvm-project/pull/209571.diff


3 Files Affected:

- (modified) libcxx/utils/libcxx/test/format.py (+1-4) 
- (modified) llvm/utils/lit/lit/TestRunner.py (+5-135) 
- (modified) llvm/utils/lit/lit/formats/shtest.py (-11) 


``````````diff
diff --git a/libcxx/utils/libcxx/test/format.py 
b/libcxx/utils/libcxx/test/format.py
index 49cbe8a8db618..306fae764dc0f 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -384,10 +384,7 @@ def _executeShTest(self, test, litConfig, steps):
             )
         else:
             _, tmpBase = _getTempPaths(test)
-            useExternalSh = False
-            return lit.TestRunner._runShTest(
-                test, litConfig, useExternalSh, script, tmpBase
-            )
+            return lit.TestRunner._runShTest(test, litConfig, script, tmpBase)
 
     def _generateGenTest(self, testSuite, pathInSuite, litConfig, localConfig):
         generator = lit.Test.Test(testSuite, pathInSuite, localConfig)
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 97cb54c63be64..da5b56ede0529 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -822,109 +822,6 @@ def make_tree(cmds):
     return out, err, exitCode, timeoutInfo, update_output
 
 
-def executeScript(
-    test, litConfig, tmpBase, commands, cwd
-) -> tuple[str, str, int, str | None, str | None]:
-    bashPath = litConfig.getBashPath()
-    isWin32CMDEXE = litConfig.isWindows and not bashPath
-    script = tmpBase + ".script"
-    if isWin32CMDEXE:
-        script += ".bat"
-
-    # Write script file
-    if isWin32CMDEXE:
-        for i, ln in enumerate(commands):
-            match = re.fullmatch(kPdbgRegex, ln)
-            if match:
-                command = match.group(2)
-                commands[i] = match.expand(
-                    "echo '\\1' > nul && " if command else "echo '\\1' > nul"
-                )
-        with open(script, "w", encoding="utf-8") as f:
-            f.write("@echo on\n")
-            f.write("\n@if %ERRORLEVEL% NEQ 0 EXIT\n".join(commands))
-    else:
-        for i, ln in enumerate(commands):
-            match = re.fullmatch(kPdbgRegex, ln)
-            if match:
-                dbg = match.group(1)
-                command = match.group(2)
-                # Echo the debugging diagnostic to stderr.
-                #
-                # For that echo command, use 'set' commands to suppress the
-                # shell's execution trace, which would just add noise.  
Suppress
-                # the shell's execution trace for the 'set' commands by
-                # redirecting their stderr to /dev/null.
-                if command:
-                    msg = f"{shlex.quote(command.lstrip())} \\# '{dbg}'"
-                else:
-                    msg = f"'{dbg}' has no command after substitutions"
-                commands[i] = (
-                    f"{{ set +x; }} 2>/dev/null && "
-                    f"echo {msg} >&2 && "
-                    f"{{ set -x; }} 2>/dev/null"
-                )
-                # Execute the command, if any.
-                #
-                # 'command' might be something like:
-                #
-                #   subcmd & PID=$!
-                #
-                # In that case, we need something like:
-                #
-                #   echo_dbg && { subcmd & PID=$!; }
-                #
-                # Without the '{ ...; }' enclosing the original 'command', '&'
-                # would put all of 'echo_dbg && subcmd' in the background.  
This
-                # would cause 'echo_dbg' to execute at the wrong time, and a
-                # later kill of $PID would target the wrong process. We have
-                # seen the latter manage to terminate the shell running lit.
-                if command:
-                    commands[i] += f" && {{ {command}; }}"
-
-        # Manually export any DYLD_* variables used by dyld on macOS because
-        # otherwise they are lost when the shell executable is run, before the
-        # lit test is executed.
-        env_str = "\n".join(
-            "export {}={};".format(k, shlex.quote(v))
-            for k, v in test.config.environment.items()
-            if k.startswith("DYLD_")
-        )
-
-        with open(script, "w", encoding="utf-8", newline="") as f:
-            if test.config.pipefail:
-                f.write("set -o pipefail;")
-            f.write(env_str)
-            f.write("set -x;")
-            f.write("{ " + "; } &&\n{ ".join(commands) + "; }")
-            f.write("\n")
-
-    if isWin32CMDEXE:
-        command = ["cmd", "/c", script]
-    else:
-        if bashPath:
-            command = [bashPath, script]
-        else:
-            command = ["/bin/sh", script]
-        if litConfig.useValgrind:
-            # FIXME: Running valgrind on sh is overkill. We probably could just
-            # run on clang with no real loss.
-            command = litConfig.valgrindArgs + command
-
-    env = dict(test.config.environment)
-    env["LIT_CURRENT_TESTCASE"] = test.getFullName()
-    try:
-        out, err, exitCode = lit.util.executeCommand(
-            command,
-            cwd=cwd,
-            env=env,
-            timeout=test.config.maxIndividualTestTime,
-        )
-        return (out, err, exitCode, None, None)
-    except lit.util.ExecuteCommandTimeoutException as e:
-        return (e.out, e.err, e.exitCode, e.msg, None)
-
-
 def parseIntegratedTestScriptCommands(source_path, keywords):
     """
     parseIntegratedTestScriptCommands(source_path) -> commands
@@ -1807,7 +1704,7 @@ def parseIntegratedTestScript(test, 
additional_parsers=[], require_script=True):
     return script
 
 
-def _runShTest(test, litConfig, useExternalSh, script, tmpBase) -> 
lit.Test.Result:
+def _runShTest(test, litConfig, script, tmpBase) -> lit.Test.Result:
     # Always returns the tuple (out, err, exitCode, timeoutInfo, status).
     def runOnce(
         execdir,
@@ -1839,12 +1736,7 @@ def runOnce(
                 scriptCopy[i] = command
 
         try:
-            if useExternalSh:
-                res = executeScript(test, litConfig, tmpBase, scriptCopy, 
execdir)
-            else:
-                res = executeScriptInternal(
-                    test, litConfig, tmpBase, scriptCopy, execdir
-                )
+            res = executeScriptInternal(test, litConfig, tmpBase, scriptCopy, 
execdir)
         except ScriptFatal as e:
             out = f"# " + "\n# ".join(str(e).splitlines()) + "\n"
             return out, "", 1, None, Test.UNRESOLVED, None
@@ -1903,25 +1795,7 @@ def runOnce(
     )
 
 
-def _expandLateSubstitutionsExternal(commandLine):
-    filePaths = []
-
-    def _replaceReadFile(match):
-        filePath = match.group(1)
-        filePaths.append(filePath)
-        return "$(cat %s)" % shlex.quote(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=[]
-):
+def executeShTest(test, litConfig, extra_substitutions=[], 
preamble_commands=[]):
     if test.config.unsupported:
         return lit.Test.Result(Test.UNSUPPORTED, "Test is unsupported")
 
@@ -1942,7 +1816,7 @@ def executeShTest(
         test,
         tmpDir,
         tmpBase,
-        normalize_slashes=useExternalSh
+        normalize_slashes=False
         or litConfig.params.get("use_normalized_slashes", False),
     )
     conditions = {feature: True for feature in test.config.available_features}
@@ -1953,8 +1827,4 @@ 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)
+    return _runShTest(test, litConfig, script, tmpBase)
diff --git a/llvm/utils/lit/lit/formats/shtest.py 
b/llvm/utils/lit/lit/formats/shtest.py
index 80490c1f3227a..5c91cd4c153bf 100644
--- a/llvm/utils/lit/lit/formats/shtest.py
+++ b/llvm/utils/lit/lit/formats/shtest.py
@@ -18,19 +18,9 @@ class ShTest(FileBasedTest):
 
     def __init__(
         self,
-        execute_external=False,
         extra_substitutions=[],
         preamble_commands=[],
-        force_execute_external=False,
     ):
-        if execute_external and not force_execute_external:
-            raise ValueError(
-                "execute_external=True is deprected as of LLVM-23 and the 
option will "
-                "be removed in LLVM-24. Please move to using the internal 
shell "
-                "(execute_external=False). If you still need to force external 
"
-                "execution to allow time for migration, set 
force_execute_external=True"
-            )
-        self.execute_external = execute_external
         self.extra_substitutions = extra_substitutions
         self.preamble_commands = preamble_commands
 
@@ -38,7 +28,6 @@ def execute(self, test, litConfig):
         return lit.TestRunner.executeShTest(
             test,
             litConfig,
-            self.execute_external,
             self.extra_substitutions,
             self.preamble_commands,
         )

``````````

</details>


https://github.com/llvm/llvm-project/pull/209571
_______________________________________________
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to