{"thread":{"id":"35806","subject":"[PATCH/RFC 0/13] makefile refactoring","startedAt":"2014-02-05T17:48:23Z","lastAt":"2014-02-10T01:15:14Z","messageCount":28,"participants":["Jeff King","Junio C Hamano","Eric Sunshine","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"234293","messageId":"20140205174823.GA15070@sigill.intra.peff.net","threadId":"35806","inReplyTo":null,"subject":"[PATCH/RFC 0/13] makefile refactoring","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:48:23Z","receivedAt":"2014-02-05T17:48:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This started with the desire to move the setting of the LESS and LV\nenvironment variables into a Makefile knob. But there's a fair bit of\ninfrastructure involved in that, and this is an attempt to factor out\nsome of that infrastructure to be more easily reusable. And if we like\nthe approach, we can move more build-time config in this direction.\n\nThere are a couple of things going on here, but the main ideas are:\n\n 1. Try to get build-time data into files as much as possible, because\n the one thing make understands is dependencies between files. Right\n now we rely on sentinel files like GIT-CFLAGS, which have two\n downsides:\n\n a. It is easy to miss a dependency that should be in a sentinel\n file, leading to failure to rebuild when we should.\n\n b. Because they are so cumbersome to use, we tend to put a lot of\n items into a small number of sentinel files, leading to\n unnecessary rebuilds (e.g., turning on XDL_FAST_HASH\n recompiles _everything_, even though only one C file cares\n about it).\n\n 2. Some light meta-programming to avoid repeating ourselves and try to\n make a few things more readable. I've done this here with $(call)\n and $(eval), which are basically the only way to do this in GNU\n make (and here we are relying heavily on GNU make, but as far as I\n know, nobody has had a huge problem with that in the past).\n\n Frankly, some of it is kind of ugly. And there's some potential for\n portability/version problems, just because we're not using more\n advanced features. If we don't like that approach, an alternative\n is to generate snippets of Makefile in a separate script and\n include them (like we do already for GIT-VERSION-FILE). That would\n let us write in whatever language we want, and avoid portability\n problems. The downside is that it may be less obvious to a reader\n not familiar with the system (e.g., you cannot necessarily grep for\n all the rules in Makefile, though that is already somewhat the case\n with pattern rules).\n\nThe two potential criticisms I expect are:\n\n 1. Portability issues with $(call), as mentioned above. We avoided\n this in 2006, but it may be sufficiently available now. See patch 3\n for exact numbers/versions.\n\n 2. Some people may simply find it ugly or too confusing for the\n benefit. Hence the RFC. :)\n\nWhile I tried to polish these patches enough to be applied, please take\nthis mostly as an RFC on the ideas and direction. There are multiple\nalternatives to implement some of these things, and I mainly want to see\nif people think this sort of make meta-programming is a good idea. The\npatches are:\n\n [01/13]: Makefile: drop USE_GETTEXT_SCHEME from GIT-CFLAGS\n [02/13]: Makefile: fix git-instaweb dependency on gitweb\n\n These first two are cleanups I noticed in the area, and can be\n applied regardless of the rest.\n\n [03/13]: Makefile: introduce make-var helper function\n [04/13]: Makefile: use tempfile/mv strategy for GIT-*\n [05/13]: Makefile: prefer printf to echo for GIT-*\n [06/13]: Makefile: store GIT-* sentinel files in MAKE/\n [07/13]: Makefile: always create files via make-var\n\n These ones factor out and improve the GIT-* file handling. Even if\n we decide not to go this route, patches 4, 5, and 7 are improvements\n that could apply to the current code. I didn't float them to the top\n of the series because it would involve making the same change in\n several different spots. If we decide not to apply this series, I\n can re-roll them as appropriate.\n\n [08/13]: Makefile: introduce sq function for shell-quoting\n [09/13]: Makefile: add c-quote helper function\n [10/13]: Makefile: drop *_SQ variables\n\n If we accept that we can use $(call), these are further readability\n cleanups we can do. They are technically optional, though, with\n respect to the rest of the series.\n\n [11/13]: Makefile: auto-build C strings from make variables\n [12/13]: Makefile: teach scripts to include make variables\n\n These ones lay the groundwork for easily getting make variables into\n shell scripts and C programs.\n\n [13/13]: move LESS/LV pager environment to Makefile\n\n And this one is the point of the series, which is fairly\n straightforward because of the earlier groundwork.\n\n-Peff\n"},{"id":"234294","messageId":"20140205174901.GA15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 01/13] Makefile: drop USE_GETTEXT_SCHEME from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:49:02Z","receivedAt":"2014-02-05T17:49:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This variable is used only by shell scripts, not by C\nprograms. It was originally included in GIT-CFLAGS as part\nof ad17ea7 (add a Makefile switch to avoid gettext\ntranslation in shell scripts, 2012-01-23), in an attempt to\ntrigger rebuilding when the variable changed.\n\nHowever, shell scripts do not respect GIT-CFLAGS. Later,\ne4dd89a (Makefile: update scripts when build-time parameters\nchange, 2012-06-20) introduced a separate GIT-SCRIPT-DEFINES\nto accomplish this, which also included USE_GETTEXT_SCHEME.\nWe can drop the redundant and useless mention in GIT-CFLAGS.\n\nSigned-off-by: Jeff King \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex dddaf4f..3cd4a92 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2164,7 +2164,7 @@ GIT-PREFIX: FORCE\n \t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n \tfi\n \n-TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME)\n+TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS))\n \n GIT-CFLAGS: FORCE\n \t@FLAGS='$(TRACK_CFLAGS)'; \\\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234295","messageId":"20140205174923.GB15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 02/13] Makefile: fix git-instaweb dependency on gitweb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:49:23Z","receivedAt":"2014-02-05T17:49:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We build git-instaweb if NO_PERL is not defined, and it in\nturns depends on \"gitweb\", which causes us to descend into\nthe gitweb subdir and recursively invoke make. But because\ngitweb is marked as .PHONY, this forces a rebuild of\ngit-instaweb every time we run make.\n\nWe can drop the dependency of git-instaweb on gitweb.\nHowever, that means we do not recurse into \"gitweb\" at all,\nso we have to add back in the dependency to \"all\".\n\nNote that the other subdirectories (like git-gui/ or perl/)\nare handled directly as build commands of \"all\", and we\ncould simply do the same thing here. However, this patch\nkeeps \"gitweb\" as a target so that \"make gitweb\" continues\nto work, in case anybody is using that.\n\nSigned-off-by: Jeff King \n---\n Makefile | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 3cd4a92..1f3e5d9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1818,8 +1818,9 @@ GIT-PERL-DEFINES: FORCE\n .PHONY: gitweb\n gitweb:\n \t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n+all:: gitweb\n \n-git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES\n+git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234296","messageId":"20140205175002.GC15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 03/13] Makefile: introduce make-var helper function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:50:02Z","receivedAt":"2014-02-05T17:50:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It's a common pattern in our Makefile to echo some make\nvariables into a file, but only if they are different from a\nprevious run. This sentinel file can then be used as a\ndependency to trigger rebuilds when the make variable\nchanges.\n\nThe code to do this is a bit ugly and repetetive; let's\nfactor it out into a reusable function.\n\nNote that this relies on the \"call\" and \"eval\" functions of\nGNU make. We previously avoided using \"call\", as explained\nin 39c015c (Fixes for ancient versions of GNU make,\n2006-02-18). However, it has been 8 years since then, so\nperhaps its use is acceptable now.\n\nThe \"call\" function dates back to GNU make 3.77.90\n(1997-07-21). The \"eval\" function dates back to 3.80\n(2002-07-08).\n\nIf it's still a problem to use these functions, we can\ndo similar meta-programming with something like:\n\n\tinclude magic.mak\n\tmagic.mak:\n\t\t./generate-magic-rules >$@+\n\t\tmv $@+ $@\n\nwhere the rules are generated by a shell (or other) script.\n\nSigned-off-by: Jeff King \n---\n Makefile | 105 ++++++++++++++++++++++++---------------------------------------\n 1 file changed, 40 insertions(+), 65 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1f3e5d9..50bf252 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1561,6 +1561,20 @@ ifneq (\"$(PROFILE)\",\"\")\n endif\n endif\n \n+# usage: $(eval $(call make-var,FN,DESC,CONTENTS))\n+#\n+# Create a rule to write $CONTENTS (which should come from a make variable)\n+# to GIT-$FN, but only if not already there. This can be used to create a\n+# dependency on a Makefile variable. Prints $DESC to the user.\n+define make-var\n+GIT-$1: FORCE\n+\t@VALUE='$$(subst ','\\'',$3)'; \\\n+\tif test x\"$$$$VALUE\" != x\"`cat $$@ 2>/dev/null`\"; then \\\n+\t\techo >&2 \" * new $2\"; \\\n+\t\techo \"$$$$VALUE\" >$$@; \\\n+\tfi\n+endef\n+\n # Shell quote (do not use $(call) to accommodate ancient setups);\n \n SHA1_HEADER_SQ = $(subst ','\\'',$(SHA1_HEADER))\n@@ -1615,13 +1629,9 @@ SHELL_PATH_CQ_SQ = $(subst ','\\'',$(SHELL_PATH_CQ))\n BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)'\n endif\n \n-GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n-GIT-USER-AGENT: FORCE\n-\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n-\t\techo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \\\n-\tfi\n+$(eval $(call make-var,USER-AGENT,user agent string,$(GIT_USER_AGENT)))\n \n ifdef DEFAULT_HELP_FORMAT\n BASIC_CFLAGS += -DDEFAULT_HELP_FORMAT='\"$(DEFAULT_HELP_FORMAT)\"'\n@@ -1737,9 +1747,17 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt\n common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n-SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n-\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n-\t$(gitwebdir_SQ):$(PERL_PATH_SQ)\n+$(eval $(call make-var,SCRIPT-DEFINES,script parameters,\\\n+\t:$(SHELL_PATH)\\\n+\t:$(DIFF)\\\n+\t:$(GIT_VERSION)\\\n+\t:$(localedir)\\\n+\t:$(NO_CURL)\\\n+\t:$(USE_GETTEXT_SCHEME)\\\n+\t:$(SANE_TOOL_PATH)\\\n+\t:$(gitwebdir)\\\n+\t:$(PERL_PATH)\\\n+))\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -1754,14 +1772,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n $@.sh >$@+\n endef\n \n-GIT-SCRIPT-DEFINES: FORCE\n-\t@FLAGS='$(SCRIPT_DEFINES)'; \\\n-\t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n-\t\techo >&2 \" * new script parameters\"; \\\n-\t\techo \"$$FLAGS\" >$@; \\\n- fi\n-\n-\n $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n@@ -1789,7 +1799,10 @@ perl/PM.stamp: FORCE\n perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n-PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)\n+$(eval $(call make-var,PERL-DEFINES,perl-specific parameters,\\\n+\t:$(PERL_PATH)\\\n+\t:$(PERLLIB_EXTRA)\\\n+))\n $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n@@ -1807,14 +1820,6 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak GIT-PERL-DEFINES G\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-GIT-PERL-DEFINES: FORCE\n-\t@FLAGS='$(PERL_DEFINES)'; \\\n-\t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n-\t\techo >&2 \" * new perl-specific parameters\"; \\\n-\t\techo \"$$FLAGS\" >$@; \\\n-\t fi\n-\n-\n .PHONY: gitweb\n gitweb:\n \t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n@@ -1835,6 +1840,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n endif # NO_PERL\n \n ifndef NO_PYTHON\n+$(eval $(call make-var,PYTHON-VARS,Python interpreter location,$(PYTHON_PATH)))\n $(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS\n $(SCRIPT_PYTHON_GEN): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n@@ -2154,34 +2160,15 @@ cscope:\n \t$(RM) cscope*\n \t$(FIND_SOURCE_FILES) | xargs cscope -b\n \n-### Detect prefix changes\n-TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n-\t\t$(localedir_SQ)\n-\n-GIT-PREFIX: FORCE\n-\t@FLAGS='$(TRACK_PREFIX)'; \\\n-\tif test x\"$$FLAGS\" != x\"`cat GIT-PREFIX 2>/dev/null`\" ; then \\\n-\t\techo >&2 \" * new prefix flags\"; \\\n-\t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n-\tfi\n-\n-TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS))\n-\n-GIT-CFLAGS: FORCE\n-\t@FLAGS='$(TRACK_CFLAGS)'; \\\n-\t if test x\"$$FLAGS\" != x\"`cat GIT-CFLAGS 2>/dev/null`\" ; then \\\n-\t\techo >&2 \" * new build flags\"; \\\n-\t\techo \"$$FLAGS\" >GIT-CFLAGS; \\\n- fi\n-\n-TRACK_LDFLAGS = $(subst ','\\'',$(ALL_LDFLAGS))\n-\n-GIT-LDFLAGS: FORCE\n-\t@FLAGS='$(TRACK_LDFLAGS)'; \\\n-\t if test x\"$$FLAGS\" != x\"`cat GIT-LDFLAGS 2>/dev/null`\" ; then \\\n-\t\techo >&2 \" * new link flags\"; \\\n-\t\techo \"$$FLAGS\" >GIT-LDFLAGS; \\\n- fi\n+$(eval $(call make-var,PREFIX,prefix flags,\\\n+\t:$(bindir)\\\n+\t:$(gitexecdir)\\\n+\t:$(template_dir)\\\n+\t:$(prefix)\\\n+\t:$(localedir)\\\n+))\n+$(eval $(call make-var,CFLAGS,build flags,$(ALL_CFLAGS)))\n+$(eval $(call make-var,LDFLAGS,link flags,$(ALL_LDFLAGS)))\n \n # We need to apply sq twice, once to protect from the shell\n # that runs GIT-BUILD-OPTIONS, and then again to protect it\n@@ -2224,18 +2211,6 @@ ifdef GIT_PERF_MAKE_OPTS\n \t@echo GIT_PERF_MAKE_OPTS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_MAKE_OPTS)))'\\' >>$@\n endif\n \n-### Detect Python interpreter path changes\n-ifndef NO_PYTHON\n-TRACK_PYTHON = $(subst ','\\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')\n-\n-GIT-PYTHON-VARS: FORCE\n-\t@VARS='$(TRACK_PYTHON)'; \\\n-\t if test x\"$$VARS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n-\t\techo >&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.5.2.500.g8060133\n"},{"id":"234297","messageId":"20140205175030.GD15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 04/13] Makefile: use tempfile/mv strategy for GIT-*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:50:30Z","receivedAt":"2014-02-05T17:50:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We create GIT-CFLAGS and related files by echoing into a\nshell redirection. It's possible for an error to cause the\nfile to end up empty or truncated. In theory, this could\ncause us to later make an incorrect comparison of the file\ncontents to a Makefile variable, and avoid rebuilding some\ndependencies.\n\nIn practice, this is very unlikely to happen, as the\nfollowup run would have to have changed the variable to\nmatch the truncation exactly. However, it is good hygiene to\nuse our usual tempfile + mv trick to make sure that the file\nis created atomically.\n\nSigned-off-by: Jeff King \n---\n Makefile | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 50bf252..b06d5ec 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1571,7 +1571,8 @@ GIT-$1: FORCE\n \t@VALUE='$$(subst ','\\'',$3)'; \\\n \tif test x\"$$$$VALUE\" != x\"`cat $$@ 2>/dev/null`\"; then \\\n \t\techo >&2 \" * new $2\"; \\\n-\t\techo \"$$$$VALUE\" >$$@; \\\n+\t\techo \"$$$$VALUE\" >$$@+ && \\\n+\t\tmv $$@+ $$@; \\\n \tfi\n endef\n \n-- \n1.8.5.2.500.g8060133\n"},{"id":"234298","messageId":"20140205175054.GE15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 05/13] Makefile: prefer printf to echo for GIT-*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:50:54Z","receivedAt":"2014-02-05T17:50:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we write Makefile variables to a sentinel file, we use\n\"echo\" to do so. Since we are writing arbitrary data which\nmay contain backslash escapes (particularly with file paths\non Windows), echo may or may not expand those escapes,\ndepending on which shell is in use.\n\nDuring the next run of \"make\", we check the sentinel file to\nsee if it is different than the Makefile variable. If\nescapes were expanded, then we will erroneously think it\nchanged and trigger a rebuild. You can see this easily by\nrunning:\n\n make prefix='foo\\bar'\n\nmultiple times; it will re-build some files repeatedly.\n\nSigned-off-by: Jeff King \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex b06d5ec..60dc53b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1571,7 +1571,7 @@ GIT-$1: FORCE\n \t@VALUE='$$(subst ','\\'',$3)'; \\\n \tif test x\"$$$$VALUE\" != x\"`cat $$@ 2>/dev/null`\"; then \\\n \t\techo >&2 \" * new $2\"; \\\n-\t\techo \"$$$$VALUE\" >$$@+ && \\\n+\t\tprintf '%s\\n' \"$$$$VALUE\" >$$@+ && \\\n \t\tmv $$@+ $$@; \\\n \tfi\n endef\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234299","messageId":"20140205175222.GF15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 06/13] Makefile: store GIT-* sentinel files in MAKE/","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:52:22Z","receivedAt":"2014-02-05T17:52:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The Makefile creates files like GIT-CFLAGS to keep track of\nthe make variables used in the last build. We have a number\nof these files now, all starting with GIT-*. Let's move them\ninto their own subdirectory, which has two advantages:\n\n 1. It's less clutter in the main directory.\n\n 2. Each file needs to be marked separately in .gitignore.\n With a subdirectory, we can easily mark them all with a\n wildcard (we cannot do a wildcard for GIT-* because\n of precious files like GIT-VERSION-GEN).\n\nThis patch moves all of the generated GIT-* files into\nMAKE/*, with one exception: GIT-VERSION-FILE. This could be\nmoved along with the rest, but there is a reasonable chance\nthat there are some out-of-tree scripts that may peek at it\n(whereas things like GIT-CFLAGS are purely internal, and we\nupdate all internal references).\n\nSigned-off-by: Jeff King \n---\nI'm not married to the name \"MAKE\", but I do think the separate\ndirectory is a win for simplicity and avoiding repetition (as you can\nsee in the diffstat). Other suggestions welcome.\n\n .gitignore | 8 -------\n MAKE/.gitignore | 1 +\n Makefile | 66 +++++++++++++++++++++++++--------------------------\n perl/Makefile | 10 ++++----\n t/perf/run | 2 +-\n t/test-lib.sh | 2 +-\n t/valgrind/analyze.sh | 4 ++--\n 7 files changed, 42 insertions(+), 51 deletions(-)\n create mode 100644 MAKE/.gitignore\n\ndiff --git a/.gitignore b/.gitignore\nindex b5f9def..586a067 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -1,11 +1,3 @@\n-/GIT-BUILD-OPTIONS\n-/GIT-CFLAGS\n-/GIT-LDFLAGS\n-/GIT-PREFIX\n-/GIT-PERL-DEFINES\n-/GIT-PYTHON-VARS\n-/GIT-SCRIPT-DEFINES\n-/GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\n /git\ndiff --git a/MAKE/.gitignore b/MAKE/.gitignore\nnew file mode 100644\nindex 0000000..72e8ffc\n--- /dev/null\n+++ b/MAKE/.gitignore\n@@ -0,0 +1 @@\n+*\ndiff --git a/Makefile b/Makefile\nindex 60dc53b..7fecdf1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1564,10 +1564,10 @@ endif\n # usage: $(eval $(call make-var,FN,DESC,CONTENTS))\n #\n # Create a rule to write $CONTENTS (which should come from a make variable)\n-# to GIT-$FN, but only if not already there. This can be used to create a\n+# to MAKE/$FN, but only if not already there. This can be used to create a\n # dependency on a Makefile variable. Prints $DESC to the user.\n define make-var\n-GIT-$1: FORCE\n+MAKE/$1: FORCE\n \t@VALUE='$$(subst ','\\'',$3)'; \\\n \tif test x\"$$$$VALUE\" != x\"`cat $$@ 2>/dev/null`\"; then \\\n \t\techo >&2 \" * new $2\"; \\\n@@ -1658,7 +1658,7 @@ all:: profile-clean\n endif\n endif\n \n-all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n+all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) MAKE/BUILD-OPTIONS\n ifneq (,$X)\n \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n endif\n@@ -1714,25 +1714,25 @@ strip: $(PROGRAMS) git$X\n # dependencies here will not need to change if the force-build\n # details change some day.\n \n-git.sp git.s git.o: GIT-PREFIX\n+git.sp git.s git.o: MAKE/PREFIX\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n-git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n+git$X: git.o MAKE/LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n \t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n \n help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h MAKE/PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n-version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT\n+version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n \t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n@@ -1773,12 +1773,12 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n $@.sh >$@+\n endef\n \n-$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES\n+$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh MAKE/SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n+$(SCRIPT_LIB) : % : %.sh MAKE/SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n@@ -1797,14 +1797,14 @@ perl/PM.stamp: FORCE\n \t{ cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \\\n \t$(RM) $@+\n \n-perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n+perl/perl.mak: MAKE/CFLAGS MAKE/PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n $(eval $(call make-var,PERL-DEFINES,perl-specific parameters,\\\n \t:$(PERL_PATH)\\\n \t:$(PERLLIB_EXTRA)\\\n ))\n-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak MAKE/PERL-DEFINES GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n \tINSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \\\n@@ -1826,7 +1826,7 @@ gitweb:\n \t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n all:: gitweb\n \n-git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n+git-instaweb: git-instaweb.sh MAKE/SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n@@ -1842,7 +1842,7 @@ endif # NO_PERL\n \n ifndef NO_PYTHON\n $(eval $(call make-var,PYTHON-VARS,Python interpreter location,$(PYTHON_PATH)))\n-$(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS\n+$(SCRIPT_PYTHON_GEN): MAKE/CFLAGS MAKE/PREFIX MAKE/PYTHON-VARS\n $(SCRIPT_PYTHON_GEN): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n@@ -1984,13 +1984,13 @@ endif\n endif\n \n ifndef CHECK_HEADER_DEPENDENCIES\n-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)\n+$(C_OBJ): %.o: %.c MAKE/CFLAGS $(missing_dep_dirs)\n \t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)\n+$(ASM_OBJ): %.o: %.S MAKE/CFLAGS $(missing_dep_dirs)\n \t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n endif\n \n-%.s: %.c GIT-CFLAGS FORCE\n+%.s: %.c MAKE/CFLAGS FORCE\n \t$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n \n ifdef USE_COMPUTED_HEADER_DEPENDENCIES\n@@ -2011,25 +2011,25 @@ else\n $(OBJECTS): $(LIB_H)\n endif\n \n-exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX\n+exec_cmd.sp exec_cmd.s exec_cmd.o: MAKE/PREFIX\n exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n \t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n \t'-DPREFIX=\"$(prefix_SQ)\"'\n \n-builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX\n+builtin/init-db.sp builtin/init-db.s builtin/init-db.o: MAKE/PREFIX\n builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n \t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n \n-config.sp config.s config.o: GIT-PREFIX\n+config.sp config.s config.o: MAKE/PREFIX\n config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n-attr.sp attr.s attr.o: GIT-PREFIX\n+attr.sp attr.s attr.o: MAKE/PREFIX\n attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n \n-gettext.sp gettext.s gettext.o: GIT-PREFIX\n+gettext.sp gettext.s gettext.o: MAKE/PREFIX\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n \n@@ -2051,21 +2051,21 @@ compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \\\n compat/nedmalloc/nedmalloc.sp: SPARSE_FLAGS += -Wno-non-pointer-null\n endif\n \n-git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n+git-%$X: %.o MAKE/LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o MAKE/LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n \n-git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n+git-http-fetch$X: http.o http-walker.o http-fetch.o MAKE/LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL)\n-git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n+git-http-push$X: http.o http-push.o MAKE/LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n \n-git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)\n+git-remote-testsvn$X: remote-testsvn.o MAKE/LDFLAGS $(GITLIBS) $(VCSSVN_LIB)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \\\n \t$(VCSSVN_LIB)\n \n@@ -2075,7 +2075,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \tln -s $< $@ 2>/dev/null || \\\n \tcp $< $@\n \n-$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)\n+$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o MAKE/LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n \n@@ -2172,9 +2172,9 @@ $(eval $(call make-var,CFLAGS,build flags,$(ALL_CFLAGS)))\n $(eval $(call make-var,LDFLAGS,link flags,$(ALL_LDFLAGS)))\n \n # We need to apply sq twice, once to protect from the shell\n-# that runs GIT-BUILD-OPTIONS, and then again to protect it\n+# that runs MAKE/BUILD-OPTIONS, and then again to protect it\n # and the first level quoting from the shell that runs \"echo\".\n-GIT-BUILD-OPTIONS: FORCE\n+MAKE/BUILD-OPTIONS: FORCE\n \t@echo SHELL_PATH=\\''$(subst ','\\'',$(SHELL_PATH_SQ))'\\' >$@\n \t@echo PERL_PATH=\\''$(subst ','\\'',$(PERL_PATH_SQ))'\\' >>$@\n \t@echo DIFF=\\''$(subst ','\\'',$(subst ','\\'',$(DIFF)))'\\' >>$@\n@@ -2255,7 +2255,7 @@ test-svn-fe$X: vcs-svn/lib.a\n \n .PRECIOUS: $(TEST_OBJS)\n \n-test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\n+test-%$X: test-%.o MAKE/LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n \n check-sha1:: test-sha1$X\n@@ -2263,7 +2263,7 @@ check-sha1:: test-sha1$X\n \n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n-$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+$(SP_OBJ): %.sp: %.c MAKE/CFLAGS FORCE\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n \t\t$(SPARSE_FLAGS) $<\n \n@@ -2477,9 +2477,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C gitk-git clean\n \t$(MAKE) -C git-gui clean\n endif\n-\t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS\n-\t$(RM) GIT-USER-AGENT GIT-PREFIX\n-\t$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS\n+\t$(RM) GIT-VERSION-FILE MAKE/*\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\ndiff --git a/perl/Makefile b/perl/Makefile\nindex 15d96fc..b27420e 100644\n--- a/perl/Makefile\n+++ b/perl/Makefile\n@@ -41,7 +41,7 @@ modules += Git/SVN/Prompt\n modules += Git/SVN/Ra\n modules += Git/SVN/Utils\n \n-$(makfile): ../GIT-CFLAGS Makefile\n+$(makfile): ../MAKE/CFLAGS Makefile\n \techo all: private-Error.pm Git.pm Git/I18N.pm > $@\n \tset -e; \\\n \tfor i in $(modules); \\\n@@ -79,11 +79,11 @@ $(makfile): ../GIT-CFLAGS Makefile\n \techo instlibdir: >> $@\n \techo '\techo $(instdir_SQ)' >> $@\n else\n-$(makfile): Makefile.PL ../GIT-CFLAGS\n+$(makfile): Makefile.PL ../MAKE/CFLAGS\n \t$(PERL_PATH) $< PREFIX='$(prefix_SQ)' INSTALL_BASE='' --localedir='$(localedir_SQ)'\n endif\n \n # this is just added comfort for calling make directly in perl dir\n-# (even though GIT-CFLAGS aren't used yet. If ever)\n-../GIT-CFLAGS:\n-\t$(MAKE) -C .. GIT-CFLAGS\n+# (even though MAKE/CFLAGS aren't used yet. If ever)\n+../MAKE/CFLAGS:\n+\t$(MAKE) -C .. MAKE/CFLAGS\ndiff --git a/t/perf/run b/t/perf/run\nindex cfd7012..489d6cb 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -73,7 +73,7 @@ GIT_PERF_AGGREGATING_LATER=t\n export GIT_PERF_AGGREGATING_LATER\n \n cd \"$(dirname $0)\"\n-. ../../GIT-BUILD-OPTIONS\n+. ../../MAKE/BUILD-OPTIONS\n \n if test $# = 0 -o \"$1\" = -- -o -f \"$1\"; then\n \tset -- . \"$@\"\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1531c24..2c1ce73 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -48,7 +48,7 @@ then\n \texit 1\n fi\n \n-. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+. \"$GIT_BUILD_DIR\"/MAKE/BUILD-OPTIONS\n export PERL_PATH SHELL_PATH\n \n # if --tee was passed, write the output not only to the terminal, but\ndiff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh\nindex 2ffc80f..346e0ff 100755\n--- a/t/valgrind/analyze.sh\n+++ b/t/valgrind/analyze.sh\n@@ -1,7 +1,7 @@\n #!/bin/sh\n \n-# Get TEST_OUTPUT_DIRECTORY from GIT-BUILD-OPTIONS if it's there...\n-. \"$(dirname \"$0\")/../../GIT-BUILD-OPTIONS\"\n+# Get TEST_OUTPUT_DIRECTORY from MAKE/BUILD-OPTIONS if it's there...\n+. \"$(dirname \"$0\")/../../MAKE/BUILD-OPTIONS\"\n # ... otherwise set it to the default value.\n : ${TEST_OUTPUT_DIRECTORY=$(dirname \"$0\")/..}\n \n-- \n1.8.5.2.500.g8060133\n"},{"id":"234300","messageId":"20140205175316.GG15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 07/13] Makefile: always create files via make-var","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:53:16Z","receivedAt":"2014-02-05T17:53:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we are trying to store an empty sentinel value for a\nmake-var, we would consider a missing file the same as an\nempty variable. E.g., repeatedly running:\n\n make GIT_USER_AGENT=\n\nwill not create MAKE/USER-AGENT at all if it does not exist,\nand subsequent make invocations will force a rebuild. This\nis not generally a problem in practice, since most of the\nfiles always have some boilerplate (even LDFLAGS, because it\nis formed with \"+=\", will have a stray space in it). But\nthis does fix the rare case, and future-proofs us as we add\nmore similar variables.\n\nSigned-off-by: Jeff King \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 7fecdf1..e12039f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1569,7 +1569,7 @@ endif\n define make-var\n MAKE/$1: FORCE\n \t@VALUE='$$(subst ','\\'',$3)'; \\\n-\tif test x\"$$$$VALUE\" != x\"`cat $$@ 2>/dev/null`\"; then \\\n+\tif ! test -e $$@ || test x\"$$$$VALUE\" != x\"`cat $$@`\"; then \\\n \t\techo >&2 \" * new $2\"; \\\n \t\tprintf '%s\\n' \"$$$$VALUE\" >$$@+ && \\\n \t\tmv $$@+ $$@; \\\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234302","messageId":"20140205175741.GH15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 08/13] Makefile: introduce sq function for shell-quoting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:57:41Z","receivedAt":"2014-02-05T17:57:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since we have recently abolished the prohibition on $(call)\nin our Makefile, we can use it to factor out the repeated\nshell-quoting we do. There are two upsides:\n\n 1. It is much more readable than inline calls to\n $(subst ','\\'').\n\n 2. It is short enough that we can do away with the _SQ\n variants of many variables (note that we do not do this\n just yet, as there is a little more cleanup needed\n first).\n\nSigned-off-by: Jeff King \n---\nThis is the one I am most on the fence about. I think (2) is nice. And\nfor (1), we do end up more readable in some instances (e.g., the\nhorrible inline subst calls in generating BUILD-OPTIONS go away). But\nmany instances are not really any more readable (e.g., see the first\nhunk below).\n\nTwo things come to mind:\n\n 1. If we really prefer reading $(FOO_SQ) but don't want to manually\n write each one, we could have a simple script that reads the\n Makefile and produces an _SQ variant of every variable, whether we\n need it or not.\n\n 2. The later parts of the series introduce a new way of getting\n values into programs that does not involve the command-line. So in\n theory many of these quoted uses of variables would just go away in\n the long run (if we choose to pursue that direction).\n\n Makefile | 120 ++++++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 64 insertions(+), 56 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e12039f..868872f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -506,11 +506,11 @@ build-python-script: $(SCRIPT_PYTHON_GEN)\n \n .PHONY: install-perl-script install-sh-script install-python-script\n install-sh-script: $(SCRIPT_SH_INS)\n-\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $^ $(call sq,$(DESTDIR)$(gitexec_instdir))\n install-perl-script: $(SCRIPT_PERL_INS)\n-\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $^ $(call sq,$(DESTDIR)$(gitexec_instdir))\n install-python-script: $(SCRIPT_PYTHON_INS)\n-\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $^ $(call sq,$(DESTDIR)$(gitexec_instdir))\n \n .PHONY: clean-perl-script clean-sh-script clean-python-script\n clean-sh-script:\n@@ -1040,7 +1040,7 @@ endif\n \n ifdef SANE_TOOL_PATH\n SANE_TOOL_PATH_SQ = $(subst ','\\'',$(SANE_TOOL_PATH))\n-BROKEN_PATH_FIX = 's|^\\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'\n+BROKEN_PATH_FIX = 's|^\\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(call sqi,$(SANE_TOOL_PATH)|'\n PATH := $(SANE_TOOL_PATH):${PATH}\n else\n BROKEN_PATH_FIX = '/^\\# @@BROKEN_PATH_FIX@@$$/d'\n@@ -1561,6 +1561,13 @@ ifneq (\"$(PROFILE)\",\"\")\n endif\n endif\n \n+# usage: $(call sq,CONTENTS)\n+#\n+# Quote the value as appropriate for the shell. Use \"sqi\" if you are\n+# already \"i\"nside single-quotes.\n+sqi = $(subst ','\\'',$1)\n+sq = '$(call sqi,$1)'\n+\n # usage: $(eval $(call make-var,FN,DESC,CONTENTS))\n #\n # Create a rule to write $CONTENTS (which should come from a make variable)\n@@ -1568,7 +1575,7 @@ endif\n # dependency on a Makefile variable. Prints $DESC to the user.\n define make-var\n MAKE/$1: FORCE\n-\t@VALUE='$$(subst ','\\'',$3)'; \\\n+\t@VALUE=$$(call sq,$3); \\\n \tif ! test -e $$@ || test x\"$$$$VALUE\" != x\"`cat $$@`\"; then \\\n \t\techo >&2 \" * new $2\"; \\\n \t\tprintf '%s\\n' \"$$$$VALUE\" >$$@+ && \\\n@@ -1603,7 +1610,7 @@ PERLLIB_EXTRA_SQ = $(subst ','\\'',$(PERLLIB_EXTRA))\n \n LIBS = $(GITLIBS) $(EXTLIBS)\n \n-BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \\\n+BASIC_CFLAGS += -DSHA1_HEADER=$(call sq,$(SHA1_HEADER)) \\\n \t$(COMPAT_CFLAGS)\n LIB_OBJS += $(COMPAT_OBJS)\n \n@@ -1665,13 +1672,13 @@ endif\n \n all::\n ifndef NO_TCLTK\n-\t$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all\n+\t$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir=$(call sq,$(gitexec_instdir)) all\n \t$(QUIET_SUBDIR0)gitk-git $(QUIET_SUBDIR1) all\n endif\n ifndef NO_PERL\n-\t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' localedir='$(localedir_SQ)' all\n+\t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH=$(call sq,$(PERL_PATH)) prefix=$(call sq,$(prefix)) localedir=$(call sq,$(localedir)) all\n endif\n-\t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'\n+\t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) SHELL_PATH=$(call sq,$(SHELL_PATH)) PERL_PATH=$(call sq,$(PERL_PATH))\n \n please_set_SHELL_PATH_to_a_more_modern_shell:\n \t@$$(:)\n@@ -1761,15 +1768,15 @@ $(eval $(call make-var,SCRIPT-DEFINES,script parameters,\\\n ))\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n-sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n- -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n- -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n- -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n+sed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n+ -e 's|@SHELL_PATH@|$(call sqi,$(SHELL_PATH))|' \\\n+ -e 's|@@DIFF@@|$(call sqi,$(DIFF))|' \\\n+ -e 's|@@LOCALEDIR@@|$(call sqi,$(localedir))|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n -e $(BROKEN_PATH_FIX) \\\n- -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n- -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n+ -e 's|@@GITWEBDIR@@|$(call sqi,$(gitwebdir))|g' \\\n+ -e 's|@@PERL@@|$(call sqi,$(PERL_PATH))|g' \\\n $@.sh >$@+\n endef\n \n@@ -1798,7 +1805,7 @@ perl/PM.stamp: FORCE\n \t$(RM) $@+\n \n perl/perl.mak: MAKE/CFLAGS MAKE/PREFIX perl/Makefile perl/Makefile.PL\n-\t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n+\t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH=$(call sq,$(PERL_PATH)) prefix=$(call sq,$(prefix)) $(@F)\n \n $(eval $(call make-var,PERL-DEFINES,perl-specific parameters,\\\n \t:$(PERL_PATH)\\\n@@ -1807,10 +1814,10 @@ $(eval $(call make-var,PERL-DEFINES,perl-specific parameters,\\\n $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak MAKE/PERL-DEFINES GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n-\tINSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \\\n+\tINSTLIBDIR_EXTRA=$(call sq,$(PERLLIB_EXTRA)) && \\\n \tINSTLIBDIR=\"$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}\" && \\\n \tsed -e '1{' \\\n-\t -e '\ts|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n+\t -e '\ts|#!.*perl|#!$(call sqi,$(PERL_PATH))|' \\\n \t -e '\th' \\\n \t -e '\ts=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || \"'\"$$INSTLIBDIR\"'\"));=' \\\n \t -e '\tH' \\\n@@ -1833,7 +1840,7 @@ git-instaweb: git-instaweb.sh MAKE/SCRIPT-DEFINES\n else # NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\tsed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n \t -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \\\n \t unimplemented.sh >$@+ && \\\n \tchmod +x $@+ && \\\n@@ -1845,14 +1852,14 @@ $(eval $(call make-var,PYTHON-VARS,Python interpreter location,$(PYTHON_PATH)))\n $(SCRIPT_PYTHON_GEN): MAKE/CFLAGS MAKE/PREFIX MAKE/PYTHON-VARS\n $(SCRIPT_PYTHON_GEN): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n+\tsed -e '1s|#!.*python|#!$(call sqi,$(PYTHON_PATH))|' \\\n \t $< >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n else # NO_PYTHON\n $(SCRIPT_PYTHON_GEN): % : unimplemented.sh\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\tsed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n \t -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \\\n \t unimplemented.sh >$@+ && \\\n \tchmod +x $@+ && \\\n@@ -2174,42 +2181,43 @@ $(eval $(call make-var,LDFLAGS,link flags,$(ALL_LDFLAGS)))\n # We need to apply sq twice, once to protect from the shell\n # that runs MAKE/BUILD-OPTIONS, and then again to protect it\n # and the first level quoting from the shell that runs \"echo\".\n+ssq = $(call sq,$(call sq,$1))\n MAKE/BUILD-OPTIONS: FORCE\n-\t@echo SHELL_PATH=\\''$(subst ','\\'',$(SHELL_PATH_SQ))'\\' >$@\n-\t@echo PERL_PATH=\\''$(subst ','\\'',$(PERL_PATH_SQ))'\\' >>$@\n-\t@echo DIFF=\\''$(subst ','\\'',$(subst ','\\'',$(DIFF)))'\\' >>$@\n-\t@echo PYTHON_PATH=\\''$(subst ','\\'',$(PYTHON_PATH_SQ))'\\' >>$@\n-\t@echo TAR=\\''$(subst ','\\'',$(subst ','\\'',$(TAR)))'\\' >>$@\n-\t@echo NO_CURL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_CURL)))'\\' >>$@\n-\t@echo USE_LIBPCRE=\\''$(subst ','\\'',$(subst ','\\'',$(USE_LIBPCRE)))'\\' >>$@\n-\t@echo NO_PERL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PERL)))'\\' >>$@\n-\t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@\n-\t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@\n+\t@echo SHELL_PATH=$(call ssq,$(SHELL_PATH)) >$@\n+\t@echo PERL_PATH=$(call ssq,$(PERL_PATH)) >>$@\n+\t@echo DIFF=$(call ssq,$(DIFF)) >>$@\n+\t@echo PYTHON_PATH=$(call ssq,$(PYTHON_PATH)) >>$@\n+\t@echo TAR=$(call ssq,$(TAR)) >>$@\n+\t@echo NO_CURL=$(call ssq,$(NO_CURL)) >>$@\n+\t@echo USE_LIBPCRE=$(call ssq,$(USE_LIBPCRE)) >>$@\n+\t@echo NO_PERL=$(call ssq,$(NO_PERL)) >>$@\n+\t@echo NO_PYTHON=$(call ssq,$(NO_PYTHON)) >>$@\n+\t@echo NO_UNIX_SOCKETS=$(call ssq,$(NO_UNIX_SOCKETS)) >>$@\n ifdef TEST_OUTPUT_DIRECTORY\n-\t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@\n+\t@echo TEST_OUTPUT_DIRECTORY=$(call ssq,$(TEST_OUTPUT_DIRECTORY)) >>$@\n endif\n ifdef GIT_TEST_OPTS\n-\t@echo GIT_TEST_OPTS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_OPTS)))'\\' >>$@\n+\t@echo GIT_TEST_OPTS=$(call ssq,$(GIT_TEST_OPTS)) >>$@\n endif\n ifdef GIT_TEST_CMP\n-\t@echo GIT_TEST_CMP=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_CMP)))'\\' >>$@\n+\t@echo GIT_TEST_CMP=$(call ssq,$(GIT_TEST_CMP)) >>$@\n endif\n ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT\n \t@echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease >>$@\n endif\n-\t@echo NO_GETTEXT=\\''$(subst ','\\'',$(subst ','\\'',$(NO_GETTEXT)))'\\' >>$@\n-\t@echo GETTEXT_POISON=\\''$(subst ','\\'',$(subst ','\\'',$(GETTEXT_POISON)))'\\' >>$@\n+\t@echo NO_GETTEXT=$(call ssq,$(NO_GETTEXT)) >>$@\n+\t@echo GETTEXT_POISON=$(call ssq,$(GETTEXT_POISON)) >>$@\n ifdef GIT_PERF_REPEAT_COUNT\n-\t@echo GIT_PERF_REPEAT_COUNT=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_REPEAT_COUNT)))'\\' >>$@\n+\t@echo GIT_PERF_REPEAT_COUNT=$(call ssq,$(GIT_PERF_REPEAT_COUNT)) >>$@\n endif\n ifdef GIT_PERF_REPO\n-\t@echo GIT_PERF_REPO=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_REPO)))'\\' >>$@\n+\t@echo GIT_PERF_REPO=$(call ssq,$(GIT_PERF_REPO)) >>$@\n endif\n ifdef GIT_PERF_LARGE_REPO\n-\t@echo GIT_PERF_LARGE_REPO=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_LARGE_REPO)))'\\' >>$@\n+\t@echo GIT_PERF_LARGE_REPO=$(call ssq,$(GIT_PERF_LARGE_REPO)) >>$@\n endif\n ifdef GIT_PERF_MAKE_OPTS\n-\t@echo GIT_PERF_MAKE_OPTS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_PERF_MAKE_OPTS)))'\\' >>$@\n+\t@echo GIT_PERF_MAKE_OPTS=$(call ssq,$(GIT_PERF_MAKE_OPTS)) >>$@\n endif\n \n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n@@ -2219,7 +2227,7 @@ all:: $(NO_INSTALL)\n \n bin-wrappers/%: wrap-for-bin.sh\n \t@mkdir -p bin-wrappers\n-\t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n \t -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n \t -e 's|@@PROG@@|$(@F)|' < $< > $@ && \\\n \tchmod +x $@\n@@ -2307,33 +2315,33 @@ mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n \n install: all\n-\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n-\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n-\t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n-\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n-\t$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n+\t$(INSTALL) -d -m 755 $(call sq,$(DESTDIR)$(bindir))\n+\t$(INSTALL) -d -m 755 $(call sq,$(DESTDIR)$(gitexec_instdir))\n+\t$(INSTALL) $(ALL_PROGRAMS) $(call sq,$(DESTDIR)$(gitexec_instdir))\n+\t$(INSTALL) -m 644 $(SCRIPT_LIB) $(call sq,$(DESTDIR)$(gitexec_instdir))\n+\t$(INSTALL) $(install_bindir_programs) $(call sq,$(DESTDIR)$(bindir))\n+\t$(MAKE) -C templates DESTDIR=$(call sq,$(DESTDIR)) install\n+\t$(INSTALL) -d -m 755 $(call sq,$(DESTDIR)$(mergetools_instdir))\n+\t$(INSTALL) -m 644 mergetools/* $(call sq,$(DESTDIR)$(mergetools_instdir))\n ifndef NO_GETTEXT\n-\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n+\t$(INSTALL) -d -m 755 $(call sq,$(DESTDIR)$(localedir))\n \t(cd po/build/locale && $(TAR) cf - .) | \\\n-\t(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)\n+\t(cd $(call sq,$(DESTDIR)$(localedir)) && umask 022 && $(TAR) xof -)\n endif\n ifndef NO_PERL\n-\t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install\n+\t$(MAKE) -C perl prefix=$(call sq,$(prefix)) DESTDIR=$(call sq,$(DESTDIR)) install\n \t$(MAKE) -C gitweb install\n endif\n ifndef NO_TCLTK\n \t$(MAKE) -C gitk-git install\n-\t$(MAKE) -C git-gui gitexecdir='$(gitexec_instdir_SQ)' install\n+\t$(MAKE) -C git-gui gitexecdir=$(call sq,$(gitexec_instdir)) install\n endif\n ifneq (,$X)\n-\t$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' -ef '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p$X' || $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)\n+\t$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test $(call sq,$(DESTDIR)$(gitexec_instdir)/$p) -ef $(call sq,$(DESTDIR)$(gitexec_instdir)/$p$X) || $(RM) $(call sq,$(DESTDIR)$(gitexec_instdir)/$p);)\n endif\n \n-\tbindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \\\n-\texecdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \\\n+\tbindir=$$(cd $(call sq,$(DESTDIR)$(bindir)) && pwd) && \\\n+\texecdir=$$(cd $(call sq,$(DESTDIR)$(gitexec_instdir)) && pwd) && \\\n \t{ test \"$$bindir/\" = \"$$execdir/\" || \\\n \t for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234303","messageId":"20140205175809.GI15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 09/13] Makefile: add c-quote helper function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:58:09Z","receivedAt":"2014-02-05T17:58:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We have to c-quote strings coming from Makefile variables\nwhen we pass them to the compiler via \"-D\". Now that we can\nuse $(call) in our Makefile, we can factor out the quoting\nto make things easier to read. We can also apply it more\nconsistently, as there were many spots that should have been\nC-quoting but did not. For example:\n\n make prefix='foo\\bar'\n\nwould produce an exec_cmd.o with a broken prefix.\n\nSigned-off-by: Jeff King \n---\n Makefile | 58 +++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 29 insertions(+), 29 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 868872f..b1c3fb4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1568,6 +1568,17 @@ endif\n sqi = $(subst ','\\'',$1)\n sq = '$(call sqi,$1)'\n \n+# usage: $(call cq,CONTENTS)\n+#\n+# Quote the value as appropriate for a C string literal.\n+cq = \"$(subst \",\\\",$(subst \\,\\\\,$1))\"\n+\n+# usage: $(call scq,CONTENTS)\n+#\n+# Quote the value as C string inside a shell string. Good for passing strings\n+# on the command line via \"-DFOO=$(call # scq,$(FOO))\".\n+scq = $(call sq,$(call cq,$1))\n+\n # usage: $(eval $(call make-var,FN,DESC,CONTENTS))\n #\n # Create a rule to write $CONTENTS (which should come from a make variable)\n@@ -1617,28 +1628,17 @@ LIB_OBJS += $(COMPAT_OBJS)\n # Quote for C\n \n ifdef DEFAULT_EDITOR\n-DEFAULT_EDITOR_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(DEFAULT_EDITOR)))\"\n-DEFAULT_EDITOR_CQ_SQ = $(subst ','\\'',$(DEFAULT_EDITOR_CQ))\n-\n-BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'\n+BASIC_CFLAGS += -DDEFAULT_EDITOR=$(call scq,$(DEFAULT_EDITOR))\n endif\n \n ifdef DEFAULT_PAGER\n-DEFAULT_PAGER_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(DEFAULT_PAGER)))\"\n-DEFAULT_PAGER_CQ_SQ = $(subst ','\\'',$(DEFAULT_PAGER_CQ))\n-\n-BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'\n+BASIC_CFLAGS += -DDEFAULT_PAGER=$(call scq,$(DEFAULT_PAGER))\n endif\n \n ifdef SHELL_PATH\n-SHELL_PATH_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(SHELL_PATH)))\"\n-SHELL_PATH_CQ_SQ = $(subst ','\\'',$(SHELL_PATH_CQ))\n-\n-BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)'\n+BASIC_CFLAGS += -DSHELL_PATH=$(call scq,$(SHELL_PATH))\n endif\n \n-GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n-GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n $(eval $(call make-var,USER-AGENT,user agent string,$(GIT_USER_AGENT)))\n \n ifdef DEFAULT_HELP_FORMAT\n@@ -1723,9 +1723,9 @@ strip: $(PROGRAMS) git$X\n \n git.sp git.s git.o: MAKE/PREFIX\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n-\t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n-\t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n+\t-DGIT_HTML_PATH=$(call scq,$(htmldir_relative)) \\\n+\t-DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \\\n+\t-DGIT_INFO_PATH=$(call scq,$(infodir_relative))\n \n git$X: git.o MAKE/LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n@@ -1735,14 +1735,14 @@ help.sp help.s help.o: common-cmds.h\n \n builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h MAKE/PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n-\t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n-\t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n+\t-DGIT_HTML_PATH=$(call scq,$(htmldir_relative)) \\\n+\t-DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \\\n+\t-DGIT_INFO_PATH=$(call scq,$(infodir_relative))\n \n version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n-\t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n+\t-DGIT_VERSION=$(call scq,$(GIT_VERSION)) \\\n+\t-DGIT_USER_AGENT=$(call scq,$(GIT_USER_AGENT))\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n@@ -2020,25 +2020,25 @@ endif\n \n exec_cmd.sp exec_cmd.s exec_cmd.o: MAKE/PREFIX\n exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n-\t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n-\t'-DPREFIX=\"$(prefix_SQ)\"'\n+\t-DGIT_EXEC_PATH=$(call scq,$(gitexecdir)) \\\n+\t-DBINDIR=$(call scq,$(bindir_relative)) \\\n+\t-DPREFIX=$(call scq,$(prefix))\n \n builtin/init-db.sp builtin/init-db.s builtin/init-db.o: MAKE/PREFIX\n builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n-\t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n+\t-DDEFAULT_GIT_TEMPLATE_DIR=$(call scq,$(template_dir))\n \n config.sp config.s config.o: MAKE/PREFIX\n config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n-\t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n+\t-DETC_GITCONFIG=$(call scq,$(ETC_GITCONFIG))\n \n attr.sp attr.s attr.o: MAKE/PREFIX\n attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n-\t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n+\t-DETC_GITATTRIBUTES=$(call scq,$(ETC_GITATTRIBUTES))\n \n gettext.sp gettext.s gettext.o: MAKE/PREFIX\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n-\t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n+\t-DGIT_LOCALE_PATH=$(call scq,$(localedir))\n \n http-push.sp http.sp http-walker.sp remote-curl.sp: SPARSE_FLAGS += \\\n \t-DCURL_DISABLE_TYPECHECK\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234304","messageId":"20140205175859.GJ15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 10/13] Makefile: drop *_SQ variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T17:58:59Z","receivedAt":"2014-02-05T17:58:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that all uses of the SQ variables have been dropped, we\ncan stop setting the variables.\n\nSigned-off-by: Jeff King \n---\n Makefile | 28 ----------------------------\n 1 file changed, 28 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b1c3fb4..cd07a70 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1039,7 +1039,6 @@ endif\n endif\n \n ifdef SANE_TOOL_PATH\n-SANE_TOOL_PATH_SQ = $(subst ','\\'',$(SANE_TOOL_PATH))\n BROKEN_PATH_FIX = 's|^\\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(call sqi,$(SANE_TOOL_PATH)|'\n PATH := $(SANE_TOOL_PATH):${PATH}\n else\n@@ -1594,31 +1593,6 @@ MAKE/$1: FORCE\n \tfi\n endef\n \n-# Shell quote (do not use $(call) to accommodate ancient setups);\n-\n-SHA1_HEADER_SQ = $(subst ','\\'',$(SHA1_HEADER))\n-ETC_GITCONFIG_SQ = $(subst ','\\'',$(ETC_GITCONFIG))\n-ETC_GITATTRIBUTES_SQ = $(subst ','\\'',$(ETC_GITATTRIBUTES))\n-\n-DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n-bindir_SQ = $(subst ','\\'',$(bindir))\n-bindir_relative_SQ = $(subst ','\\'',$(bindir_relative))\n-mandir_relative_SQ = $(subst ','\\'',$(mandir_relative))\n-infodir_relative_SQ = $(subst ','\\'',$(infodir_relative))\n-localedir_SQ = $(subst ','\\'',$(localedir))\n-gitexecdir_SQ = $(subst ','\\'',$(gitexecdir))\n-template_dir_SQ = $(subst ','\\'',$(template_dir))\n-htmldir_relative_SQ = $(subst ','\\'',$(htmldir_relative))\n-prefix_SQ = $(subst ','\\'',$(prefix))\n-gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))\n-\n-SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n-PYTHON_PATH_SQ = $(subst ','\\'',$(PYTHON_PATH))\n-TCLTK_PATH_SQ = $(subst ','\\'',$(TCLTK_PATH))\n-DIFF_SQ = $(subst ','\\'',$(DIFF))\n-PERLLIB_EXTRA_SQ = $(subst ','\\'',$(PERLLIB_EXTRA))\n-\n LIBS = $(GITLIBS) $(EXTLIBS)\n \n BASIC_CFLAGS += -DSHA1_HEADER=$(call sq,$(SHA1_HEADER)) \\\n@@ -2302,7 +2276,6 @@ gitexec_instdir = $(gitexecdir)\n else\n gitexec_instdir = $(prefix)/$(gitexecdir)\n endif\n-gitexec_instdir_SQ = $(subst ','\\'',$(gitexec_instdir))\n export gitexec_instdir\n \n ifneq ($(filter /%,$(firstword $(mergetoolsdir))),)\n@@ -2310,7 +2283,6 @@ mergetools_instdir = $(mergetoolsdir)\n else\n mergetools_instdir = $(prefix)/$(mergetoolsdir)\n endif\n-mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n \n install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n \n-- \n1.8.5.2.500.g8060133\n"},{"id":"234305","messageId":"20140205180206.GK15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 11/13] Makefile: auto-build C strings from make variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T18:02:06Z","receivedAt":"2014-02-05T18:02:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already insert the values of some make variables into\nfiles in MAKE/*. We can therefore build a simple pattern\nrule for converting such a value into a C string in a header\nfile, which can then be included and used as normal.\n\nThe new system is demonstrated on version.c, where it can\nreplace the use of \"-D\" on the command-line. Note that we\nstill have to manually specify a dependency in the\nMakefile. In an ideal world, we would auto-detect the header\ndependency; however, there are two holdups:\n\n 1. We cannot rely on having automatic header dependency\n generation on all platforms.\n\n 2. Even when we do generate the dependencies, we rely on\n being able to compile the file once, which means our\n system cannot handle generated headers without a manual\n dependency to bootstrap it.\n\nSigned-off-by: Jeff King \n---\nThis is a technique that I have used in other projects with great\nsuccess, as the make variable dependencies are represented as file\ndependencies (which is the only type of dependency make knows about).\n\n_But_ it ends up a lot less nice here because of the way we do\nauto-dependencies. I'm totally open to revamping the way we handle our\nheader dependencies, but I didn't do that in this series.\n\n Makefile | 11 +++++++----\n script/mkcstring | 18 ++++++++++++++++++\n version.c | 6 ++++--\n 3 files changed, 29 insertions(+), 6 deletions(-)\n create mode 100644 script/mkcstring\n\ndiff --git a/Makefile b/Makefile\nindex cd07a70..203171d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1593,6 +1593,11 @@ MAKE/$1: FORCE\n \tfi\n endef\n \n+MAKE/%-string.h: MAKE/% script/mkcstring\n+\t$(QUIET_GEN)$(SHELL_PATH) script/mkcstring \\\n+\t\t$(subst -,_,$*) <$< >$@+ && \\\n+\t\tmv $@+ $@\n+\n LIBS = $(GITLIBS) $(EXTLIBS)\n \n BASIC_CFLAGS += -DSHA1_HEADER=$(call sq,$(SHA1_HEADER)) \\\n@@ -1614,6 +1619,7 @@ BASIC_CFLAGS += -DSHELL_PATH=$(call scq,$(SHELL_PATH))\n endif\n \n $(eval $(call make-var,USER-AGENT,user agent string,$(GIT_USER_AGENT)))\n+$(eval $(call make-var,VERSION,version,$(GIT_VERSION)))\n \n ifdef DEFAULT_HELP_FORMAT\n BASIC_CFLAGS += -DDEFAULT_HELP_FORMAT='\"$(DEFAULT_HELP_FORMAT)\"'\n@@ -1713,10 +1719,7 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \\\n \t-DGIT_INFO_PATH=$(call scq,$(infodir_relative))\n \n-version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT\n-version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n-\t-DGIT_VERSION=$(call scq,$(GIT_VERSION)) \\\n-\t-DGIT_USER_AGENT=$(call scq,$(GIT_USER_AGENT))\n+version.sp version.s version.o: MAKE/VERSION-string.h MAKE/USER-AGENT-string.h\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\ndiff --git a/script/mkcstring b/script/mkcstring\nnew file mode 100644\nindex 0000000..c01f430\n--- /dev/null\n+++ b/script/mkcstring\n@@ -0,0 +1,18 @@\n+#!/bin/sh\n+\n+name=$1; shift\n+\n+c_quote() {\n+\tsed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/'\n+}\n+\n+cat <<-EOF\n+#ifndef MAKE_${name}_H\n+#define MAKE_${name}_H\n+\n+/* Auto-generated by mkcstring */\n+\n+#define MAKE_${name} \"$(c_quote)\"\n+\n+#endif /* MAKE_${name}_H */\n+EOF\ndiff --git a/version.c b/version.c\nindex 6106a80..f68a93b 100644\n--- a/version.c\n+++ b/version.c\n@@ -1,8 +1,10 @@\n #include \"git-compat-util.h\"\n #include \"version.h\"\n #include \"strbuf.h\"\n+#include \"MAKE/USER-AGENT-string.h\"\n+#include \"MAKE/VERSION-string.h\"\n \n-const char git_version_string[] = GIT_VERSION;\n+const char git_version_string[] = MAKE_VERSION;\n \n const char *git_user_agent(void)\n {\n@@ -11,7 +13,7 @@ const char *git_user_agent(void)\n \tif (!agent) {\n \t\tagent = getenv(\"GIT_USER_AGENT\");\n \t\tif (!agent)\n-\t\t\tagent = GIT_USER_AGENT;\n+\t\t\tagent = MAKE_USER_AGENT;\n \t}\n \n \treturn agent;\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234306","messageId":"20140205180547.GL15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 12/13] Makefile: teach scripts to include make variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T18:05:47Z","receivedAt":"2014-02-05T18:05:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current scheme for getting build-time variables into a\nshell script is to munge the script with sed, and stick the\nmunged variable into a special sentinel file so that \"make\"\nknows about the dependency.\n\nInstead, we can combine both functions by generating a shell\nsnippet with our value, and then \"building\" shell scripts by\nconcatenating their snippets. \"make\" then handles the\ndependency automatically, and it's easy to generate tighter\ndependencies.\n\nWe demonstrate here by moving the \"DIFF\" substitution into\nits own snippet, which lets us rebuild only the single\naffected file when it changes.\n\nSigned-off-by: Jeff King \n---\nThe astute reader will notice that:\n\n MAKE_DIFF='diff'\n $MAKE_DIFF ...\n\nthat we end up with is not _quite_ the same as replacing \"$MAKE_DIFF\" in\nthe actual script text with \"diff\" at build-time. In particular, the way\nthat whitespace and quotes are treated is different. I'm not sure what\nwe would want to do here. Calling \"eval\" would work. Or we could have\nthe snippet produce a function, rather than a variable, like:\n\n DIFF() {\n diff \"$@\"\n }\n\n Makefile | 18 +++++++++++++++---\n git-sh-setup.sh | 2 +-\n script/mksh | 4 ++++\n 3 files changed, 20 insertions(+), 4 deletions(-)\n create mode 100644 script/mksh\n\ndiff --git a/Makefile b/Makefile\nindex 203171d..ad3100d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1598,6 +1598,11 @@ MAKE/%-string.h: MAKE/% script/mkcstring\n \t\t$(subst -,_,$*) <$< >$@+ && \\\n \t\tmv $@+ $@\n \n+MAKE/%.sh: MAKE/% script/mksh\n+\t$(QUIET_GEN)$(SHELL_PATH) script/mksh \\\n+\t\t$(subst -,_,$*) <$< >$@+ && \\\n+\t\tmv $@+ $@\n+\n LIBS = $(GITLIBS) $(EXTLIBS)\n \n BASIC_CFLAGS += -DSHA1_HEADER=$(call sq,$(SHA1_HEADER)) \\\n@@ -1734,7 +1739,6 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)\n \n $(eval $(call make-var,SCRIPT-DEFINES,script parameters,\\\n \t:$(SHELL_PATH)\\\n-\t:$(DIFF)\\\n \t:$(GIT_VERSION)\\\n \t:$(localedir)\\\n \t:$(NO_CURL)\\\n@@ -1743,18 +1747,24 @@ $(eval $(call make-var,SCRIPT-DEFINES,script parameters,\\\n \t:$(gitwebdir)\\\n \t:$(PERL_PATH)\\\n ))\n+$(eval $(call make-var,DIFF,diff command,$(DIFF)))\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n+{ \\\n+includes=\"$(filter MAKE/%.sh,$^)\"; \\\n+if ! test -z \"$$includes\"; then \\\n+\tcat $$includes; \\\n+fi && \\\n sed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n -e 's|@SHELL_PATH@|$(call sqi,$(SHELL_PATH))|' \\\n- -e 's|@@DIFF@@|$(call sqi,$(DIFF))|' \\\n -e 's|@@LOCALEDIR@@|$(call sqi,$(localedir))|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n -e $(BROKEN_PATH_FIX) \\\n -e 's|@@GITWEBDIR@@|$(call sqi,$(gitwebdir))|g' \\\n -e 's|@@PERL@@|$(call sqi,$(PERL_PATH))|g' \\\n- $@.sh >$@+\n+ $@.sh; \\\n+} >$@+\n endef\n \n $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh MAKE/SCRIPT-DEFINES\n@@ -1766,6 +1776,8 @@ $(SCRIPT_LIB) : % : %.sh MAKE/SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n+git-sh-setup: MAKE/DIFF.sh\n+\n git.res: git.rc GIT-VERSION-FILE\n \t$(QUIET_RC)$(RC) \\\n \t $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., ,$(GIT_VERSION))))) \\\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex fffa3c7..627d289 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -285,7 +285,7 @@ clear_local_git_env() {\n # remove lines from $1 that are not in $2, leaving only common lines.\n create_virtual_base() {\n \tsz0=$(wc -c <\"$1\")\n-\t@@DIFF@@ -u -La/\"$1\" -Lb/\"$1\" \"$1\" \"$2\" | git apply --no-add\n+\t$MAKE_DIFF -u -La/\"$1\" -Lb/\"$1\" \"$1\" \"$2\" | git apply --no-add\n \tsz1=$(wc -c <\"$1\")\n \n \t# If we do not have enough common material, it is not\ndiff --git a/script/mksh b/script/mksh\nnew file mode 100644\nindex 0000000..d41e77a\n--- /dev/null\n+++ b/script/mksh\n@@ -0,0 +1,4 @@\n+#!/bin/sh\n+\n+name=$1; shift\n+printf \"MAKE_%s='%s'\\n\" \"$name\" \"$(sed \"s/'/'\\\\''/g\")\"\n-- \n1.8.5.2.500.g8060133\n"},{"id":"234307","messageId":"20140205180857.GM15218@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205174823.GA15070@sigill.intra.peff.net","subject":"[PATCH 13/13] move LESS/LV pager environment to Makefile","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T18:08:57Z","receivedAt":"2014-02-05T18:08:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We set the LESS and LV variables to sensible defaults if\nthey are not already set. However, the code is brittle.\nThere is no easy way to change the defaults at compile time,\nand we have to duplicate the code in git-sh-setup and in\npager.c.\n\nLet's turn it into a normal Makefile knob instead.\n\nSigned-off-by: Jeff King \n---\nBits of this patch were liberally stolen from Junio's weather-balloon\npatch. All bugs are mine. :)\n\nNote that setup_pager_env in the C code still does do a little parsing,\nsince we need to have the name of each variable in a separate buffer to\npass to getenv(). I chose to do it this way so that we could introduce\na standard \"mkcarray\" helper that could be used in other places. But it\nwould also be easy to do all of that parsing at compile time and produce\nan array of structs like:\n\n { \"LESS\", \"LESS=-FRSX\" },\n { \"LV\", \"LV=-c\" }\n\n Makefile | 22 +++++++++++++++++++++-\n git-sh-setup.sh | 9 +++++----\n pager.c | 34 +++++++++++++++++++++++-----------\n script/mkcarray | 25 +++++++++++++++++++++++++\n 4 files changed, 74 insertions(+), 16 deletions(-)\n create mode 100644 script/mkcarray\n\ndiff --git a/Makefile b/Makefile\nindex ad3100d..fff8e72 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -342,6 +342,14 @@ all::\n # Define DEFAULT_HELP_FORMAT to \"man\", \"info\" or \"html\"\n # (defaults to \"man\") if you want to have a different default when\n # \"git help\" is called without a parameter specifying the format.\n+#\n+# Define PAGER_ENV to a SP separated VAR=VAL pairs to define\n+# default environment variables to be passed when a pager is spawned, e.g.\n+#\n+# PAGER_ENV = LESS=-FRSX LV=-c\n+#\n+# to say \"export LESS=-FRSX (and LV=-c) if the environment variable\n+# LESS (and LV) is not set, respectively\".\n \n GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -1505,6 +1513,10 @@ ifeq ($(PYTHON_PATH),)\n NO_PYTHON = NoThanks\n endif\n \n+ifndef PAGER_ENV\n+PAGER_ENV = LESS=-FRSX LV=-c\n+endif\n+\n QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n QUIET_SUBDIR1 =\n \n@@ -1598,6 +1610,11 @@ MAKE/%-string.h: MAKE/% script/mkcstring\n \t\t$(subst -,_,$*) <$< >$@+ && \\\n \t\tmv $@+ $@\n \n+MAKE/%-array.h: MAKE/% script/mkcarray\n+\t$(QUIET_GEN)$(SHELL_PATH) script/mkcarray \\\n+\t\t$(subst -,_,$*) <$< >$@+ && \\\n+\t\tmv $@+ $@\n+\n MAKE/%.sh: MAKE/% script/mksh\n \t$(QUIET_GEN)$(SHELL_PATH) script/mksh \\\n \t\t$(subst -,_,$*) <$< >$@+ && \\\n@@ -1726,6 +1743,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \n version.sp version.s version.o: MAKE/VERSION-string.h MAKE/USER-AGENT-string.h\n \n+$(eval $(call make-var,PAGER-ENV,pager environment,$(PAGER_ENV)))\n+pager.sp pager.s pager.o: MAKE/PAGER-ENV-array.h\n+\n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n \tln $< $@ 2>/dev/null || \\\n@@ -1776,7 +1796,7 @@ $(SCRIPT_LIB) : % : %.sh MAKE/SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n-git-sh-setup: MAKE/DIFF.sh\n+git-sh-setup: MAKE/DIFF.sh MAKE/PAGER-ENV.sh\n \n git.res: git.rc GIT-VERSION-FILE\n \t$(QUIET_RC)$(RC) \\\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 627d289..be4a58a 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -158,10 +158,11 @@ git_pager() {\n \telse\n \t\tGIT_PAGER=cat\n \tfi\n-\t: ${LESS=-FRSX}\n-\t: ${LV=-c}\n-\texport LESS LV\n-\n+\tfor vardef in $MAKE_PAGER_ENV\n+\tdo\n+\t\tvar=${vardef%%=*}\n+\t\teval \": \\${$vardef} && export $var\"\n+\tdone\n \teval \"$GIT_PAGER\" '\"$@\"'\n }\n \ndiff --git a/pager.c b/pager.c\nindex 0cc75a8..6db84c6 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -1,6 +1,8 @@\n #include \"cache.h\"\n #include \"run-command.h\"\n #include \"sigchain.h\"\n+#include \"argv-array.h\"\n+#include \"MAKE/PAGER-ENV-array.h\"\n \n #ifndef DEFAULT_PAGER\n #define DEFAULT_PAGER \"less\"\n@@ -60,9 +62,26 @@ const char *git_pager(int stdout_is_tty)\n \treturn pager;\n }\n \n+static void setup_pager_env(struct argv_array *env)\n+{\n+\tint i;\n+\n+\tfor (i = 0; MAKE_PAGER_ENV[i]; i++) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tconst char *p = MAKE_PAGER_ENV[i];\n+\t\tconst char *eq = strchrnul(p, '=');\n+\n+\t\tstrbuf_add(&buf, p, eq - p);\n+\t\tif (!getenv(buf.buf))\n+\t\t\targv_array_push(env, p);\n+\t\tstrbuf_release(&buf);\n+\t}\n+}\n+\n void setup_pager(void)\n {\n \tconst char *pager = git_pager(isatty(1));\n+\tstruct argv_array env = ARGV_ARRAY_INIT;\n \n \tif (!pager || pager_in_use())\n \t\treturn;\n@@ -80,17 +99,10 @@ void setup_pager(void)\n \tpager_process.use_shell = 1;\n \tpager_process.argv = pager_argv;\n \tpager_process.in = -1;\n-\tif (!getenv(\"LESS\") || !getenv(\"LV\")) {\n-\t\tstatic const char *env[3];\n-\t\tint i = 0;\n-\n-\t\tif (!getenv(\"LESS\"))\n-\t\t\tenv[i++] = \"LESS=FRSX\";\n-\t\tif (!getenv(\"LV\"))\n-\t\t\tenv[i++] = \"LV=-c\";\n-\t\tenv[i] = NULL;\n-\t\tpager_process.env = env;\n-\t}\n+\n+\tsetup_pager_env(&env);\n+\tpager_process.env = argv_array_detach(&env, NULL);\n+\n \tif (start_command(&pager_process))\n \t\treturn;\n \ndiff --git a/script/mkcarray b/script/mkcarray\nnew file mode 100644\nindex 0000000..ed980ab\n--- /dev/null\n+++ b/script/mkcarray\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+\n+name=$1; shift\n+\n+quote() {\n+\techo \"$1\" | sed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/'\n+}\n+\n+cat <<-EOF\n+#ifndef ${name}_H\n+#define ${name}_H\n+\n+static const char *MAKE_${name}[] = {\n+EOF\n+\n+for i in $(cat); do\n+\tprintf '\\t\"%s\",\\n' \"$(quote \"$i\")\"\n+done\n+\n+cat < writes:\n\n> This patch moves all of the generated GIT-* files into\n> MAKE/*, with one exception: GIT-VERSION-FILE. This could be\n> moved along with the rest, but there is a reasonable chance\n> that there are some out-of-tree scripts that may peek at it\n> (whereas things like GIT-CFLAGS are purely internal, and we\n> update all internal references).\n\nOK. We do have internal references to include the version file to\nMakefiles in subdirectories, which we could adjust if we wanted to.\nI tend to agree that leaving it there would be a safer thing to do,\nbut at the same time I think it is OK to move it if we wanted to.\nThe third parties need to adjust and they are capable of adjusting.\n\nThanks. The changes look good. I do not have a strong opinion on\nthe name, but I do agree a separate directory unclutters things in a\nvery good way.\n\n> Signed-off-by: Jeff King \n> ---\n> I'm not married to the name \"MAKE\", but I do think the separate\n> directory is a win for simplicity and avoiding repetition (as you can\n> see in the diffstat). Other suggestions welcome.\n>\n> .gitignore | 8 -------\n> MAKE/.gitignore | 1 +\n> Makefile | 66 +++++++++++++++++++++++++--------------------------\n> perl/Makefile | 10 ++++----\n> t/perf/run | 2 +-\n> t/test-lib.sh | 2 +-\n> t/valgrind/analyze.sh | 4 ++--\n> 7 files changed, 42 insertions(+), 51 deletions(-)\n> create mode 100644 MAKE/.gitignore\n>\n> diff --git a/.gitignore b/.gitignore\n> index b5f9def..586a067 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -1,11 +1,3 @@\n> -/GIT-BUILD-OPTIONS\n> -/GIT-CFLAGS\n> -/GIT-LDFLAGS\n> -/GIT-PREFIX\n> -/GIT-PERL-DEFINES\n> -/GIT-PYTHON-VARS\n> -/GIT-SCRIPT-DEFINES\n> -/GIT-USER-AGENT\n> /GIT-VERSION-FILE\n> /bin-wrappers/\n> /git\n> diff --git a/MAKE/.gitignore b/MAKE/.gitignore\n> new file mode 100644\n> index 0000000..72e8ffc\n> --- /dev/null\n> +++ b/MAKE/.gitignore\n> @@ -0,0 +1 @@\n> +*\n> diff --git a/Makefile b/Makefile\n> index 60dc53b..7fecdf1 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1564,10 +1564,10 @@ endif\n> # usage: $(eval $(call make-var,FN,DESC,CONTENTS))\n> #\n> # Create a rule to write $CONTENTS (which should come from a make variable)\n> -# to GIT-$FN, but only if not already there. This can be used to create a\n> +# to MAKE/$FN, but only if not already there. This can be used to create a\n> # dependency on a Makefile variable. Prints $DESC to the user.\n> define make-var\n> -GIT-$1: FORCE\n> +MAKE/$1: FORCE\n> \t@VALUE='$$(subst ','\\'',$3)'; \\\n> \tif test x\"$$$$VALUE\" != x\"`cat $$@ 2>/dev/null`\"; then \\\n> \t\techo >&2 \" * new $2\"; \\\n> @@ -1658,7 +1658,7 @@ all:: profile-clean\n> endif\n> endif\n> \n> -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n> +all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) MAKE/BUILD-OPTIONS\n> ifneq (,$X)\n> \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n> endif\n> @@ -1714,25 +1714,25 @@ strip: $(PROGRAMS) git$X\n> # dependencies here will not need to change if the force-build\n> # details change some day.\n> \n> -git.sp git.s git.o: GIT-PREFIX\n> +git.sp git.s git.o: MAKE/PREFIX\n> git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n> \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n> \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n> \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n> \n> -git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n> +git$X: git.o MAKE/LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n> \t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n> \n> help.sp help.s help.o: common-cmds.h\n> \n> -builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX\n> +builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h MAKE/PREFIX\n> builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n> \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n> \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n> \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n> \n> -version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT\n> +version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT\n> version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n> \t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n> \t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n> @@ -1773,12 +1773,12 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n> $@.sh >$@+\n> endef\n> \n> -$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES\n> +$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh MAKE/SCRIPT-DEFINES\n> \t$(QUIET_GEN)$(cmd_munge_script) && \\\n> \tchmod +x $@+ && \\\n> \tmv $@+ $@\n> \n> -$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n> +$(SCRIPT_LIB) : % : %.sh MAKE/SCRIPT-DEFINES\n> \t$(QUIET_GEN)$(cmd_munge_script) && \\\n> \tmv $@+ $@\n> \n> @@ -1797,14 +1797,14 @@ perl/PM.stamp: FORCE\n> \t{ cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \\\n> \t$(RM) $@+\n> \n> -perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n> +perl/perl.mak: MAKE/CFLAGS MAKE/PREFIX perl/Makefile perl/Makefile.PL\n> \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n> \n> $(eval $(call make-var,PERL-DEFINES,perl-specific parameters,\\\n> \t:$(PERL_PATH)\\\n> \t:$(PERLLIB_EXTRA)\\\n> ))\n> -$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE\n> +$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak MAKE/PERL-DEFINES GIT-VERSION-FILE\n> \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n> \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n> \tINSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \\\n> @@ -1826,7 +1826,7 @@ gitweb:\n> \t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n> all:: gitweb\n> \n> -git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n> +git-instaweb: git-instaweb.sh MAKE/SCRIPT-DEFINES\n> \t$(QUIET_GEN)$(cmd_munge_script) && \\\n> \tchmod +x $@+ && \\\n> \tmv $@+ $@\n> @@ -1842,7 +1842,7 @@ endif # NO_PERL\n> \n> ifndef NO_PYTHON\n> $(eval $(call make-var,PYTHON-VARS,Python interpreter location,$(PYTHON_PATH)))\n> -$(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS\n> +$(SCRIPT_PYTHON_GEN): MAKE/CFLAGS MAKE/PREFIX MAKE/PYTHON-VARS\n> $(SCRIPT_PYTHON_GEN): % : %.py\n> \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n> \tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n> @@ -1984,13 +1984,13 @@ endif\n> endif\n> \n> ifndef CHECK_HEADER_DEPENDENCIES\n> -$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)\n> +$(C_OBJ): %.o: %.c MAKE/CFLAGS $(missing_dep_dirs)\n> \t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n> -$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)\n> +$(ASM_OBJ): %.o: %.S MAKE/CFLAGS $(missing_dep_dirs)\n> \t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n> endif\n> \n> -%.s: %.c GIT-CFLAGS FORCE\n> +%.s: %.c MAKE/CFLAGS FORCE\n> \t$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n> \n> ifdef USE_COMPUTED_HEADER_DEPENDENCIES\n> @@ -2011,25 +2011,25 @@ else\n> $(OBJECTS): $(LIB_H)\n> endif\n> \n> -exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX\n> +exec_cmd.sp exec_cmd.s exec_cmd.o: MAKE/PREFIX\n> exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n> \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n> \t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n> \t'-DPREFIX=\"$(prefix_SQ)\"'\n> \n> -builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX\n> +builtin/init-db.sp builtin/init-db.s builtin/init-db.o: MAKE/PREFIX\n> builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n> \t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n> \n> -config.sp config.s config.o: GIT-PREFIX\n> +config.sp config.s config.o: MAKE/PREFIX\n> config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n> \t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n> \n> -attr.sp attr.s attr.o: GIT-PREFIX\n> +attr.sp attr.s attr.o: MAKE/PREFIX\n> attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n> \t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n> \n> -gettext.sp gettext.s gettext.o: GIT-PREFIX\n> +gettext.sp gettext.s gettext.o: MAKE/PREFIX\n> gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n> \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n> \n> @@ -2051,21 +2051,21 @@ compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \\\n> compat/nedmalloc/nedmalloc.sp: SPARSE_FLAGS += -Wno-non-pointer-null\n> endif\n> \n> -git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n> +git-%$X: %.o MAKE/LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n> \n> -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n> +git-imap-send$X: imap-send.o MAKE/LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n> \t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n> \n> -git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n> +git-http-fetch$X: http.o http-walker.o http-fetch.o MAKE/LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n> \t\t$(LIBS) $(CURL_LIBCURL)\n> -git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n> +git-http-push$X: http.o http-push.o MAKE/LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n> \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n> \n> -git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)\n> +git-remote-testsvn$X: remote-testsvn.o MAKE/LDFLAGS $(GITLIBS) $(VCSSVN_LIB)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \\\n> \t$(VCSSVN_LIB)\n> \n> @@ -2075,7 +2075,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n> \tln -s $< $@ 2>/dev/null || \\\n> \tcp $< $@\n> \n> -$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)\n> +$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o MAKE/LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n> \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n> \n> @@ -2172,9 +2172,9 @@ $(eval $(call make-var,CFLAGS,build flags,$(ALL_CFLAGS)))\n> $(eval $(call make-var,LDFLAGS,link flags,$(ALL_LDFLAGS)))\n> \n> # We need to apply sq twice, once to protect from the shell\n> -# that runs GIT-BUILD-OPTIONS, and then again to protect it\n> +# that runs MAKE/BUILD-OPTIONS, and then again to protect it\n> # and the first level quoting from the shell that runs \"echo\".\n> -GIT-BUILD-OPTIONS: FORCE\n> +MAKE/BUILD-OPTIONS: FORCE\n> \t@echo SHELL_PATH=\\''$(subst ','\\'',$(SHELL_PATH_SQ))'\\' >$@\n> \t@echo PERL_PATH=\\''$(subst ','\\'',$(PERL_PATH_SQ))'\\' >>$@\n> \t@echo DIFF=\\''$(subst ','\\'',$(subst ','\\'',$(DIFF)))'\\' >>$@\n> @@ -2255,7 +2255,7 @@ test-svn-fe$X: vcs-svn/lib.a\n> \n> .PRECIOUS: $(TEST_OBJS)\n> \n> -test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\n> +test-%$X: test-%.o MAKE/LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n> \n> check-sha1:: test-sha1$X\n> @@ -2263,7 +2263,7 @@ check-sha1:: test-sha1$X\n> \n> SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n> \n> -$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n> +$(SP_OBJ): %.sp: %.c MAKE/CFLAGS FORCE\n> \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n> \t\t$(SPARSE_FLAGS) $<\n> \n> @@ -2477,9 +2477,7 @@ ifndef NO_TCLTK\n> \t$(MAKE) -C gitk-git clean\n> \t$(MAKE) -C git-gui clean\n> endif\n> -\t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS\n> -\t$(RM) GIT-USER-AGENT GIT-PREFIX\n> -\t$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS\n> +\t$(RM) GIT-VERSION-FILE MAKE/*\n> \n> .PHONY: all install profile-clean clean strip\n> .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n> diff --git a/perl/Makefile b/perl/Makefile\n> index 15d96fc..b27420e 100644\n> --- a/perl/Makefile\n> +++ b/perl/Makefile\n> @@ -41,7 +41,7 @@ modules += Git/SVN/Prompt\n> modules += Git/SVN/Ra\n> modules += Git/SVN/Utils\n> \n> -$(makfile): ../GIT-CFLAGS Makefile\n> +$(makfile): ../MAKE/CFLAGS Makefile\n> \techo all: private-Error.pm Git.pm Git/I18N.pm > $@\n> \tset -e; \\\n> \tfor i in $(modules); \\\n> @@ -79,11 +79,11 @@ $(makfile): ../GIT-CFLAGS Makefile\n> \techo instlibdir: >> $@\n> \techo '\techo $(instdir_SQ)' >> $@\n> else\n> -$(makfile): Makefile.PL ../GIT-CFLAGS\n> +$(makfile): Makefile.PL ../MAKE/CFLAGS\n> \t$(PERL_PATH) $< PREFIX='$(prefix_SQ)' INSTALL_BASE='' --localedir='$(localedir_SQ)'\n> endif\n> \n> # this is just added comfort for calling make directly in perl dir\n> -# (even though GIT-CFLAGS aren't used yet. If ever)\n> -../GIT-CFLAGS:\n> -\t$(MAKE) -C .. GIT-CFLAGS\n> +# (even though MAKE/CFLAGS aren't used yet. If ever)\n> +../MAKE/CFLAGS:\n> +\t$(MAKE) -C .. MAKE/CFLAGS\n> diff --git a/t/perf/run b/t/perf/run\n> index cfd7012..489d6cb 100755\n> --- a/t/perf/run\n> +++ b/t/perf/run\n> @@ -73,7 +73,7 @@ GIT_PERF_AGGREGATING_LATER=t\n> export GIT_PERF_AGGREGATING_LATER\n> \n> cd \"$(dirname $0)\"\n> -. ../../GIT-BUILD-OPTIONS\n> +. ../../MAKE/BUILD-OPTIONS\n> \n> if test $# = 0 -o \"$1\" = -- -o -f \"$1\"; then\n> \tset -- . \"$@\"\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1531c24..2c1ce73 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -48,7 +48,7 @@ then\n> \texit 1\n> fi\n> \n> -. \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n> +. \"$GIT_BUILD_DIR\"/MAKE/BUILD-OPTIONS\n> export PERL_PATH SHELL_PATH\n> \n> # if --tee was passed, write the output not only to the terminal, but\n> diff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh\n> index 2ffc80f..346e0ff 100755\n> --- a/t/valgrind/analyze.sh\n> +++ b/t/valgrind/analyze.sh\n> @@ -1,7 +1,7 @@\n> #!/bin/sh\n> \n> -# Get TEST_OUTPUT_DIRECTORY from GIT-BUILD-OPTIONS if it's there...\n> -. \"$(dirname \"$0\")/../../GIT-BUILD-OPTIONS\"\n> +# Get TEST_OUTPUT_DIRECTORY from MAKE/BUILD-OPTIONS if it's there...\n> +. \"$(dirname \"$0\")/../../MAKE/BUILD-OPTIONS\"\n> # ... otherwise set it to the default value.\n> : ${TEST_OUTPUT_DIRECTORY=$(dirname \"$0\")/..}\n"},{"id":"234313","messageId":"xmqqr47h4bn2.fsf@gitster.dls.corp.google.com","threadId":"35806","inReplyTo":"20140205175741.GH15218@sigill.intra.peff.net","subject":"Re: [PATCH 08/13] Makefile: introduce sq function for shell-quoting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T19:12:17Z","receivedAt":"2014-02-05T19:12:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Since we have recently abolished the prohibition on $(call)\n> in our Makefile, we can use it to factor out the repeated\n> shell-quoting we do. There are two upsides:\n>\n> 1. It is much more readable than inline calls to\n> $(subst ','\\'').\n>\n> 2. It is short enough that we can do away with the _SQ\n> variants of many variables (note that we do not do this\n> just yet, as there is a little more cleanup needed\n> first).\n\nYay.\n\n> .... But\n> many instances are not really any more readable (e.g., see the first\n> hunk below).\n> ...\n> .PHONY: install-perl-script install-sh-script install-python-script\n> install-sh-script: $(SCRIPT_SH_INS)\n> -\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> +\t$(INSTALL) $^ $(call sq,$(DESTDIR)$(gitexec_instdir))\n\nHmph, I do not see it as bad as the \"make-var\", which forces you to\nsay $(eval $(call ...)); this $(call sq, ...) is fairly readable.\n"},{"id":"234314","messageId":"xmqqmwi54bl7.fsf@gitster.dls.corp.google.com","threadId":"35806","inReplyTo":"20140205175809.GI15218@sigill.intra.peff.net","subject":"Re: [PATCH 09/13] Makefile: add c-quote helper function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T19:13:24Z","receivedAt":"2014-02-05T19:13:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> We have to c-quote strings coming from Makefile variables\n> when we pass them to the compiler via \"-D\". Now that we can\n> use $(call) in our Makefile, we can factor out the quoting\n> to make things easier to read. We can also apply it more\n> consistently, as there were many spots that should have been\n> C-quoting but did not. For example:\n>\n> make prefix='foo\\bar'\n>\n> would produce an exec_cmd.o with a broken prefix.\n>\n> Signed-off-by: Jeff King \n> ---\n> Makefile | 58 +++++++++++++++++++++++++++++-----------------------------\n> 1 file changed, 29 insertions(+), 29 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 868872f..b1c3fb4 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1568,6 +1568,17 @@ endif\n> sqi = $(subst ','\\'',$1)\n> sq = '$(call sqi,$1)'\n> \n> +# usage: $(call cq,CONTENTS)\n> +#\n> +# Quote the value as appropriate for a C string literal.\n> +cq = \"$(subst \",\\\",$(subst \\,\\\\,$1))\"\n> +\n> +# usage: $(call scq,CONTENTS)\n> +#\n> +# Quote the value as C string inside a shell string. Good for passing strings\n> +# on the command line via \"-DFOO=$(call # scq,$(FOO))\".\n\n\"call # scq\"???\n\n> +scq = $(call sq,$(call cq,$1))\n> +\n> # usage: $(eval $(call make-var,FN,DESC,CONTENTS))\n> #\n> # Create a rule to write $CONTENTS (which should come from a make variable)\n> @@ -1617,28 +1628,17 @@ LIB_OBJS += $(COMPAT_OBJS)\n> # Quote for C\n> \n> ifdef DEFAULT_EDITOR\n> -DEFAULT_EDITOR_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(DEFAULT_EDITOR)))\"\n> -DEFAULT_EDITOR_CQ_SQ = $(subst ','\\'',$(DEFAULT_EDITOR_CQ))\n> -\n> -BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'\n> +BASIC_CFLAGS += -DDEFAULT_EDITOR=$(call scq,$(DEFAULT_EDITOR))\n> endif\n> \n> ifdef DEFAULT_PAGER\n> -DEFAULT_PAGER_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(DEFAULT_PAGER)))\"\n> -DEFAULT_PAGER_CQ_SQ = $(subst ','\\'',$(DEFAULT_PAGER_CQ))\n> -\n> -BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'\n> +BASIC_CFLAGS += -DDEFAULT_PAGER=$(call scq,$(DEFAULT_PAGER))\n> endif\n> \n> ifdef SHELL_PATH\n> -SHELL_PATH_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(SHELL_PATH)))\"\n> -SHELL_PATH_CQ_SQ = $(subst ','\\'',$(SHELL_PATH_CQ))\n> -\n> -BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)'\n> +BASIC_CFLAGS += -DSHELL_PATH=$(call scq,$(SHELL_PATH))\n> endif\n> \n> -GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n> -GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n> $(eval $(call make-var,USER-AGENT,user agent string,$(GIT_USER_AGENT)))\n> \n> ifdef DEFAULT_HELP_FORMAT\n> @@ -1723,9 +1723,9 @@ strip: $(PROGRAMS) git$X\n> \n> git.sp git.s git.o: MAKE/PREFIX\n> git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n> -\t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n> -\t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n> -\t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n> +\t-DGIT_HTML_PATH=$(call scq,$(htmldir_relative)) \\\n> +\t-DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \\\n> +\t-DGIT_INFO_PATH=$(call scq,$(infodir_relative))\n> \n> git$X: git.o MAKE/LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n> @@ -1735,14 +1735,14 @@ help.sp help.s help.o: common-cmds.h\n> \n> builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h MAKE/PREFIX\n> builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n> -\t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n> -\t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n> -\t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n> +\t-DGIT_HTML_PATH=$(call scq,$(htmldir_relative)) \\\n> +\t-DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \\\n> +\t-DGIT_INFO_PATH=$(call scq,$(infodir_relative))\n> \n> version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT\n> version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n> -\t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n> -\t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n> +\t-DGIT_VERSION=$(call scq,$(GIT_VERSION)) \\\n> +\t-DGIT_USER_AGENT=$(call scq,$(GIT_USER_AGENT))\n> \n> $(BUILT_INS): git$X\n> \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n> @@ -2020,25 +2020,25 @@ endif\n> \n> exec_cmd.sp exec_cmd.s exec_cmd.o: MAKE/PREFIX\n> exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n> -\t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n> -\t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n> -\t'-DPREFIX=\"$(prefix_SQ)\"'\n> +\t-DGIT_EXEC_PATH=$(call scq,$(gitexecdir)) \\\n> +\t-DBINDIR=$(call scq,$(bindir_relative)) \\\n> +\t-DPREFIX=$(call scq,$(prefix))\n> \n> builtin/init-db.sp builtin/init-db.s builtin/init-db.o: MAKE/PREFIX\n> builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n> -\t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n> +\t-DDEFAULT_GIT_TEMPLATE_DIR=$(call scq,$(template_dir))\n> \n> config.sp config.s config.o: MAKE/PREFIX\n> config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n> -\t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n> +\t-DETC_GITCONFIG=$(call scq,$(ETC_GITCONFIG))\n> \n> attr.sp attr.s attr.o: MAKE/PREFIX\n> attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n> -\t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n> +\t-DETC_GITATTRIBUTES=$(call scq,$(ETC_GITATTRIBUTES))\n> \n> gettext.sp gettext.s gettext.o: MAKE/PREFIX\n> gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n> -\t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n> +\t-DGIT_LOCALE_PATH=$(call scq,$(localedir))\n> \n> http-push.sp http.sp http-walker.sp remote-curl.sp: SPARSE_FLAGS += \\\n> \t-DCURL_DISABLE_TYPECHECK\n"},{"id":"234315","messageId":"xmqqiost4bjz.fsf@gitster.dls.corp.google.com","threadId":"35806","inReplyTo":"20140205175859.GJ15218@sigill.intra.peff.net","subject":"Re: [PATCH 10/13] Makefile: drop *_SQ variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T19:14:08Z","receivedAt":"2014-02-05T19:14:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Again, Yay!\n"},{"id":"234316","messageId":"xmqqeh3h4beu.fsf@gitster.dls.corp.google.com","threadId":"35806","inReplyTo":"20140205180206.GK15218@sigill.intra.peff.net","subject":"Re: [PATCH 11/13] Makefile: auto-build C strings from make variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T19:17:13Z","receivedAt":"2014-02-05T19:17:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/script/mkcstring b/script/mkcstring\n> new file mode 100644\n> index 0000000..c01f430\n> --- /dev/null\n> +++ b/script/mkcstring\n> @@ -0,0 +1,18 @@\n> +#!/bin/sh\n> +\n> +name=$1; shift\n> +\n> +c_quote() {\n> +\tsed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/'\n\nNo 'g' for the second one?\n\n> +}\n> +\n> +cat <<-EOF\n> +#ifndef MAKE_${name}_H\n> +#define MAKE_${name}_H\n> +\n> +/* Auto-generated by mkcstring */\n> +\n> +#define MAKE_${name} \"$(c_quote)\"\n> +\n> +#endif /* MAKE_${name}_H */\n> +EOF\n> diff --git a/version.c b/version.c\n> index 6106a80..f68a93b 100644\n> --- a/version.c\n> +++ b/version.c\n> @@ -1,8 +1,10 @@\n> #include \"git-compat-util.h\"\n> #include \"version.h\"\n> #include \"strbuf.h\"\n> +#include \"MAKE/USER-AGENT-string.h\"\n> +#include \"MAKE/VERSION-string.h\"\n> \n> -const char git_version_string[] = GIT_VERSION;\n> +const char git_version_string[] = MAKE_VERSION;\n> \n> const char *git_user_agent(void)\n> {\n> @@ -11,7 +13,7 @@ const char *git_user_agent(void)\n> \tif (!agent) {\n> \t\tagent = getenv(\"GIT_USER_AGENT\");\n> \t\tif (!agent)\n> -\t\t\tagent = GIT_USER_AGENT;\n> +\t\t\tagent = MAKE_USER_AGENT;\n> \t}\n> \n> \treturn agent;\n"},{"id":"234317","messageId":"20140205191728.GA15489@sigill.intra.peff.net","threadId":"35806","inReplyTo":"xmqqmwi54bl7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 09/13] Makefile: add c-quote helper function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T19:17:28Z","receivedAt":"2014-02-05T19:17:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 11:13:24AM -0800, Junio C Hamano wrote:\n\n> > +# Quote the value as C string inside a shell string. Good for passing strings\n> > +# on the command line via \"-DFOO=$(call # scq,$(FOO))\".\n> \n> \"call # scq\"???\n\nWhoops. Bad rewrapping of the comment. It should obviously just be\n\n -DFOO=$(call scq,$(FOO))\n\n-Peff\n"},{"id":"234318","messageId":"20140205192033.GB15489@sigill.intra.peff.net","threadId":"35806","inReplyTo":"xmqqeh3h4beu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 11/13] Makefile: auto-build C strings from make variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T19:20:33Z","receivedAt":"2014-02-05T19:20:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 11:17:13AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > diff --git a/script/mkcstring b/script/mkcstring\n> > new file mode 100644\n> > index 0000000..c01f430\n> > --- /dev/null\n> > +++ b/script/mkcstring\n> > @@ -0,0 +1,18 @@\n> > +#!/bin/sh\n> > +\n> > +name=$1; shift\n> > +\n> > +c_quote() {\n> > +\tsed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/'\n> \n> No 'g' for the second one?\n\nThat's a bug. Thanks for catching.\n\nI tested most of these changes manually, but I missed this one by only\ntesting a value with a single quote. At one point I had introduced:\n\n $(eval $(call make-var,FOO,debug variable,$(FOO)))\n\nso you could do \"make MAKE/FOO\" and \"make MAKE/foo-string.c\", but I did\nnot include it in the series. Adding a test suite to our Makefile kind\nof seems like overkill, but as it gets complex, maybe some simple sanity\nchecks are worthwhile (not part of the regular test suite, but maybe\njust a \"./test-make\" script to make sure it behaves). I dunno.\n\n-Peff\n"},{"id":"234319","messageId":"20140205192350.GA16009@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205180857.GM15218@sigill.intra.peff.net","subject":"Re: [PATCH 13/13] move LESS/LV pager environment to Makefile","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T19:23:50Z","receivedAt":"2014-02-05T19:23:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 01:08:57PM -0500, Jeff King wrote:\n\n> +quote() {\n> +\techo \"$1\" | sed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/'\n> +}\n\nThis of course has the same \"/g\" bug as the earlier patch in the series.\n\nI was tempted to pull the function out into script/lib-c.sh, so that\nboth can source it, but perhaps that is overkill.\n\n-Peff\n"},{"id":"234320","messageId":"xmqqa9e54ayl.fsf@gitster.dls.corp.google.com","threadId":"35806","inReplyTo":"20140205180547.GL15218@sigill.intra.peff.net","subject":"Re: [PATCH 12/13] Makefile: teach scripts to include make variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T19:26:58Z","receivedAt":"2014-02-05T19:26:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> define cmd_munge_script\n> $(RM) $@ $@+ && \\\n> +{ \\\n> +includes=\"$(filter MAKE/%.sh,$^)\"; \\\n> +if ! test -z \"$$includes\"; then \\\n> +\tcat $$includes; \\\n> +fi && \\\n> sed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n> -e 's|@SHELL_PATH@|$(call sqi,$(SHELL_PATH))|' \\\n> - -e 's|@@DIFF@@|$(call sqi,$(DIFF))|' \\\n> -e 's|@@LOCALEDIR@@|$(call sqi,$(localedir))|g' \\\n> -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n> -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n> -e $(BROKEN_PATH_FIX) \\\n> -e 's|@@GITWEBDIR@@|$(call sqi,$(gitwebdir))|g' \\\n> -e 's|@@PERL@@|$(call sqi,$(PERL_PATH))|g' \\\n> - $@.sh >$@+\n> + $@.sh; \\\n> +} >$@+\n> endef\n\nSorry, but I am not quite sure what is going on here.\n\n - if $includes does not exist, cat $includes will barf but that is\n OK;\n\n - if $includes is empty, there is no point running cat so that is\n OK;\n\n - if $includes is not empty, we want to cat.\n\nAnd then after emitting that piece, we start processing the *.sh\nsource file, replacing she-bang line?\n\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index fffa3c7..627d289 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -285,7 +285,7 @@ clear_local_git_env() {\n> # remove lines from $1 that are not in $2, leaving only common lines.\n> create_virtual_base() {\n> \tsz0=$(wc -c <\"$1\")\n> -\t@@DIFF@@ -u -La/\"$1\" -Lb/\"$1\" \"$1\" \"$2\" | git apply --no-add\n> +\t$MAKE_DIFF -u -La/\"$1\" -Lb/\"$1\" \"$1\" \"$2\" | git apply --no-add\n> \tsz1=$(wc -c <\"$1\")\n\nThis would mean that after this mechanism is extensively employed\nthroughout our codebase, any random environment variable the user\nhas whose name happens to begin with \"MAKE_\" will interfere with us\n(rather, we will override such a variable while we run). Having to\ncarve out our own namespace in such a way is OK, but we would want\nto see that namespace somewhat related to the name of our project,\nnot to the name of somebody else's like \"make\", no?\n\n> \n> \t# If we do not have enough common material, it is not\n> diff --git a/script/mksh b/script/mksh\n> new file mode 100644\n> index 0000000..d41e77a\n> --- /dev/null\n> +++ b/script/mksh\n> @@ -0,0 +1,4 @@\n> +#!/bin/sh\n> +\n> +name=$1; shift\n> +printf \"MAKE_%s='%s'\\n\" \"$name\" \"$(sed \"s/'/'\\\\''/g\")\"\n"},{"id":"234322","messageId":"20140205195052.GA16634@sigill.intra.peff.net","threadId":"35806","inReplyTo":"xmqqa9e54ayl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 12/13] Makefile: teach scripts to include make variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T19:50:52Z","receivedAt":"2014-02-05T19:50:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 11:26:58AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > define cmd_munge_script\n> > $(RM) $@ $@+ && \\\n> > +{ \\\n> > +includes=\"$(filter MAKE/%.sh,$^)\"; \\\n> > +if ! test -z \"$$includes\"; then \\\n> > +\tcat $$includes; \\\n> > +fi && \\\n> > sed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \\\n> > -e 's|@SHELL_PATH@|$(call sqi,$(SHELL_PATH))|' \\\n> > - -e 's|@@DIFF@@|$(call sqi,$(DIFF))|' \\\n> > -e 's|@@LOCALEDIR@@|$(call sqi,$(localedir))|g' \\\n> > -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n> > -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n> > -e $(BROKEN_PATH_FIX) \\\n> > -e 's|@@GITWEBDIR@@|$(call sqi,$(gitwebdir))|g' \\\n> > -e 's|@@PERL@@|$(call sqi,$(PERL_PATH))|g' \\\n> > - $@.sh >$@+\n> > + $@.sh; \\\n> > +} >$@+\n> > endef\n> \n> Sorry, but I am not quite sure what is going on here.\n> [...]\n> And then after emitting that piece, we start processing the *.sh\n> source file, replacing she-bang line?\n\nYes, there is a bug here. The intent was to end up with:\n\n #!/bin/sh\n [include snippets]\n [the actual script]\n\nbut of course I bungled that, because #!/bin/sh is part of the file\nalready, and our snippets should not come before. If we take this\ntechnique to its logical conclusion, the sed replacement goes away\nentirely, and we end up with something like:\n\n %: %.sh MAKE/SHELL_SHEBANG.sh\n { cat $(filter MAKE/%.sh,$^) && sed 1d $<; } >$@+\n chmod +x $@+\n mv $@+ $@\n\n MAKE/SHELL_SHEBANG.sh: MAKE/SHELL_SHEBANG\n echo \"#!$(head -1 $<)\" >$@+ &&\n mv $@+ $@\n\nI.e., the shebang line simply becomes the first snippet.\n\n> > create_virtual_base() {\n> > \tsz0=$(wc -c <\"$1\")\n> > -\t@@DIFF@@ -u -La/\"$1\" -Lb/\"$1\" \"$1\" \"$2\" | git apply --no-add\n> > +\t$MAKE_DIFF -u -La/\"$1\" -Lb/\"$1\" \"$1\" \"$2\" | git apply --no-add\n> > \tsz1=$(wc -c <\"$1\")\n> \n> This would mean that after this mechanism is extensively employed\n> throughout our codebase, any random environment variable the user\n> has whose name happens to begin with \"MAKE_\" will interfere with us\n> (rather, we will override such a variable while we run). Having to\n> carve out our own namespace in such a way is OK, but we would want\n> to see that namespace somewhat related to the name of our project,\n> not to the name of somebody else's like \"make\", no?\n\nYes, it probably makes sense to carve out our own namespace. I kind of\ndislike the use of environment variables at all, though, just because it\nreally bleeds through into how you write the script (as opposed to just\nreplacing like we do now, or in C, where a snippet defining a\npreprocessor macro is just fine).\n\nAn alternative is that we could do something like:\n\n %: %.sh\n script/mkshellscript $^ >$@+ &&\n chmod +x $@+ &&\n mv $@+ $@\n\nand then have mkshellscript look like (and this could obviously be\ninline in the Makefile, but I think it's worth pulling it out to avoid\nthe quoting hell):\n\n #!/bin/sh\n\n main=$1; shift\n\n sed_quote() {\n sed 's/\\\\/\\\\\\\\/g; s/|/\\\\|/g'\n }\n\n # generate a sed expression that will replace tokens; if we are given\n # the file MAKE/FOO.sh, the expression will replace @@FOO@@ with the\n # contents of that file\n sed_replace() {\n for var in \"$@\"; do\n key=${i#MAKE/}\n key=${key%.sh}\n printf 's|@@%s@@|%s|g' \"$key\" \"$(sed_quote <$var)\"\n done\n }\n\n sed \"$(sed_replace \"$@\")\" <\"$main\"\n\n-Peff\n"},{"id":"234323","messageId":"20140205195206.GA16753@sigill.intra.peff.net","threadId":"35806","inReplyTo":"20140205192350.GA16009@sigill.intra.peff.net","subject":"Re: [PATCH 13/13] move LESS/LV pager environment to Makefile","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T19:52:06Z","receivedAt":"2014-02-05T19:52:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 02:23:50PM -0500, Jeff King wrote:\n\n> On Wed, Feb 05, 2014 at 01:08:57PM -0500, Jeff King wrote:\n> \n> > +quote() {\n> > +\techo \"$1\" | sed 's/\\\\/\\\\\\\\/g; s/\"/\\\\\"/'\n> > +}\n> \n> This of course has the same \"/g\" bug as the earlier patch in the series.\n> \n> I was tempted to pull the function out into script/lib-c.sh, so that\n> both can source it, but perhaps that is overkill.\n\nAlso, this (and the other spot) should probably be using \"printf\"\ninstead of echo for portability (i.e., the config variables might have\nbackslashes in them).\n\n-Peff\n"},{"id":"234356","messageId":"CAPig+cTPUHJ6wDPWL_v=_7Zq6O=r2g2Hz02TQHZO+0gq60Fbnw@mail.gmail.com","threadId":"35806","inReplyTo":"20140205175002.GC15218@sigill.intra.peff.net","subject":"Re: [PATCH 03/13] Makefile: introduce make-var helper function","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-02-06T08:55:23Z","receivedAt":"2014-02-06T08:55:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Feb 5, 2014 at 12:50 PM, Jeff King wrote:\n> It's a common pattern in our Makefile to echo some make\n> variables into a file, but only if they are different from a\n> previous run. This sentinel file can then be used as a\n> dependency to trigger rebuilds when the make variable\n> changes.\n>\n> The code to do this is a bit ugly and repetetive; let's\n\ns/repetetive/repetitive/\n\n> factor it out into a reusable function.\n>\n> Note that this relies on the \"call\" and \"eval\" functions of\n> GNU make. We previously avoided using \"call\", as explained\n> in 39c015c (Fixes for ancient versions of GNU make,\n> 2006-02-18). However, it has been 8 years since then, so\n> perhaps its use is acceptable now.\n>\n> The \"call\" function dates back to GNU make 3.77.90\n> (1997-07-21). The \"eval\" function dates back to 3.80\n> (2002-07-08).\n>\n> If it's still a problem to use these functions, we can\n> do similar meta-programming with something like:\n>\n> include magic.mak\n> magic.mak:\n> ./generate-magic-rules >$@+\n> mv $@+ $@\n>\n> where the rules are generated by a shell (or other) script.\n>\n> Signed-off-by: Jeff King \n"},{"id":"234533","messageId":"87a9e1l1jv.fsf@thomasrast.ch","threadId":"35806","inReplyTo":"20140205180547.GL15218@sigill.intra.peff.net","subject":"Re: [PATCH 12/13] Makefile: teach scripts to include make variables","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-02-08T21:47:16Z","receivedAt":"2014-02-08T21:47:16Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King writes:\n\n> The current scheme for getting build-time variables into a\n> shell script is to munge the script with sed, and stick the\n> munged variable into a special sentinel file so that \"make\"\n> knows about the dependency.\n>\n> Instead, we can combine both functions by generating a shell\n> snippet with our value, and then \"building\" shell scripts by\n> concatenating their snippets. \"make\" then handles the\n> dependency automatically, and it's easy to generate tighter\n> dependencies.\n>\n> We demonstrate here by moving the \"DIFF\" substitution into\n> its own snippet, which lets us rebuild only the single\n> affected file when it changes.\n\nI can't look right now *why* this happens, but this breaks\n./t2300-cd-to-toplevel.sh --valgrind with messages like\n\n expecting success: \n (\n cd 'repo' &&\n . \"$(git --exec-path)\"/git-sh-setup &&\n cd_to_toplevel &&\n [ \"$(pwd -P)\" = \"$TOPLEVEL\" ]\n )\n\n ./test-lib.sh: line 414: /home/thomas/g/t/valgrind/bin/git-sh-setup: No such file or directory\n not ok 1 - at physical root\n #\n # (\n # cd 'repo' &&\n # . \"$(git --exec-path)\"/git-sh-setup &&\n # cd_to_toplevel &&\n # [ \"$(pwd -P)\" = \"$TOPLEVEL\" ]\n # )\n #\n\nI don't know why it only affects this test, or why it doesn't break when\nwithin 'git bisect run' -- probably there's something funky going on in\nthe environment, quite possibly in my own configs.\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"},{"id":"234565","messageId":"20140210011511.GA12773@sigill.intra.peff.net","threadId":"35806","inReplyTo":"87a9e1l1jv.fsf@thomasrast.ch","subject":"Re: [PATCH 12/13] Makefile: teach scripts to include make variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-10T01:15:14Z","receivedAt":"2014-02-10T01:15:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 08, 2014 at 10:47:16PM +0100, Thomas Rast wrote:\n\n> Jeff King writes:\n> \n> > The current scheme for getting build-time variables into a\n> > shell script is to munge the script with sed, and stick the\n> > munged variable into a special sentinel file so that \"make\"\n> > knows about the dependency.\n> >\n> > Instead, we can combine both functions by generating a shell\n> > snippet with our value, and then \"building\" shell scripts by\n> > concatenating their snippets. \"make\" then handles the\n> > dependency automatically, and it's easy to generate tighter\n> > dependencies.\n> >\n> > We demonstrate here by moving the \"DIFF\" substitution into\n> > its own snippet, which lets us rebuild only the single\n> > affected file when it changes.\n> \n> I can't look right now *why* this happens, but this breaks\n> ./t2300-cd-to-toplevel.sh --valgrind with messages like\n\nI think it's the bug that Junio already pointed out; git-sh-setup gets\nthe DIFF=... snippet instead of the initial #!-line. I didn't look at\nthe details, but that probably screws up valgrind's symlinking, since we\nno longer realize it's a shell script.\n\nOnce that bug is fixed, I'll double-check that the problem goes away.\n\n-Peff\n"}]}