patchMakefile: detect when PYTHON_PATH changes

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(-)

Christian Couder <chriscool@tuxfamily.org> writes:

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.

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

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.

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?

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.

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

Ok I will change this.

Thanks, Christian.