[PATCH] D29515: [lit] Don't use bash on Windows. Pipeing stdout to Filecheck doesn't work.

[Zachary Turner via Phabricator via llvm-commits]

zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.

I support this patch because it reduces unpredictableness in the test suite.  I see no advantage to having the test suite silently behave differently depending on whether you have a piece of 3rd party software installed on Windows.

If we decide to revisit this in the future, we should make sure that we only support the official Windows Ubuntu (not git Bash), and we should have a buildbot which explicitly tests using this configuration.  I've seen issues crop up in the past that would only occur on my machine but not another person's machine (or vice versa), and it's very likely this could have been the cause.

Give it some time to see what other people say, but my vote is for lgtm.



================
Comment at: utils/lit/lit/TestRunner.py:575
     bashPath = litConfig.getBashPath()
-    isWin32CMDEXE = (litConfig.isWindows and not bashPath)
+    isWin32CMDEXE = litConfig.isWindows
     script = tmpBase + '.script'
----------------
I would rather initialize the variable `isWin32CMDEXE` first, and then for `bashPath`, say `bashPath = None if isWin32CMDEXE else litConfig.getBashPath()`.

This makes sure that some logic later on will not break if it remembers to only check one variable or the other.


https://reviews.llvm.org/D29515