{"thread":{"id":"32346","subject":"[PATCH] Makefile: detect when PYTHON_PATH changes","startedAt":"2012-12-15T14:07:18Z","lastAt":"2012-12-15T20:09:50Z","messageCount":3,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"204908","messageId":"20121215140719.2409.27365.chriscool@tuxfamily.org","threadId":"32346","inReplyTo":null,"subject":"[PATCH] Makefile: detect when PYTHON_PATH changes","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2012-12-15T14:07:18Z","receivedAt":"2012-12-15T14:07:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When make is run the python script are created from *.py files that\nare changed to use the python given by PYTHON_PATH. And PYTHON_PATH\nis set by default to /usr/bin/python on Linux.\n\nThis is nice except when you run make another time setting a\ndifferent PYTHON_PATH, because, as the python scripts have already\nbeen created, make finds nothing to do.\n\nThe goal of this patch is to detect when the PYTHON_PATH changes and\nto create the python scripts again when this happens. To do that we\nuse the same trick that is done to track prefix, flags and tcl/tk\npath. We update a GIT-PYTHON-VARS file with the PYTHON_PATH and check\nif it changed.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Makefile | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4ad6fbd..bd86063 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2245,7 +2245,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n endif # NO_PERL\n \n ifndef NO_PYTHON\n-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX\n+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n@@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE\n             fi\n endif\n \n+### Detect Python interpreter path changes\n+ifndef NO_PYTHON\n+TRACK_VARS = $(subst ','\\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')\n+\n+GIT-PYTHON-VARS: FORCE\n+\t@VARS='$(TRACK_VARS)'; \\\n+\t    if test x\"$$VARS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n+\t\techo 1>&2 \"    * new Python interpreter location\"; \\\n+\t\techo \"$$VARS\" >$@; \\\n+            fi\n+endif\n+\n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n \n all:: $(TEST_PROGRAMS) $(test_bindir_programs)\n-- \n1.8.1.rc1.1.g9537e17\n"},{"id":"204911","messageId":"7vehirustv.fsf@alter.siamese.dyndns.org","threadId":"32346","inReplyTo":"20121215140719.2409.27365.chriscool@tuxfamily.org","subject":"Re: [PATCH] Makefile: detect when PYTHON_PATH changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-15T17:29:48Z","receivedAt":"2012-12-15T17:29:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> @@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE\n>              fi\n>  endif\n>  \n> +### Detect Python interpreter path changes\n> +ifndef NO_PYTHON\n> +TRACK_VARS = $(subst ','\\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')\n> +\n> +GIT-PYTHON-VARS: FORCE\n> +\t@VARS='$(TRACK_VARS)'; \\\n> +\t    if test x\"$$VARS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n> +\t\techo 1>&2 \"    * new Python interpreter location\"; \\\n> +\t\techo \"$$VARS\" >$@; \\\n> +            fi\n> +endif\n\nDo we have the same issue when you decide to use /usr/local/bin/sh\nafter building with /bin/sh set to SHELL_PATH?\n\n - If yes, I presume that there will be follow-up patches to this\n   one, for SHELL_PATH, PERL_PATH, and TCLTK_PATH (there may be\n   other languages but I didn't bother to check).  How would the use\n   of language agnostic looking TRACK_VARS variable in this patch\n   affect such follow-up patches?\n\n - If no (in other words, if we rebuild shell-scripted porcelains\n   when SHELL_PATH changes), wouldn't it be better to see how it is\n   done and hook into the same mechanism?\n\n - If no, and if the approach taken in this patch is better than the\n   one used to rebuild shell scripts (it may limit the scope of\n   rebuilding when path changes, or something, but again, I didn't\n   bother to check), then again follow-up patches to this one to\n   describe dependencies to build scripts in other languages in a\n   similar way to this patch would come in the future.  The same\n   question regarding TRACK_VARS applies in this case.\n\nBy the way, \"1>&2\" is easier to read if written as just \">&2\", I\nthink.\n\nThanks.\n"},{"id":"204945","messageId":"20121215.210950.1929890617364602070.chriscool@tuxfamily.org","threadId":"32346","inReplyTo":"7vehirustv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: detect when PYTHON_PATH changes","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2012-12-15T20:09:50Z","receivedAt":"2012-12-15T20:09:50Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\nSubject: Re: [PATCH] Makefile: detect when PYTHON_PATH changes\nDate: Sat, 15 Dec 2012 09:29:48 -0800\n\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> \n>> @@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE\n>>              fi\n>>  endif\n>>  \n>> +### Detect Python interpreter path changes\n>> +ifndef NO_PYTHON\n>> +TRACK_VARS = $(subst ','\\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')\n>> +\n>> +GIT-PYTHON-VARS: FORCE\n>> +\t@VARS='$(TRACK_VARS)'; \\\n>> +\t    if test x\"$$VARS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n>> +\t\techo 1>&2 \"    * new Python interpreter location\"; \\\n>> +\t\techo \"$$VARS\" >$@; \\\n>> +            fi\n>> +endif\n> \n> Do we have the same issue when you decide to use /usr/local/bin/sh\n> after building with /bin/sh set to SHELL_PATH?\n> \n>  - If yes, I presume that there will be follow-up patches to this\n>    one, for SHELL_PATH, PERL_PATH, and TCLTK_PATH (there may be\n>    other languages but I didn't bother to check).  How would the use\n>    of language agnostic looking TRACK_VARS variable in this patch\n>    affect such follow-up patches?\n\nActually, just above the above hunk, there is:\n\n### Detect Tck/Tk interpreter path changes\nifndef NO_TCLTK\nTRACK_VARS = $(subst ','\\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')\n\nGIT-GUI-VARS: FORCE\n        @VARS='$(TRACK_VARS)'; \\\n            if test x\"$$VARS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n                echo 1>&2 \"    * new Tcl/Tk interpreter location\"; \\\n                echo \"$$VARS\" >$@; \\\n            fi\nendif\n\nBut you are right, TRACK_VARS should probably be something like\nTRACK_TCLTK when used for TCLTK and TRACK_PYTHON when used for Python.\n\nBy the way it looks like GIT-GUI-VARS is not used, so perhaps the\ndetection of Tck/Tk interpreter path changes above could be removed.\n\n(The detection is done in git-gui/Makefile too.)\n\nAbout shell, there is the following:\n\nSCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n        $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n        $(gitwebdir_SQ):$(PERL_PATH_SQ)\n\nand then\n\nGIT-SCRIPT-DEFINES: FORCE\n        @FLAGS='$(SCRIPT_DEFINES)'; \\\n            if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n                echo 1>&2 \"    * new script parameters\"; \\\n                echo \"$$FLAGS\" >$@; \\\n            fi\n\n\n$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES\n        $(QUIET_GEN)$(cmd_munge_script) && \\\n        chmod +x $@+ && \\\n        mv $@+ $@\n\n$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n        $(QUIET_GEN)$(cmd_munge_script) && \\\n        mv $@+ $@\n\nSo it looks to me that at least for SHELL_PATH, things are done\nproperly. \n\n>  - If no (in other words, if we rebuild shell-scripted porcelains\n>    when SHELL_PATH changes), wouldn't it be better to see how it is\n>    done and hook into the same mechanism?\n\nYou would like me to just add $(PYTHON_PATH_SQ) in SCRIPT_DEFINES and\nthen use GIT-SCRIPT-DEFINES instead of GIT-PYTHON-VARS to decide if\npython scripts should be rebuilt?\n\n>  - If no, and if the approach taken in this patch is better than the\n>    one used to rebuild shell scripts (it may limit the scope of\n>    rebuilding when path changes, or something, but again, I didn't\n>    bother to check), \n\nYeah, I think it is better because it limits the scope of rebuilding,\nand because nothing is done for Python, while there are some\nmechanisms in place for other languages.\n\n> then again follow-up patches to this one to\n>    describe dependencies to build scripts in other languages in a\n>    similar way to this patch would come in the future.  The same\n>    question regarding TRACK_VARS applies in this case.\n\nI agree about TRACK_VARS. About other languages, I will have another\nlook, but it seems that they already have what they need.\n\n> By the way, \"1>&2\" is easier to read if written as just \">&2\", I\n> think.\n\nOk I will change this.\n\nThanks,\nChristian.\n"}]}