Hi Junio,

On Thu, 28 Jul 2022, Junio C Hamano wrote:

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:

quoted

From: Johannes Schindelin <redacted>

While Git for Windows does not _ship_ Python (in order to save on
bandwidth), MSYS2 provides very fine Python interpreters that users can
easily take advantage of, by using Git for Windows within its SDK.

It may be an accurate description of the world and there may not be
anything incorrect in the above statement, but it took quite an
effort to try matching that statement to what the patch does.

I think

    Builds on $uname_S==MINGW by default sets NO_PYTHON=YesPlease
    and it benefits Git for Windows by allowing to omit Python.
    However, when "Git for Windows" is used within MSYS2's SDK, we
    can allow users to take advantage of Python interpreter that
    comes with it.  Override NO_PYTHON when the presence of
    ../THIS_IS_MSYSGIT indicates that we are in that situation.

is how the logic in this patch can be explained, but I have to
wonder if a more natural and easier-to-understand solution is to
move NO_PYTHON=YesPlease into "if we do not have ../THIS_IS_MSYSGIT,
do these things" ifneq() block, like the attached patch.

The outline is:

ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
	[...]
else
        ifneq ($(shell expr "$(uname_R)" : '1\.'),2)
                # MSys2
		[...]
	else
		[...]
	endif
endif

By moving it into the `THIS_IS_MSYSGIT` part, you changed the behavior of
the MSys part (which is in the `else` block of that `uname_R`
conditional).

Now, I vaguely remember that j6t said that they switched to MSYS2 some
time ago, but all I found on the Git mailing list was
https://lore.kernel.org/git/6c2bbca7-7a8f-d3d8-04b6-31494a3e1b43@kdbg.org/ (local)
which says that in 2017, MSys1 was still used by the person who apart from
myself did the most crucial work to support Git on Windows (and that
counts a lot in my book, so in this instance I am willing to bear a bit
more maintenance burden than I otherwise would for a single person, even
if the Windows-specific part of `config.mak.uname` is quite messy, I
admit).

Hannes, do you still build with MSys1?

I didn't touch it but NO_GETTEXT does not appear in the common
section above "do we have ../THIS_IS_MSYSGIT?", and gets set
after "we do not have ../THIS_IS_MSYSGIT", so I do not think
we need "NO_GETTEXT = " that clears it in the "we do have
../THIS_IS_MSYSGIT" part.

True. This is my mistake: in f9206ce2681 (mingw: let's use gettext with
MSYS2, 2016-01-26), I should have looked more closely and realized that
`NO_GETTEXT` is not defined in the MSYS2-specific part of
`config.mak.uname`, and hence the line should not have changed to
`NO_GETTEXT =` but it should have been removed instead.

I'll revamp the patch and send another iteration (but please do not expect
any further work from me this coming week, I plan on staying off of work).

Ciao,
Dscho