From: Junio C Hamano
Subject: Re: [PATCH] Makefile: detect when PYTHON_PATH changes
Date: Sat, 15 Dec 2012 09:29:48 -0800
> Christian Couder 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
