[Patch] lit.cfg: better check for MSYS

[Hans Wennborg]

On Mon, Jul 29, 2013 at 6:07 PM, Hans Wennborg <hans at chromium.org> wrote:
> On Mon, Jul 29, 2013 at 3:28 PM, Reid Kleckner <rnk at google.com> wrote:
>> bash -help is pretty fast to start, but it also seems pretty heavyweight for
>> lit.cfg.  It's probably better to check not execute_external like this:
>>
>> $ git diff test/lit.cfg
>> diff --git a/test/lit.cfg b/test/lit.cfg
>> index a5bb350..f2f16fb 100644
>> --- a/test/lit.cfg
>> +++ b/test/lit.cfg
>> @@ -245,7 +245,7 @@ if execute_external:
>>      config.available_features.add('shell')
>>
>>  # Exclude MSYS due to transforming '/' to 'X:/mingwroot/'.
>> -if not platform.system() in ['Windows'] or lit.getBashPath() == '':
>> +if not platform.system() in ['Windows'] or not execute_external:
>>      config.available_features.add('shell-preserves-root')
>
> That sounds good to me. Takumi, what do you think about this change? I
> see we do (almost) the same check earlier in the file, in
> getClangBuiltinIncludeDir().

Ping?

>> On Mon, Jul 29, 2013 at 1:57 PM, Hans Wennborg <hans at chromium.org> wrote:
>>>
>>> On Mon, Jul 29, 2013 at 11:44 AM, Reid Kleckner <rnk at google.com> wrote:
>>> > On Mon, Jul 29, 2013 at 11:33 AM, Hans Wennborg <hans at chromium.org>
>>> > wrote:
>>> >>
>>> >> On Mon, Jul 29, 2013 at 11:01 AM, NAKAMURA Takumi
>>> >> <geek4civic at gmail.com>
>>> >> wrote:
>>> >> > I suppose that system's python should be /usr/bin/python in Cygwin's
>>> >> > world.
>>> >> > I didn't know Python/win32 could run Lit for cygwin tests.
>>> >> >
>>> >> > What is a motivation for you to take *external* python on cygwin?
>>> >>
>>> >> I build Clang outside Cygwin, in cmd.exe, because I want to build in a
>>> >> "pure win32" environment. But I fail at running the tests in cmd.exe,
>>> >> so I use Cygwin to run the tests.
>>> >>
>>> >> If I try to use Cygwin's /usr/bin/python to run the tests, it fails
>>> >> because of some Windows path. Probably because I didn't build in
>>> >> Cygwin.
>>> >>
>>> >> I guess my configuration is confusing to lit.cfg, because I have
>>> >> platform.system() = "Windows" and lit.getBashPath() =
>>> >> "C:\src\cygwin\bin\bash.EXE". But it's still not MSYS :)
>>> >
>>> >
>>> > This configuration is pretty handy because it's way faster to spawn
>>> > processes from native win32 python than from cygwin bash or cygwin
>>> > python.
>>> >
>>> > I think the odd duck that we should be checking for here is MSys, so
>>> > using
>>> > the MSYSTEM env var sounds good to me.
>>>
>>> I got paranoid and started worrying about the possibility of MSYS bash
>>> ending up on the path, even if the test suite is started outside an
>>> MSYS shell. In that case, looking for $MSYSTEM wouldn't work.
>>>
>>> Maybe we should just ask the bash binary which version it has? I'm
>>> attaching a patch that does that, please take a look.
>>>
>>> I realize this isn't super important, as the tests are still being run
>>> by the cygwin buildbots, but it would be nice if this configuration
>>> were supported too.
>>>
>>> Thanks,
>>> Hans