threads / patch / 22123

patchmingw: enable NO_PYTHON

Subject: [PATCH] mingw: enable NO_PYTHON

## tl;dr

9 messages between Jan 7, 2010 and Jan 8, 2010. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Erik Faye-Lund· Jan 7, 2010, 21:52 UTC · lore

Python is not commonly installed on Windows machines, so we should disable it there by default.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

This patch is against Junio's current master, and enables msysgit to compile upstream git again after Sverre's addition of the python remote-helpers (2fe40b6).

 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to Makefile +1 −0
diff --git a/Makefile b/Makefile
index 1c7668a..a2780a2 100644
--- a/Makefile
+++ b/Makefile
@@ -1028,6 +1028,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	BLK_SHA1 = YesPlease
 	NO_INET_PTON = YesPlease
 	NO_INET_NTOP = YesPlease
+	NO_PYTHON = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o

-- 
1.6.6.95.g82b1b.dirty
Erik Faye-Lund· Jan 7, 2010, 22:00 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] mingw: enable NO_PYTHON

Uhm, I just realized that I sent out a patch that wasn't clean against Junio's master after all. I'll send out a fixed one ASAP.

Sorry about the noise.

On Thu, Jan 7, 2010 at 10:52 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:

Show 29 quoted lines
> Python is not commonly installed on Windows machines, so
> we should disable it there by default.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>
> This patch is against Junio's current master, and enables
> msysgit to compile upstream git again after Sverre's addition
> of the python remote-helpers (2fe40b6).
>
>  Makefile |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1c7668a..a2780a2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1028,6 +1028,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>        BLK_SHA1 = YesPlease
>        NO_INET_PTON = YesPlease
>        NO_INET_NTOP = YesPlease
> +       NO_PYTHON = YesPlease
>        COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch
>        COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
>        COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o
> --
> 1.6.6.95.g82b1b.dirty
>
>
-- 
Erik "kusma" Faye-Lund
Erik Faye-Lund· Jan 7, 2010, 22:07 UTC · re: Erik Faye-Lund · lore

[PATCH] mingw: disable Python

Python is not commonly installed on Windows machines, so we should disable it there by default.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

This patch is against Junio's current master, and enables msysgit to compile upstream git again after Sverre's addition of the python remote-helpers (2fe40b6).

 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to Makefile +1 −0
diff --git a/Makefile b/Makefile
index 015bfab..0004c52 100644
--- a/Makefile
+++ b/Makefile
@@ -1027,6 +1027,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
 	NO_REGEX = YesPlease
 	BLK_SHA1 = YesPlease
+	NO_PYTHON = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o

-- 
1.6.6.90.g50bc9.dirty
Johannes Sixt· Jan 8, 2010, 08:07 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] mingw: disable Python

Erik Faye-Lund schrieb:
Show 7 quoted lines
> Python is not commonly installed on Windows machines, so
> we should disable it there by default.
> 
> --- a/Makefile
> +++ b/Makefile
> @@ -1027,6 +1027,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> +	NO_PYTHON = YesPlease

I'm worried that with this solution it is impossible to re-enable Python in config.mak (how do you undefine a Makefile variable?); it would be necessary to hack Makefile.

Wouldn't it be superior to set
	PYTHON_PATH =
in the MinGW section[*]. It works because there is this heuristic later:

ifeq ($(PYTHON_PATH),) NO_PYTHON=NoThanks endif

To enable Python, the user would have to set PYTHON_PATH in config.mak.

[I have only Python 1.6 to test (doh!), so I can only tell that it gets used during 'make', but this fails due to missing modules, so I cannot tell whether there would be a usable result if Python were sufficiently recent.]

[*] You should probably set the MSVC section as well, even if you cannot test it. The effect of the change is predictable enough, I think.

-- Hannes
Johannes Schindelin· Jan 8, 2010, 10:35 UTC · re: Johannes Sixt · lore

Re: [PATCH] mingw: disable Python

Hi,
On Fri, 8 Jan 2010, Johannes Sixt wrote:
Show 11 quoted lines
> Erik Faye-Lund schrieb:
> > Python is not commonly installed on Windows machines, so
> > we should disable it there by default.
> > 
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1027,6 +1027,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> > +	NO_PYTHON = YesPlease
> 
> I'm worried that with this solution it is impossible to re-enable Python
> in config.mak (how do you undefine a Makefile variable?);
How about
	NO_PYTHON=
in config.mak?

Ciao, Dscho

Erik Faye-Lund· Jan 8, 2010, 10:49 UTC · re: Johannes Schindelin · lore

Re: [PATCH] mingw: disable Python

On Fri, Jan 8, 2010 at 9:07 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Wouldn't it be superior to set
>
>        PYTHON_PATH =

Yes, I think it would. I've tested it (I've got Python 2.6 installed in c:\Python26\, so with your fix I can re-enable it by setting "PYTHON_PATH=/c/Python26/python.exe" in config.mak.

On Fri, Jan 8, 2010 at 11:35 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 15 quoted lines
> Hi,
>
> On Fri, 8 Jan 2010, Johannes Sixt wrote:
>
>> Erik Faye-Lund schrieb:
>> > Python is not commonly installed on Windows machines, so
>> > we should disable it there by default.
>> >
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -1027,6 +1027,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>> > +   NO_PYTHON = YesPlease
>>
>> I'm worried that with this solution it is impossible to re-enable Python
>> in config.mak (how do you undefine a Makefile variable?);
Show 6 quoted lines
> How about
>
>        NO_PYTHON=
>
> in config.mak?
>

That doesn't work for me, at least not out of the box. NO_PYTHON is still defined, it's just defined to an empty string. I guess we could change to Makefile to accept empty NO_PYTHON as enabled, but since Hannes' suggestion works fine, I think I'll stick with it, even if it's a little inconsistent with the other stuff in the MinGW-section of Makefile.

I'll resend a bit later.
-- 
Erik "kusma" Faye-Lund
Junio C Hamano· Jan 8, 2010, 13:35 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] mingw: disable Python

Erik Faye-Lund <kusmabite@googlemail.com> writes:
Show 22 quoted lines
>> On Fri, 8 Jan 2010, Johannes Sixt wrote:
>>
>>> Erik Faye-Lund schrieb:
>>> > Python is not commonly installed on Windows machines, so
>>> > we should disable it there by default.
>>> >
>>> > --- a/Makefile
>>> > +++ b/Makefile
>>> > @@ -1027,6 +1027,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>>> > +   NO_PYTHON = YesPlease
>>>
>>> I'm worried that with this solution it is impossible to re-enable Python
>>> in config.mak (how do you undefine a Makefile variable?);
>
>> How about
>>
>>        NO_PYTHON=
>>
>> in config.mak?
>
> That doesn't work for me, at least not out of the box. NO_PYTHON is
> still defined, it's just defined to an empty string.

I think Dscho is right. "ifdef FOO" yields (and should yield) false if FOO is set to empty.

-- >8 -- cut here and try it for yourself -- >8 -- #!/bin/sh

rm -f config.mk
cat >Makefile <<\EOF
# The default is...
all::
FROTZ = NITFOL
-include config.mk
ifdef FROTZ
all::
	echo FROTZ is $(FROTZ)
else
all::
	echo FROTZ is not defined
endif
EOF

make # says "NITFOL"

echo "FROTZ =" >config.mk make # says "not defined"

exit -- 8< --

Johannes Sixt· Jan 8, 2010, 13:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] mingw: disable Python

Junio C Hamano schrieb:
Show 13 quoted lines
> Erik Faye-Lund <kusmabite@googlemail.com> writes:
>>> On Fri, 8 Jan 2010, Johannes Sixt wrote:
>>>> (how do you undefine a Makefile variable?);
>>> How about
>>>
>>>        NO_PYTHON=
>>>
>>> in config.mak?
>> That doesn't work for me, at least not out of the box. NO_PYTHON is
>> still defined, it's just defined to an empty string.
> 
> I think Dscho is right.  "ifdef FOO" yields (and should yield) false if
> FOO is set to empty.

Indeed. Strange. It didn't work when I tested it a few hours ago, but no it works as you say. According to the docs, 'ifdef' actually does not test defined-ness, but emptyness. So, Erik's original version is fine:

Acked-by: Johannes Sixt <j6t@kdbg.org>
-- Hannes
Erik Faye-Lund· Jan 8, 2010, 13:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] mingw: disable Python

On Fri, Jan 8, 2010 at 2:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> Erik Faye-Lund <kusmabite@googlemail.com> writes:
>
>>> On Fri, 8 Jan 2010, Johannes Sixt wrote:
>>>
>>>> Erik Faye-Lund schrieb:
>>>> > Python is not commonly installed on Windows machines, so
>>>> > we should disable it there by default.
>>>> >
>>>> > --- a/Makefile
>>>> > +++ b/Makefile
>>>> > @@ -1027,6 +1027,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>>>> > +   NO_PYTHON = YesPlease
>>>>
>>>> I'm worried that with this solution it is impossible to re-enable Python
>>>> in config.mak (how do you undefine a Makefile variable?);
>>
>>> How about
>>>
>>>        NO_PYTHON=
>>>
>>> in config.mak?
>>
>> That doesn't work for me, at least not out of the box. NO_PYTHON is
>> still defined, it's just defined to an empty string.
>
> I think Dscho is right.  "ifdef FOO" yields (and should yield) false if
> FOO is set to empty.
>
In that case I guess I messed up my test. I'll retest.
Yes, I messed up. "NO_PYTHON=" works like a charm.
So, no repost :)
-- 
Erik "kusma" Faye-Lund

← back to recent threads