[PATCH] D65697: [lit] Fix internal env calling internal commands

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:247
         # Support for the -u flag (unsetting) for env command
         # e.g., env -u FOO -u BAR will remove both FOO and BAR
         # from the environment.
----------------
Hm, I bet we shouldn't handle `-u` for `export`.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:803
+
+    # Extract env commands and their arguments so we don't overlook any shell
+    # builtin.  Don't modify the original args as we'll need to process the
----------------
I don't like the way we have to process env prefixes twice, once for in-process shell builtins and once for out-of-process builtins. I think we should try to make things more uniform by moving the in-process builtins into the loop between 'env' handling and processRedirects. I'd suggest reducing the boilerplate by building a set of in-process builtins (`inproc_builtins`?), checking against the set, and then calling a helper function that returns a ShellCommandResult or throws. You can do the check for "cannot be part of a pipeline" in the shared code and raise the same InternalShellErrors. Does that seem straightforward?

Speaking of which, I see that `diff` actually can take one of the files to compare on stdin, so technically we should move it out of process.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65697/new/

https://reviews.llvm.org/D65697