[PATCH] Makefile: detect when PYTHON_PATH changes

[PATCH] Makefile: detect when PYTHON_PATH changes

[Christian Couder]

When make is run the python script are created from *.py files that
are changed to use the python given by PYTHON_PATH. And PYTHON_PATH
is set by default to /usr/bin/python on Linux.

This is nice except when you run make another time setting a
different PYTHON_PATH, because, as the python scripts have already
been created, make finds nothing to do.

The goal of this patch is to detect when the PYTHON_PATH changes and
to create the python scripts again when this happens. To do that we
use the same trick that is done to track prefix, flags and tcl/tk
path. We update a GIT-PYTHON-VARS file with the PYTHON_PATH and check
if it changed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4ad6fbd..bd86063 100644
--- a/Makefile
+++ b/Makefile
@@ -2245,7 +2245,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
 endif # NO_PERL
 
 ifndef NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
 $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
@@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE
             fi
 endif
 
+### Detect Python interpreter path changes
+ifndef NO_PYTHON
+TRACK_VARS = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
+
+GIT-PYTHON-VARS: FORCE
+	@VARS='$(TRACK_VARS)'; \
+	    if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+		echo 1>&2 "    * new Python interpreter location"; \
+		echo "$$VARS" >$@; \
+            fi
+endif
+
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
-- 
1.8.1.rc1.1.g9537e17

Re: [PATCH] Makefile: detect when PYTHON_PATH changes

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> @@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE
>              fi
>  endif
>  
> +### Detect Python interpreter path changes
> +ifndef NO_PYTHON
> +TRACK_VARS = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
> +
> +GIT-PYTHON-VARS: FORCE
> +	@VARS='$(TRACK_VARS)'; \
> +	    if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
> +		echo 1>&2 "    * new Python interpreter location"; \
> +		echo "$$VARS" >$@; \
> +            fi
> +endif

Do we have the same issue when you decide to use /usr/local/bin/sh
after building with /bin/sh set to SHELL_PATH?

 - If yes, I presume that there will be follow-up patches to this
   one, for SHELL_PATH, PERL_PATH, and TCLTK_PATH (there may be
   other languages but I didn't bother to check).  How would the use
   of language agnostic looking TRACK_VARS variable in this patch
   affect such follow-up patches?

 - If no (in other words, if we rebuild shell-scripted porcelains
   when SHELL_PATH changes), wouldn't it be better to see how it is
   done and hook into the same mechanism?

 - If no, and if the approach taken in this patch is better than the
   one used to rebuild shell scripts (it may limit the scope of
   rebuilding when path changes, or something, but again, I didn't
   bother to check), then again follow-up patches to this one to
   describe dependencies to build scripts in other languages in a
   similar way to this patch would come in the future.  The same
   question regarding TRACK_VARS applies in this case.

By the way, "1>&2" is easier to read if written as just ">&2", I
think.

Thanks.

Re: [PATCH] Makefile: detect when PYTHON_PATH changes

[Christian Couder]

From: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] Makefile: detect when PYTHON_PATH changes
Date: Sat, 15 Dec 2012 09:29:48 -0800

> Christian Couder <chriscool@tuxfamily.org> writes:
> 
>> @@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE
>>              fi
>>  endif
>>  
>> +### Detect Python interpreter path changes
>> +ifndef NO_PYTHON
>> +TRACK_VARS = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
>> +
>> +GIT-PYTHON-VARS: FORCE
>> +	@VARS='$(TRACK_VARS)'; \
>> +	    if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
>> +		echo 1>&2 "    * new Python interpreter location"; \
>> +		echo "$$VARS" >$@; \
>> +            fi
>> +endif
> 
> Do we have the same issue when you decide to use /usr/local/bin/sh
> after building with /bin/sh set to SHELL_PATH?
> 
>  - If yes, I presume that there will be follow-up patches to this
>    one, for SHELL_PATH, PERL_PATH, and TCLTK_PATH (there may be
>    other languages but I didn't bother to check).  How would the use
>    of language agnostic looking TRACK_VARS variable in this patch
>    affect such follow-up patches?

Actually, just above the above hunk, there is:

### Detect Tck/Tk interpreter path changes
ifndef NO_TCLTK
TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')

GIT-GUI-VARS: FORCE
        @VARS='$(TRACK_VARS)'; \
            if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
                echo 1>&2 "    * new Tcl/Tk interpreter location"; \
                echo "$$VARS" >$@; \
            fi
endif

But you are right, TRACK_VARS should probably be something like
TRACK_TCLTK when used for TCLTK and TRACK_PYTHON when used for Python.

By the way it looks like GIT-GUI-VARS is not used, so perhaps the
detection of Tck/Tk interpreter path changes above could be removed.

(The detection is done in git-gui/Makefile too.)

About shell, there is the following:

SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
        $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
        $(gitwebdir_SQ):$(PERL_PATH_SQ)

and then

GIT-SCRIPT-DEFINES: FORCE
        @FLAGS='$(SCRIPT_DEFINES)'; \
            if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
                echo 1>&2 "    * new script parameters"; \
                echo "$$FLAGS" >$@; \
            fi


$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES
        $(QUIET_GEN)$(cmd_munge_script) && \
        chmod +x $@+ && \
        mv $@+ $@

$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
        $(QUIET_GEN)$(cmd_munge_script) && \
        mv $@+ $@

So it looks to me that at least for SHELL_PATH, things are done
properly. 

>  - If no (in other words, if we rebuild shell-scripted porcelains
>    when SHELL_PATH changes), wouldn't it be better to see how it is
>    done and hook into the same mechanism?

You would like me to just add $(PYTHON_PATH_SQ) in SCRIPT_DEFINES and
then use GIT-SCRIPT-DEFINES instead of GIT-PYTHON-VARS to decide if
python scripts should be rebuilt?

>  - If no, and if the approach taken in this patch is better than the
>    one used to rebuild shell scripts (it may limit the scope of
>    rebuilding when path changes, or something, but again, I didn't
>    bother to check), 

Yeah, I think it is better because it limits the scope of rebuilding,
and because nothing is done for Python, while there are some
mechanisms in place for other languages.

> then again follow-up patches to this one to
>    describe dependencies to build scripts in other languages in a
>    similar way to this patch would come in the future.  The same
>    question regarding TRACK_VARS applies in this case.

I agree about TRACK_VARS. About other languages, I will have another
look, but it seems that they already have what they need.

> By the way, "1>&2" is easier to read if written as just ">&2", I
> think.

Ok I will change this.

Thanks,
Christian.