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

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 15:54:57 PDT 2019


jdenny added a comment.

In D65697#1640202 <https://reviews.llvm.org/D65697#1640202>, @probinson wrote:

> In D65697#1640136 <https://reviews.llvm.org/D65697#1640136>, @jdenny wrote:
>
> > In D65697#1640109 <https://reviews.llvm.org/D65697#1640109>, @rnk wrote:
> >
> > > Most LLVM test suites are configured to use the internal shell on Windows. I think what's happening is that users happen to have an external copy of `diff` that gets called when it appears in a pipeline, or they are prefixed by `not`. The external `diff` is called, and things "work out" and we never noticed.
> >
> >
> > Are these patches going to break those tests on Windows?  We now see `diff` anywhere in a pipeline, and we reject its appearance in a pipeline.
>


I've confirmed that, when using lit's internal shell (not on Windows, but that shouldn't matter), `echo foo | diff %t -` previously called an external `diff`, but fails after this patch series is applied.

> `grep -r -l 'RUN:.* diff ' llvm/test clang/test` gets 240 hits.  Searching on `| diff` gets 41, mostly in clang/test/Analysis.  So not a super common pattern, but if it's straightforward to build a Python 'diff' that handles pipes, that would be less invasive, and piping does have a minor speed advantage.

I guess I have more work to do either way....


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65697





More information about the llvm-commits mailing list