[llvm] 0d4e651 - [lit] Fix internal env calling other internal commands
Joel E. Denny via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 31 11:39:53 PDT 2019
Author: Joel E. Denny
Date: 2019-10-31T14:37:51-04:00
New Revision: 0d4e6519c5dd81034935d4da9c519c17e41b1202
URL: https://github.com/llvm/llvm-project/commit/0d4e6519c5dd81034935d4da9c519c17e41b1202
DIFF: https://github.com/llvm/llvm-project/commit/0d4e6519c5dd81034935d4da9c519c17e41b1202.diff
LOG: [lit] Fix internal env calling other internal commands
Without this patch, when using lit's internal shell, if `env` on a lit
RUN line calls `cd`, `mkdir`, or any of the other in-process shell
builtins that lit implements, lit accidentally searches for the latter
as an external executable.
This patch puts such builtins in a map so that boilerplate for them
need be implemented only once. This patch moves that handling after
processing of `env` so that `env` calling such a builtin can be
detected. Finally, because such calls appear to be useless, this
patch takes the safe approach of diagnosing them rather than
supporting them.
Reviewed By: probinson, mgorny, rnk
Differential Revision: https://reviews.llvm.org/D66506
Added:
llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt
llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt
llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt
llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt
llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt
llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt
Modified:
llvm/utils/lit/lit/TestRunner.py
llvm/utils/lit/tests/shtest-env.py
Removed:
################################################################################
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index a334d0d638ee..278f6dea5014 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -260,6 +260,27 @@ def updateEnv(env, args):
env.env[key] = val
return args[arg_idx_next:]
+def executeBuiltinCd(cmd, shenv):
+ """executeBuiltinCd - Change the current directory."""
+ if len(cmd.args) != 2:
+ raise InternalShellError("'cd' supports only one argument")
+ newdir = cmd.args[1]
+ # Update the cwd in the parent environment.
+ if os.path.isabs(newdir):
+ shenv.cwd = newdir
+ else:
+ shenv.cwd = os.path.realpath(os.path.join(shenv.cwd, newdir))
+ # The cd builtin always succeeds. If the directory does not exist, the
+ # following Popen calls will fail instead.
+ return ShellCommandResult(cmd, "", "", 0, False)
+
+def executeBuiltinExport(cmd, shenv):
+ """executeBuiltinExport - Set an environment variable."""
+ if len(cmd.args) != 2:
+ raise InternalShellError("'export' supports only one argument")
+ updateEnv(shenv, cmd.args)
+ return ShellCommandResult(cmd, "", "", 0, False)
+
def executeBuiltinEcho(cmd, shenv):
"""Interpret a redirected echo command"""
opened_files = []
@@ -319,9 +340,8 @@ def maybeUnescape(arg):
for (name, mode, f, path) in opened_files:
f.close()
- if not is_redirected:
- return stdout.getvalue()
- return ""
+ output = "" if is_redirected else stdout.getvalue()
+ return ShellCommandResult(cmd, output, "", 0, False)
def executeBuiltinMkdir(cmd, cmd_shenv):
"""executeBuiltinMkdir - Create new directories."""
@@ -456,6 +476,10 @@ class SHFILEOPSTRUCTW(Structure):
exitCode = 1
return ShellCommandResult(cmd, "", stderr.getvalue(), exitCode, False)
+def executeBuiltinColon(cmd, cmd_shenv):
+ """executeBuiltinColon - Discard arguments and exit with status 0."""
+ return ShellCommandResult(cmd, "", "", 0, False)
+
def processRedirects(cmd, stdin_source, cmd_shenv, opened_files):
"""Return the standard fds for cmd after applying redirects
@@ -581,64 +605,6 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
raise ValueError('Unknown shell command: %r' % cmd.op)
assert isinstance(cmd, ShUtil.Pipeline)
- # Handle shell builtins first.
- if cmd.commands[0].args[0] == 'cd':
- if len(cmd.commands) != 1:
- raise ValueError("'cd' cannot be part of a pipeline")
- if len(cmd.commands[0].args) != 2:
- raise ValueError("'cd' supports only one argument")
- newdir = cmd.commands[0].args[1]
- # Update the cwd in the parent environment.
- if os.path.isabs(newdir):
- shenv.cwd = newdir
- else:
- shenv.cwd = os.path.realpath(os.path.join(shenv.cwd, newdir))
- # The cd builtin always succeeds. If the directory does not exist, the
- # following Popen calls will fail instead.
- return 0
-
- # Handle "echo" as a builtin if it is not part of a pipeline. This greatly
- # speeds up tests that construct input files by repeatedly echo-appending to
- # a file.
- # FIXME: Standardize on the builtin echo implementation. We can use a
- # temporary file to sidestep blocking pipe write issues.
- if cmd.commands[0].args[0] == 'echo' and len(cmd.commands) == 1:
- output = executeBuiltinEcho(cmd.commands[0], shenv)
- results.append(ShellCommandResult(cmd.commands[0], output, "", 0,
- False))
- return 0
-
- if cmd.commands[0].args[0] == 'export':
- if len(cmd.commands) != 1:
- raise ValueError("'export' cannot be part of a pipeline")
- if len(cmd.commands[0].args) != 2:
- raise ValueError("'export' supports only one argument")
- updateEnv(shenv, cmd.commands[0].args)
- return 0
-
- if cmd.commands[0].args[0] == 'mkdir':
- if len(cmd.commands) != 1:
- raise InternalShellError(cmd.commands[0], "Unsupported: 'mkdir' "
- "cannot be part of a pipeline")
- cmdResult = executeBuiltinMkdir(cmd.commands[0], shenv)
- results.append(cmdResult)
- return cmdResult.exitCode
-
- if cmd.commands[0].args[0] == 'rm':
- if len(cmd.commands) != 1:
- raise InternalShellError(cmd.commands[0], "Unsupported: 'rm' "
- "cannot be part of a pipeline")
- cmdResult = executeBuiltinRm(cmd.commands[0], shenv)
- results.append(cmdResult)
- return cmdResult.exitCode
-
- if cmd.commands[0].args[0] == ':':
- if len(cmd.commands) != 1:
- raise InternalShellError(cmd.commands[0], "Unsupported: ':' "
- "cannot be part of a pipeline")
- results.append(ShellCommandResult(cmd.commands[0], '', '', 0, False))
- return 0;
-
procs = []
default_stdin = subprocess.PIPE
stderrTempFiles = []
@@ -646,6 +612,12 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
named_temp_files = []
builtin_commands = set(['cat', '
diff '])
builtin_commands_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), "builtin_commands")
+ inproc_builtins = {'cd': executeBuiltinCd,
+ 'export': executeBuiltinExport,
+ 'echo': executeBuiltinEcho,
+ 'mkdir': executeBuiltinMkdir,
+ 'rm': executeBuiltinRm,
+ ':': executeBuiltinColon}
# To avoid deadlock, we use a single stderr stream for piped
# output. This is null until we have seen some output using
# stderr.
@@ -663,6 +635,27 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
raise InternalShellError(j,
"Error: 'env' requires a subcommand")
+ # Handle in-process builtins.
+ #
+ # Handle "echo" as a builtin if it is not part of a pipeline. This
+ # greatly speeds up tests that construct input files by repeatedly
+ # echo-appending to a file.
+ # FIXME: Standardize on the builtin echo implementation. We can use a
+ # temporary file to sidestep blocking pipe write issues.
+ inproc_builtin = inproc_builtins.get(args[0], None)
+ if inproc_builtin and (args[0] != 'echo' or len(cmd.commands) == 1):
+ # env calling an in-process builtin is useless, so we take the safe
+ # approach of complaining.
+ if not cmd_shenv is shenv:
+ raise InternalShellError(j, "Error: 'env' cannot call '{}'"
+ .format(args[0]))
+ if len(cmd.commands) != 1:
+ raise InternalShellError(j, "Unsupported: '{}' cannot be part"
+ " of a pipeline".format(args[0]))
+ result = inproc_builtin(j, cmd_shenv)
+ results.append(result)
+ return result.exitCode
+
stdin, stdout, stderr = processRedirects(j, default_stdin, cmd_shenv,
opened_files)
diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt
new file mode 100644
index 000000000000..b045506ba31b
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt
@@ -0,0 +1 @@
+# RUN: env -u FOO BAR=3 cd foobar
diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt
new file mode 100644
index 000000000000..f2566c43b2d2
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt
@@ -0,0 +1 @@
+# RUN: env -u FOO BAR=3 :
diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt
new file mode 100644
index 000000000000..b4591ee5f231
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt
@@ -0,0 +1 @@
+# RUN: env -u FOO BAR=3 echo hello world
diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt
new file mode 100644
index 000000000000..9712e42f2025
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt
@@ -0,0 +1 @@
+# RUN: env -u FOO BAR=3 export BAZ=3
diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt
new file mode 100644
index 000000000000..6116a25c94f3
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt
@@ -0,0 +1 @@
+# RUN: env -u FOO BAR=3 mkdir foobar
diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt
new file mode 100644
index 000000000000..6c5e8e873914
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt
@@ -0,0 +1 @@
+# RUN: env -u FOO BAR=3 rm foobar
diff --git a/llvm/utils/lit/tests/shtest-env.py b/llvm/utils/lit/tests/shtest-env.py
index e89bfd6156b6..3b547dff85ee 100644
--- a/llvm/utils/lit/tests/shtest-env.py
+++ b/llvm/utils/lit/tests/shtest-env.py
@@ -7,7 +7,7 @@
# Make sure env commands are included in printed commands.
-# CHECK: -- Testing: 7 tests{{.*}}
+# CHECK: -- Testing: 13 tests{{.*}}
# CHECK: FAIL: shtest-env :: env-args-last-is-assign.txt ({{[^)]*}})
# CHECK: Error: 'env' requires a subcommand
@@ -25,19 +25,52 @@
# CHECK: Error: 'env' requires a subcommand
# CHECK: error: command failed with exit status: {{.*}}
+# CHECK: FAIL: shtest-env :: env-calls-cd.txt ({{[^)]*}})
+# CHECK: $ "env" "-u" "FOO" "BAR=3" "cd" "foobar"
+# CHECK: Error: 'env' cannot call 'cd'
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-env :: env-calls-colon.txt ({{[^)]*}})
+# CHECK: $ "env" "-u" "FOO" "BAR=3" ":"
+# CHECK: Error: 'env' cannot call ':'
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-env :: env-calls-echo.txt ({{[^)]*}})
+# CHECK: $ "env" "-u" "FOO" "BAR=3" "echo" "hello" "world"
+# CHECK: Error: 'env' cannot call 'echo'
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-env :: env-calls-export.txt ({{[^)]*}})
+# CHECK: $ "env" "-u" "FOO" "BAR=3" "export" "BAZ=3"
+# CHECK: Error: 'env' cannot call 'export'
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-env :: env-calls-mkdir.txt ({{[^)]*}})
+# CHECK: $ "env" "-u" "FOO" "BAR=3" "mkdir" "foobar"
+# CHECK: Error: 'env' cannot call 'mkdir'
+# CHECK: error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-env :: env-calls-rm.txt ({{[^)]*}})
+# CHECK: $ "env" "-u" "FOO" "BAR=3" "rm" "foobar"
+# CHECK: Error: 'env' cannot call 'rm'
+# CHECK: error: command failed with exit status: {{.*}}
+
# CHECK: PASS: shtest-env :: env-u.txt ({{[^)]*}})
# CHECK: $ "{{[^"]*}}" "print_environment.py"
# CHECK: $ "env" "-u" "FOO" "{{[^"]*}}" "print_environment.py"
# CHECK: $ "env" "-u" "FOO" "-u" "BAR" "{{[^"]*}}" "print_environment.py"
+# CHECK-NOT: ${{.*}}print_environment.py
# CHECK: PASS: shtest-env :: env.txt ({{[^)]*}})
# CHECK: $ "env" "A_FOO=999" "{{[^"]*}}" "print_environment.py"
# CHECK: $ "env" "A_FOO=1" "B_BAR=2" "C_OOF=3" "{{[^"]*}}" "print_environment.py"
+# CHECK-NOT: ${{.*}}print_environment.py
# CHECK: PASS: shtest-env :: mixed.txt ({{[^)]*}})
# CHECK: $ "env" "A_FOO=999" "-u" "FOO" "{{[^"]*}}" "print_environment.py"
# CHECK: $ "env" "A_FOO=1" "-u" "FOO" "B_BAR=2" "-u" "BAR" "C_OOF=3" "{{[^"]*}}" "print_environment.py"
+# CHECK-NOT: ${{.*}}print_environment.py
# CHECK: Expected Passes : 3
-# CHECK: Unexpected Failures: 4
+# CHECK: Unexpected Failures: 10
# CHECK-NOT: {{.}}
More information about the llvm-commits
mailing list