[lit] Check for accidental external command calls

[jdenny]

Summary

This patch extends lit's test suite to check that lit's internal shell
doesn't accidentally execute internal commands as external commands.
It does so by putting fake failing versions of those commands in
PATH while the entire lit test suite is running. Without the fixes
in D65697 but with its tests, this approach catches accidental
external env calls.

[probinson]

IIUC this means if anyone tries to write a RUN line that pipes something to diff or cd or whatever, it would explicitly fail. That's probably a good move.
I suspect that : is not a legal filename on Windows. I'm not able to create such a file from the Windows CMD shell, and grepping the llvm and clang test suites for 'RUN:.* :' doesn't turn up anything that tries to use : as a command. Maybe we should just eliminate that command?

[jdenny]

In D66293#1635995, @probinson wrote:

IIUC this means if anyone tries to write a RUN line that pipes something to diff or cd or whatever, it would explicitly fail.

That's not quite what I was after. I should reword the summary to make it clear this patch is for catching specific bugs in lit. As such, this patch affects only lit's own test suite. The specific bugs are ones where lit's internal shell calls an external command instead of relying on its internal implementation of that command.

For example, when using lit's internal shell without D65697, env FOO=1 env BAR=2 some_command calls lit's internal env for the first env, but it searches PATH for the second env. With this patch, we can exercise such cases in lit's test suite and catch such bugs on any platform, including ones that happen to have a compatible external env implementation, which masks the bug.

I suspect that : is not a legal filename on Windows. I'm not able to create such a file from the Windows CMD shell, and grepping the llvm and clang test suites for 'RUN:.* :' doesn't turn up anything that tries to use : as a command. Maybe we should just eliminate that command?

Good point. It's probably not worth having that one. I'm only aware of : being used for reporting RUN line numbers.

Thanks for the reviews!

[probinson]

I was actually misunderstanding how lit/tests/lit.cfg fit in, and reacting to the comment about piping in lit.cfg. The top-level description actually looks okay. It makes sense to verify lit itself is behaving as desired in lit's own test suite, and not add these external commands to the PATH of all tests.
LGTM

llvm/trunk/utils/lit/tests/Inputs/fake-externals/cd

+#!/usr/bin/env python
+
+import fake_external
+
+fake_external.execute(__file__)

llvm/trunk/utils/lit/tests/Inputs/fake-externals/diff

+#!/usr/bin/env python
+
+import fake_external
+
+fake_external.execute(__file__)

llvm/trunk/utils/lit/tests/Inputs/fake-externals/env

+#!/usr/bin/env python
+
+import fake_external
+
+fake_external.execute(__file__)

llvm/trunk/utils/lit/tests/Inputs/fake-externals/export

+#!/usr/bin/env python
+
+import fake_external
+
+fake_external.execute(__file__)

llvm/trunk/utils/lit/tests/Inputs/fake-externals/fake_external.py

+import os
+import sys
+
+def execute(fileName):
+  sys.stderr.write("error: external '{}' command called unexpectedly\n"
+                   .format(os.path.basename(fileName)));
+  sys.exit(1)

llvm/trunk/utils/lit/tests/Inputs/fake-externals/mkdir

+#!/usr/bin/env python
+
+import fake_external
+
+fake_external.execute(__file__)

llvm/trunk/utils/lit/tests/Inputs/fake-externals/rm

+#!/usr/bin/env python
+
+import fake_external
+
+fake_external.execute(__file__)

llvm/trunk/utils/lit/tests/lit.cfg

lit_config.warning('Setting a timeout per test not supported. ' + errormsg
                          ' command line argument will not work.')
 
 # When running the lit tests standalone, we want to define the same features
 # that the llvm_config defines. This means that the 'system-windows' feature
 # (and any others) need to match the names in llvm_config for consistency
 if not llvm_config:
   if sys.platform.startswith('win') or sys.platform.startswith('cygwin'):
     config.available_features.add('system-windows')
+
+# For each of lit's internal shell commands ('env', 'cd', 'diff', etc.), put
+# a fake command that always fails at the start of PATH.  This helps us check
+# that we always use lit's internal version rather than some external version
+# that might not be present or behave correctly on all platforms.  Don't do
+# this for 'echo' because an external version is used when it appears in a
+# pipeline.  Don't do this for ':' because it doesn't appear to be a valid file
+# name under Windows.
+test_bin = os.path.join(os.path.dirname(__file__), 'Inputs', 'fake-externals')
+config.environment['PATH'] = os.path.pathsep.join((test_bin,
+                                                   config.environment['PATH']))