Support Windows/ARM64

[dscho]

Git for Windows has started building artifacts for Windows/ARM64 since v2.47.1 (November 25th 2024). Now that Windows/ARM64 GitHub Action runners are available in public preview at long last, it is high time to upstream the minimal set of patches to build Git on Windows/ARM64 and pass the test suite.

Changes since v1:

• Replaced an #else #if construct by an #elif one.

cc: Patrick Steinhardt [email protected]

[dscho]

/submit

[gitgitgadget]

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1904/dscho/support-clangarm64-v1

To fetch this version to local tag pr-1904/dscho/support-clangarm64-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1904/dscho/support-clangarm64-v1

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> Git for Windows has started building artifacts for Windows/ARM64 since
> v2.47.1 (November 25th 2024). Now that Windows/ARM64 GitHub Action runners
> are available in public preview
> [https://github.blog/changelog/2025-04-14-windows-arm64-hosted-runners-now-available-in-public-preview/]
> at long last, it is high time to upstream the minimal set of patches to
> build Git on Windows/ARM64 and pass the test suite.
>
> Dennis Ameling (2):
>   bswap.h: add support for built-in bswap functions
>   config.mak.uname: add support for clangarm64
>
> Johannes Schindelin (4):
>   mingw: do not use nedmalloc on Windows/ARM64
>   msvc: do handle builds on Windows/ARM64
>   mingw(arm64): do move the `/etc/git*` location
>   max_tree_depth: lower it for clangarm64 on Windows
>
>  compat/bswap.h   | 14 +++++++++++++-
>  config.mak.uname | 18 ++++++++++++++----
>  environment.c    | 12 ++++++++++++
>  3 files changed, 39 insertions(+), 5 deletions(-)
>
>
> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1904%2Fdscho%2Fsupport-clangarm64-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1904/dscho/support-clangarm64-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1904

Will queue.  Thanks for a cleanly structured series.

[gitgitgadget]

This patch series was integrated into seen via git@5658ab3.

[gitgitgadget]

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Mon, Apr 21, 2025 at 12:39:07PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <[email protected]>
> 
> It does not compile there, and seeing as nedmalloc has been pretty much
> unmaintained since at least November 2017, as per
> https://github.com/ned14/nedmalloc/issues/20#issuecomment-343432314,
> there is also no hope that any fixes will materialize there.

This kind of raises the question whether we want to keep on maintaining
nedmalloc in our codebase at all. Is there any strong reason to have it?

Patrick

[gitgitgadget]

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Patrick,

On Tue, 22 Apr 2025, Patrick Steinhardt wrote:

> On Mon, Apr 21, 2025 at 12:39:07PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <[email protected]>
> > 
> > It does not compile there, and seeing as nedmalloc has been pretty much
> > unmaintained since at least November 2017, as per
> > https://github.com/ned14/nedmalloc/issues/20#issuecomment-343432314,
> > there is also no hope that any fixes will materialize there.
> 
> This kind of raises the question whether we want to keep on maintaining
> nedmalloc in our codebase at all. Is there any strong reason to have it?

To the contrary, There is a very strong reason to drop it: nedmalloc is
unmaintained.

There is just a teeny tiny blocker before it can be dropped, though:

$ git grep -n USE_NED_ALLOCATOR upstream/master -- ':(exclude)Makefile'
upstream/master:config.mak.uname:478:   # USE_NED_ALLOCATOR = YesPlease
upstream/master:config.mak.uname:741:   USE_NED_ALLOCATOR = YesPlease
upstream/master:contrib/buildsystems/CMakeLists.txt:258:                                USE_NED_ALLOCATOR OVERRIDE_STRDUP MMAP_PREVENTS_DELETE USE_WIN32_MMAP

The commented-out one is the MSVC build (I had experimental Git for
Windows patches to enable nedmalloc even in MSVC builds, which I abandoned
in favor of https://github.com/git-for-windows/git/pull/4580 to enable
mimalloc in MSVC builds, but I abandoned that effort, too, because Git
decided to favor Meson over first-class MSVC support and I decided to
focus on avoiding to have my time wasted by the Git project).

As you are quite aware (because it caused plenty of trouble with your
reftable patch series), Git for Windows switched to mimalloc quite a while
ago (https://github.com/microsoft/mimalloc).

When I switched Git for Windows to mimalloc, I did (re-)run a couple of
performance tests to see whether having a custom allocator is still
necessary, and from my (unfortunately too vague) recollection, Windows
11's default allocator seems to have performed quite well in comparison.
Which is in stark contrast to the results of the performance tests I ran
when originally integrating nedmalloc. So: In theory, Git for Windows
could drop building with a custom allocator, iff it wasn't for older
Windows version that are still supported.

Which means that I would like to upstream the vendored-in mimalloc first,
with the patch to use it when building on Windows by default, before
dropping nedmalloc from Git's source code.

Ciao,
Johannes

[gitgitgadget]

User Patrick Steinhardt <[email protected]> has been added to the cc: list.

[gitgitgadget]

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Mon, Apr 21, 2025 at 12:39:10PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/environment.c b/environment.c
> index 9e4c7781be0..cc853950bb2 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -82,9 +82,21 @@ int max_allowed_tree_depth =
>  	 * the stack overflow can occur.
>  	 */
>  	512;
> +#else
> +#if defined(GIT_WINDOWS_NATIVE) && defined(__clang__) && defined(__aarch64__)

Tiny nit, only because it puzzled me for a second: this should probably
be `#elif`.

> +	/*
> +	 * Similar to Visual C, it seems that on Windows/ARM64 the clang-based
> +	 * builds have a smaller stack space available. When running out of
> +	 * that stack space, a `STATUS_STACK_OVERFLOW` is produced. When the
> +	 * Git command was run from an MSYS2 Bash, this unfortunately results
> +	 * in an exit code 127. Let's prevent that by lowering the maximal
> +	 * tree depth; This value seems to be low enough.
> +	 */
> +	1280;
>  #else
>  	2048;
>  #endif
> +#endif

Hm. This whole construct feels rather awful, if you ask me. Instead of
papering over the issue it would be nice if we eventually fixed the root
cause, which is that we use recursion on a data structure that has an
unbounded depth in theory.

Anyway, that is clearly outside of the scope of this patch series, so
the bandaid is good enough for now.

Patrick

[gitgitgadget]

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Patrick,

On Tue, 22 Apr 2025, Patrick Steinhardt wrote:

> On Mon, Apr 21, 2025 at 12:39:10PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/environment.c b/environment.c
> > index 9e4c7781be0..cc853950bb2 100644
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -82,9 +82,21 @@ int max_allowed_tree_depth =
> >  	 * the stack overflow can occur.
> >  	 */
> >  	512;
> > +#else
> > +#if defined(GIT_WINDOWS_NATIVE) && defined(__clang__) && defined(__aarch64__)
> 
> Tiny nit, only because it puzzled me for a second: this should probably
> be `#elif`.

I will change it.

> > +	/*
> > +	 * Similar to Visual C, it seems that on Windows/ARM64 the clang-based
> > +	 * builds have a smaller stack space available. When running out of
> > +	 * that stack space, a `STATUS_STACK_OVERFLOW` is produced. When the
> > +	 * Git command was run from an MSYS2 Bash, this unfortunately results
> > +	 * in an exit code 127. Let's prevent that by lowering the maximal
> > +	 * tree depth; This value seems to be low enough.
> > +	 */
> > +	1280;
> >  #else
> >  	2048;
> >  #endif
> > +#endif
> 
> Hm. This whole construct feels rather awful, if you ask me. Instead of
> papering over the issue it would be nice if we eventually fixed the root
> cause, which is that we use recursion on a data structure that has an
> unbounded depth in theory.

True.

It is also quite awful that I cannot find a way to represent
`STATUS_STACK_OVERFLOW` by anything else than exit code 127, which always
misleads me into thinking that an executable or a DLL might be missing.
But I did not find any.

> Anyway, that is clearly outside of the scope of this patch series, so
> the bandaid is good enough for now.

True enough!

Thank you,
Johannes