[PATCH] D66293: [lit] Check for accidental external command calls

Mon Aug 19 15:19:57 PDT 2019

jdenny added a comment.

In D66293#1635995 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66293