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

Wed Aug 21 09:21:15 PDT 2019

jdenny added a comment.

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

> > Do people know why an internal diff was implemented originally? Is there a portability or performance issue?
>
> See D39567 <https://reviews.llvm.org/D39567> and in particular an enthused zturner looked forward to the day when all "shell" commands are internal, eliminating cross-platform issues and most if not all dependencies on additional packages other than Python (e.g. GnuWin32 on Windows).  The only external commands would be LLVM-built tools/utilities.  I see rnk participated in that review (late 2017).
>
> > If it's worthwhile to have an internal diff, is it worthwhile for it to support everything external diffs do? All pipes and redirects could be replaced by temporary files.
>
> Creating temp files is slower than piping, given we have tens of thousands of tests it feels worthwhile to support piping.  For diff in particular I found only one (I think unnecessary) redirect so not supporting that seems reasonable.

Thanks.  Makes sense.

When rnk suggested diff be moved out of process, I originally thought he meant we should depend on external (that is, not provided by lit) implementations of diff.  Now I think he meant we should move it to an external python script, similar to `llvm/utils/lit/lit/builtin_commands/cat.py`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65697