Re: [PATCH 1/2] mingw: remove duplicate `USE_NED_ALLOCATOR` directive

[Junio C Hamano <gitster@pobox.com>]

"Harshil-Jani via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harshil-Jani <harshiljani2002@gmail.com>
>
> nedalloc was added to fix the slowness of memory allocator. Here
> specifically for the MSys2 build there seems to be a duplication of
> USE_NED_ALLOCATOR directive.

Yes, if your platform's "uname -S" says "MINGW", your build is done
in a directory with ../THIS_IS_MSYSGIT marker file, and your "uname -R"
does not begin with "1.", then there are two USE_NED_ALLOCATOR=YesPlease
in effect.

> So this patch intends to remove the
> duplicate USE_NED_ALLOCATOR and keeping it only into the MSys2 config
> section so it still uses the nedalloc.

What about other folks whose "uname -S" says "MINGW"?

> Signed-off-by: Harshil-Jani <harshiljani2002@gmail.com>
> ---
>  config.mak.uname | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index d63629fe807..377667c4bbc 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -652,7 +652,6 @@ ifeq ($(uname_S),MINGW)
>  	USE_WIN32_IPC = YesPlease
>  	USE_WIN32_MMAP = YesPlease
>  	MMAP_PREVENTS_DELETE = UnfortunatelyYes
> -	USE_NED_ALLOCATOR = YesPlease
>  	UNRELIABLE_FSTAT = UnfortunatelyYes
>  	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
>  	NO_REGEX = YesPlease

The original in a wider context looks like this:

        ifeq ($(uname_S),MINGW)
                ...
                MMAP_PREVENTS_DELETE = UnfortunatelyYes
                USE_NED_ALLOCATOR = YesPlease
                UNRELIABLE_FSTAT = UnfortunatelyYes
                ...
                X = .exe
        ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
                htmldir = doc/git/html/
                ...
                COMPAT_CFLAGS += -D__USE_MINGW_ACCESS
        else
                ifneq ($(shell expr "$(uname_R)" : '1\.'),2)
                        # MSys2
                        ...
                        USE_LIBPCRE = YesPlease
                        USE_NED_ALLOCATOR = YesPlease
                        ifeq (/mingw64,$(subst 32,64,$(prefix)))
                                # Move system config into top-level /etc/
                                ...
                        endif
                else
                        ...
                endif
        endif
        endif

With this patch, a build that has ../THIS_IS_MSYSGIT marker file, or
whose "uname -R" output begins with "1.", will no longer get
USE_NED_ALLOCATOR.  Intended?

Without knowing much about the Windows/MSYS/Git for Windows SDK
ecosystem, it is the inner one that looks duplicated, but the patch
is removing the outer one that helps every platform whose "uname -S"
identifies it as MINGW.

Perhaps that is what the patch meant to do, but then the proposed
log message explains it very differently.  It only talks about the
no-op case.  It does not explain why other folks whose "uname -S"
says MINGW (1) are not broken if this patch suddenly robs NED
allocator from them, and (2) are better off without using NED
allocator.