From: Junio C Hamano <gits...@pobox.com>
Subject: Re: [PATCH] Makefile: detect when PYTHON_PATH changes
Date: Sat, 15 Dec 2012 09:29:48 -0800
> Christian Couder <chrisc...@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.
--
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
