Skip to content

Support Windows/ARM64 #1904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support Windows/ARM64 #1904

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b89f39c
bswap.h: add support for built-in bswap functions
dennisameling Oct 4, 2022
2feeadb
config.mak.uname: add support for clangarm64
dennisameling Oct 4, 2022
6c2e17e
mingw: do not use nedmalloc on Windows/ARM64
dscho Apr 20, 2025
c89ead8
msvc: do handle builds on Windows/ARM64
dscho Feb 25, 2025
939bcb0
mingw(arm64): do move the `/etc/git*` location
dscho Feb 25, 2025
e0e78bd
max_tree_depth: lower it for clangarm64 on Windows
dscho Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion compat/bswap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,19 @@ static inline uint64_t default_bswap64(uint64_t val)
#undef bswap32
#undef bswap64

#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
/**
* __has_builtin is available since Clang 10 and GCC 10.
* Below is a fallback for older compilers.
*/
#ifndef __has_builtin
#define __has_builtin(x) 0
#endif

#if __has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64)
#define bswap32(x) __builtin_bswap32((x))
#define bswap64(x) __builtin_bswap64((x))

#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))

#define bswap32 git_bswap32
static inline uint32_t git_bswap32(uint32_t x)
Expand Down
8 changes: 7 additions & 1 deletion config.mak.uname
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,10 @@ ifeq ($(uname_S),MINGW)
prefix = /mingw64
HOST_CPU = x86_64
BASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup
else ifeq (CLANGARM64,$(MSYSTEM))
prefix = /clangarm64
HOST_CPU = aarch64
BASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup
else
COMPAT_CFLAGS += -D_USE_32BIT_TIME_T
BASIC_LDFLAGS += -Wl,--large-address-aware
Expand All @@ -738,7 +742,9 @@ ifeq ($(uname_S),MINGW)
HAVE_LIBCHARSET_H = YesPlease
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

> -	USE_NED_ALLOCATOR = YesPlease
> +	ifneq (CLANGARM64,$(MSYSTEM))
> +		USE_NED_ALLOCATOR = YesPlease
> +	endif
>          ifeq (/mingw64,$(subst 32,64,$(prefix)))

Notice the funny indentation above?  It turns out that the one in
the context that looks funnily indented uses the "correct"
indentation, which is quite counter-intuitive and confusing X-<.

I forgot about the rule while reviewing the previous round, but we
had to prepare for newer GNU make with commits like c18400c6
(Makefile(s): avoid recipe prefix in conditional statements,
2024-04-08).  In short, the conditionals like ifn?eq, ifn?def, else,
endif should not be indented with HT and we instead want them to be
indented with SP.

    $ git diff master... config.mak.uname |
      grep -E -e '^[+]       +(ifn?eq|ifn?def|else|endif)'

found them in a few patches in the series that touch
config.mak.uname

    msvc: do handle builds on Windows/ARM64
    mingw: do not use nedmalloc on Windows/ARM64

Fix-up for "mingw: do not use nedmalloc on Windows/ARM64"

 config.mak.uname | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git c/config.mak.uname w/config.mak.uname
index 6222d2c5a4..3ec82d95e6 100644
--- c/config.mak.uname
+++ w/config.mak.uname
@@ -742,9 +742,9 @@ ifeq ($(uname_S),MINGW)
 	HAVE_LIBCHARSET_H = YesPlease
 	USE_GETTEXT_SCHEME = fallthrough
 	USE_LIBPCRE = YesPlease
-	ifneq (CLANGARM64,$(MSYSTEM))
+        ifneq (CLANGARM64,$(MSYSTEM))
 		USE_NED_ALLOCATOR = YesPlease
-	endif
+        endif
         ifeq (/mingw64,$(subst 32,64,$(prefix)))
 		# Move system config into top-level /etc/
 		ETC_GITCONFIG = ../etc/gitconfig


Fix-up for "msvc: do handle builds on Windows/ARM64"
    
 config.mak.uname | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git c/config.mak.uname w/config.mak.uname
index 4831e9ccf6..4ef453ebcd 100644
--- c/config.mak.uname
+++ w/config.mak.uname
@@ -432,11 +432,11 @@ ifeq ($(uname_S),Windows)
         ifeq (MINGW32,$(MSYSTEM))
 		prefix = /mingw32
         else
-		ifeq (CLANGARM64,$(MSYSTEM))
+                ifeq (CLANGARM64,$(MSYSTEM))
 			prefix = /clangarm64
-		else
+                else
 			prefix = /mingw64
-		endif
+                endif
         endif
 	# Prepend MSVC 64-bit tool-chain to PATH.
 	#

Taken as a whole, here is a range-diff of what I will queue based on
this iteration.

Thanks.

1:  da1408a34e ! 1:  734bf24007 mingw: do not use nedmalloc on Windows/ARM64
    @@ Commit message
         there is also no hope that any fixes will materialize there.
     
         Signed-off-by: Johannes Schindelin <[email protected]>
    +    [jc: adjust config.mak.uname for c18400c6]
         Signed-off-by: Junio C Hamano <[email protected]>
     
      ## config.mak.uname ##
    @@ config.mak.uname: ifeq ($(uname_S),MINGW)
      	USE_GETTEXT_SCHEME = fallthrough
      	USE_LIBPCRE = YesPlease
     -	USE_NED_ALLOCATOR = YesPlease
    -+	ifneq (CLANGARM64,$(MSYSTEM))
    ++        ifneq (CLANGARM64,$(MSYSTEM))
     +		USE_NED_ALLOCATOR = YesPlease
    -+	endif
    ++        endif
              ifeq (/mingw64,$(subst 32,64,$(prefix)))
      		# Move system config into top-level /etc/
      		ETC_GITCONFIG = ../etc/gitconfig
2:  e27caa3dca ! 2:  8945fba590 msvc: do handle builds on Windows/ARM64
    @@ Commit message
         is time to do the same in the MS Visual C part.
     
         Signed-off-by: Johannes Schindelin <[email protected]>
    +    [jc: adjust config.mak.uname for c18400c6]
         Signed-off-by: Junio C Hamano <[email protected]>
     
      ## config.mak.uname ##
    @@ config.mak.uname: ifeq ($(uname_S),Windows)
      		prefix = /mingw32
              else
     -		prefix = /mingw64
    -+		ifeq (CLANGARM64,$(MSYSTEM))
    ++                ifeq (CLANGARM64,$(MSYSTEM))
     +			prefix = /clangarm64
    -+		else
    ++                else
     +			prefix = /mingw64
    -+		endif
    ++                endif
              endif
      	# Prepend MSVC 64-bit tool-chain to PATH.
      	#
3:  f1f6c1f2fa ! 3:  619950d421 mingw(arm64): do move the `/etc/git*` location
    @@ config.mak.uname: ifeq ($(uname_S),Windows)
      	ETC_GITCONFIG = ../etc/gitconfig
      	ETC_GITATTRIBUTES = ../etc/gitattributes
     @@ config.mak.uname: ifeq ($(uname_S),MINGW)
    - 	ifneq (CLANGARM64,$(MSYSTEM))
    +         ifneq (CLANGARM64,$(MSYSTEM))
      		USE_NED_ALLOCATOR = YesPlease
    - 	endif
    +         endif
     -        ifeq (/mingw64,$(subst 32,64,$(prefix)))
     +        ifeq (/mingw64,$(subst 32,64,$(subst clangarm,mingw,$(prefix))))
      		# Move system config into top-level /etc/
4:  687bd4ea96 = 4:  436a42215e max_tree_depth: lower it for clangarm64 on Windows

USE_GETTEXT_SCHEME = fallthrough
USE_LIBPCRE = YesPlease
USE_NED_ALLOCATOR = YesPlease
ifneq (CLANGARM64,$(MSYSTEM))
USE_NED_ALLOCATOR = YesPlease
endif
ifeq (/mingw64,$(subst 32,64,$(prefix)))
# Move system config into top-level /etc/
ETC_GITCONFIG = ../etc/gitconfig
Expand Down