Add full mingw-w64-git (i.e. regular MSYS2 ecosystem) support
#5971
+40
−123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have upstreamed this already as 436a422 (max_tree_depth: lower it for clangarm64 on Windows, 2025-04-23). This reverts commit 1d52d30 (max_tree_depth: lower it for clangarm64 on Windows, 2025-04-15). Signed-off-by: Johannes Schindelin <[email protected]>
It is still one of the really vexing quirks of Git's test suite that it requires executables that are not installed via the `install` target. To be able to validate MinGit, I introduced a Makefile target once upon a time to address that omission so as to be able to run the test suite on any installed Git version by bundling the test suite together with those test-supporting executables. However, I never got around to upstream this because the principal reason (moving toward a BusyBox-backed MinGit) went away: BusyBox-w32 is simply not maintained in a confidence-inducing manner and therefore I all but abandoned that initiative. Therefore, this Makefile target is not in a good shape, and it's time to let it go. Signed-off-by: Johannes Schindelin <[email protected]>
This option was added in fa93bb2 (MinGW: Fix stat definitions to work with MinGW runtime version 4.0, 2013-09-11), i.e. a _long_ time ago. So long, in fact, that it still targeted MinGW. But we switched to mingw-w64 in 2015, which seems not to share the problem, and therefore does not require a fix. Even worse: This flag is incompatible with UCRT64, which we are about to support by way of upstreaming `mingw-w64-git` to the MSYS2 project, see msys2/MINGW-packages#26470 for details. So let's send that option into its well-deserved retirement. Signed-off-by: Johannes Schindelin <[email protected]>
That option only matters there, and is in fact only really understood in those builds; UCRT64 versions of GCC, for example, do not know what to do with that option. Signed-off-by: Johannes Schindelin <[email protected]>
In bf2d5d8 (Don't let ld strip relocations, 2016-01-16) (picked from git-for-windows@6a237925bf10), Git for Windows introduced the `-Wl,-pic-executable` flag, specifying the exact entry point via `-e`. This required discerning between i686 and x86_64 code because the former required the symbol to be prefixed with an underscore, the latter did not. As per https://sourceware.org/bugzilla/show_bug.cgi?id=10865, the specified symbols are already the default, though. So let's drop the overly-specific definition. Signed-off-by: Johannes Schindelin <[email protected]>
MSYS2 already defines a couple of helpful environment variables, and we can use those to infer the installation location as well as the CPU. No need for hard-coding ;-) Signed-off-by: Johannes Schindelin <[email protected]>
The tell-tale is the presence of the `MSYSTEM` value while compiling, of course. In that case, we want to ensure that `MSYSTEM` is set when running `git.exe`, and also enable the magic MSYS2 tty detection. Signed-off-by: Johannes Schindelin <[email protected]>
MSYS2 defines some helpful environment variables, e.g. `MSYSTEM`. There is code in Git for Windows to ensure that that `MSYSTEM` variable is set, hard-coding a default. However, the existing solution jumps through hoops to reconstruct the proper default, and is even incomplete doing so, as we found out when we extended it to support CLANGARM64. This is absolutely unnecessary because there is already a perfectly valid `MSYSTEM` value we can use at build time. This is even true when building the MINGW32 variant on a MINGW64 system because `makepkg-mingw` will override the `MSYSTEM` value as per the `MINGW_ARCH` array. The same is equally true for the `/mingw64`, `/mingw32` and `/clangarm64` prefix: those values are already available via the `MINGW_PREFIX` environment variable, and we just need to pass that setting through. Only when `MINGW_PREFIX` is not set (as is the case in Git for Windows' minimal SDK, where only `MSYSTEM` is guaranteed to be set correctly), we use as fall-back the top-level directory whose name is the down-cased value of the `MSYSTEM` variable. Incidentally, this also broadens the support to all the configurations supported by the MSYS2 project, i.e. clang64 & ucrt64, too. Signed-off-by: Johannes Schindelin <[email protected]>
Special-casing even more configurations simply does not make sense. Signed-off-by: Johannes Schindelin <[email protected]>
In 436a422 (max_tree_depth: lower it for clangarm64 on Windows, 2025-04-23), I provided a work-around for a nasty issue with clangarm builds, where the stack is exhausted before the maximal tree depth is reached, and the resulting error cannot easily be handled by Git (because it would require Windows-specific handling). Turns out that this is not at all limited to ARM64. In my tests with CLANG64 in MSYS2 on the GitHub Actions runners, the test t6700.4 failed in the exact same way. What's worse: The limit needs to be quite a bit lower for x86_64 than for aarch64. In aforementioned tests, the breaking point was 1232: With 1231 it still worked as expected, with 1232 it would fail with the `STATUS_STACK_OVERFLOW` incorrectly mapped to exit code 127. For comparison, in my tests on GitHub Actions' Windows/ARM64 runners, the breaking point was 1439 instead. Therefore the condition needs to be adapted once more, to accommodate (with some safety margin) both aarch64 and x86_64 in clang-based builds on Windows, to let that test pass. Signed-off-by: Johannes Schindelin <[email protected]>
This is no longer true in general, not with supporting Clang out of the box. Signed-off-by: Johannes Schindelin <[email protected]>
dscho
force-pushed
the
add-full-mingw-w64-git-support
branch
from
e2683ce to
78589cf
Compare
Member
Author
|
Something seems to still be wrong with the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Every once in a while, there are bug reports in Git for Windows' bug tracker that describe an issue running inside MSYS2 proper, totally ignoring the big, honking warning on top of the page that spells out clearly that this is an unsupported use case.
At the same time, we cannot easily deflect and say "just use MSYS2 directly" (and leave the "and stop pestering us" out). We cannot do that because there is only an MSYS
gitpackage in MSYS2 (i.e. a Git that uses the quite slow POSIX emulation layer provided by the MSYS2 runtime), but nomingw-w64-gitpackage (which would be equivalent in speed to Git for Windows).In msys2/MINGW-packages#26470, I am preparing to change that. As part of that PR, I noticed and fixed a couple of issues _in
git-for-windows/gitthat prevented full support formingw-w64-gitin MSYS2, such as problems with CLANG64 and UCRT64.While at it, I simplified the entire setup to trust MSYS2's
MINGW_PREFIX& related environment variables instead of hard-coding values like the installation prefix and whatMSYSTEMto fall back on if it is unset.