[PATCH] D11960: [windows] Always use the lit shell on Windows, even if bash is present

[Reid Kleckner via llvm-commits]

rnk added a comment.

In http://reviews.llvm.org/D11960#222206, @filcab wrote:

> In http://reviews.llvm.org/D11960#222184, @yaron.keren wrote:
>
> > The bash shell on Windows from MSYS expands anyhting that looks like a relative path, such as /m.  It has workaround for switches but not for environment vas so tests using 'env' were failing without any way to fix.
>
>
> Can you provide an example of that issue, please? I'm running all my tests (only ASan and UBSan, for now) on a Windows host (not a Windows target), and I don't seem to see this. Can the issue be solved in another way? Quoting parameters that start with `/X`, for example?
>
> I'm using the bash.exe that comes with git:
>
>   $ bash --version
>   GNU bash, version 3.1.20(4)-release (i686-pc-msys)
>   Copyright (C) 2005 Free Software Foundation, Inc.
>


I have no idea what git's msys bash does. The normal msys bash will turn all command line arguments starting with / into Windows paths when invoking Windows executables that don't link the mingw crt. Cygwin's bash is also buggy and is generally really slow at process creation due to fork emulation. Personally, I think we should move towards using the internal lit shell everywhere when possible. It makes behavior a lot more predictable.

Anyway, whether we use an external bash is definitely a choice of the host. It saves us from debugging issues around incompatible bash implementations.

> > In addition, it did not pass environment variable TZ since it does not know how ot map between Unix and Windows TZ. This also broke tests.

> 

> 

> Which tests? Can this be solved with a script for `%run`? A substitution in lit? It seems like a bug somewhere else. Why is `TZ` not being passed while other env vars are?


Some MSys bug. There is no workaround.

> > Finally, process creation time on Windows is much higher than on Linux so any command that's implemented internally in the lit shell makes the tests go faster. Note that not all bells and whistles of the commands are really used in the tests.

> 

> 

> This tells me that maybe a toggle would be good, to force an internal lit shell. But I don't see the benefit in not running all the tests we can...


In LLVM and clang we have this:

  use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")

It overrides the normal decision. It didn't seem worth duplicating here. If we want it, we should roll this decision up into some helper in lit.util that we can share, like lit.util.usePlatformSdkOnDarwin.


http://reviews.llvm.org/D11960