windows: use default msys64 root

[sluongng]

On Windows, msys2 is always installed to C:\msys64 path by default(1).
Current users would have to override this with this flag

common:windows --shell_executable=c:/msys64/usr/bin/bash.exe

Let's use the correct default instead for a better Out-of-the-box UX.

(1): https://github.com/search?q=org%3Amsys2+%2Fc%3A.*msys64%2F&type=code&p=1

[sluongng]

cc: @tjgq @jayconrod as I guess the path was picked from the conversation in #18584

[fmeum]

I remember not having to set that flag on GitHub Actions Windows runners. Do you happen to know why? Do they use a different default path?

[sluongng]

Good question. From their image it seems like they used the standard path. https://github.com/actions/runner-images/blob/2530c697b5ca66ec6da5aa22f09cf7887bdc644c/images/windows/scripts/build/Install-Msys2.ps1#L33

I will check what did Bazel pick up in the actual action 🤔

[katre]

Looks like some level of test cleanup is needed.

I've reassigned this to @meteorcloudy because he has more windows knowledge than I do, and may know better why these values were chosen and have been working.

[meteorcloudy]

I believe the reason we chose C:\tools\msys2 instead of C:\msys2 as the default value is because when installed with choco install msys2, the default path would be under C:\tools

https://github.com/bazelbuild/continuous-integration/blob/9f04f3ef19b132ab0c407cfbc0eb33f211290043/buildkite/setup-windows.ps1#L78-L83

And Bazel should be able to locate the msys2 bash.exe in the client: https://cs.opensource.google/bazel/bazel/+/master:src/main/cpp/blaze_util_windows.cc;l=1251-1268

I'm not sure this is needed. Maybe we just need to fix --shell_executable to respect BAZEL_SH detected somehow?

[sluongng]

ah interesting that choco is putting things under C:\tools.

The "new" official Windows package manager is also putting it under C:\msys2 with winget install --id MSYS2.MSYS2 though. That plus Github Action made me think my new suggested path here should be the default instead.

For Bazel-CI, and other choco setups, perhaps we could apply an override flag to point back to tools\msys2?

I discovered this while deploying our RBE worker on an environment setup with winget. Was a bit surprise to see the Bazel client requesting for a non-standard path on the system.

[meteorcloudy]

Good to know, yeah, I guess c:/msys64/usr/bin/bash.exe is indeed better, we can probably fix the CI test failure with bazelbuild/continuous-integration#1944. But still this will probably be a breaking change for users, so we cannot backport it.

[sluongng]

Im ok with leaving this change for HEAD + Bazel 8 👍

[jayconrod]

+1 for C:/msys64 as the root, instead of C:/tools/msys64.

I don't remember needing to set --shell_executable though. BAZEL_SH seemed to work for me.

[sluongng]

Either --shell_executable or BAZEL_SH should work https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java;l=172-184;drc=70bb1da7328af216c66221463b7574754421a0d7

Personally, I think the flag is nicer to have inside .bazelrc 🤗

[sluongng]

@jayconrod after a bit more reading, it seems like BAZEL_SH does a bit more than the flag.

I raised #21988 so that we could discuss it separately.

[meteorcloudy]

I'm deploying bazelbuild/continuous-integration#1944 and will rerun the failed windows jobs after that.