From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>, Johannes Sixt <j6t@kdbg.org>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH] mingw: include the Python parts in the build
Date: Fri, 29 Jul 2022 16:29:47 +0200 (CEST) [thread overview]
Message-ID: <0s3o2pr2-o0q3-q394-83n3-n117355o8o29@tzk.qr> (raw)
In-Reply-To: <xmqqczdpcfhx.fsf@gitster.g>
Hi Junio,
On Thu, 28 Jul 2022, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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/
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
> We may want to see if there are other things that needs cleaning up
> around this area.
>
> config.mak.uname | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git i/config.mak.uname w/config.mak.uname
> index ce83cad47a..999a7ae270 100644
> --- i/config.mak.uname
> +++ w/config.mak.uname
> @@ -656,7 +656,6 @@ ifeq ($(uname_S),MINGW)
> UNRELIABLE_FSTAT = UnfortunatelyYes
> OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
> NO_REGEX = YesPlease
> - NO_PYTHON = YesPlease
> ETAGS_TARGET = ETAGS
> NO_POSIX_GOODIES = UnfortunatelyYes
> DEFAULT_HELP_FORMAT = html
> @@ -686,6 +685,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
> INTERNAL_QSORT = YesPlease
> HAVE_LIBCHARSET_H = YesPlease
> NO_GETTEXT = YesPlease
> + NO_PYTHON = YesPlease
> COMPAT_CFLAGS += -D__USE_MINGW_ACCESS
> else
> ifneq ($(shell expr "$(uname_R)" : '1\.'),2)
>
next prev parent reply other threads:[~2022-07-29 14:29 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-28 14:01 [PATCH] mingw: include the Python parts in the build Johannes Schindelin via GitGitGadget
2022-07-28 17:29 ` Junio C Hamano
2022-07-29 14:29 ` Johannes Schindelin [this message]
2022-07-29 21:31 ` Johannes Sixt
2022-08-10 9:29 ` Johannes Schindelin
2022-07-29 15:41 ` [PATCH v2 0/3] " Johannes Schindelin via GitGitGadget
2022-07-29 15:41 ` [PATCH v2 1/3] windows: include the Python bits when building Git for Windows Johannes Schindelin via GitGitGadget
2022-07-29 15:41 ` [PATCH v2 2/3] mingw: remove unneeded `NO_GETTEXT` directive Johannes Schindelin via GitGitGadget
2022-07-29 15:41 ` [PATCH v2 3/3] mingw: remove unneeded `NO_CURL` directive Johannes Schindelin via GitGitGadget
2022-07-29 15:56 ` [PATCH v2 0/3] mingw: include the Python parts in the build Johannes Schindelin
2022-07-29 16:58 ` Junio C Hamano
2022-08-10 9:33 ` Overriding creation factor in format-patch, was " Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0s3o2pr2-o0q3-q394-83n3-n117355o8o29@tzk.qr \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).