Re: [PATCH] Makefile: escape ' in bin-wrappers rule

From: Jeff King <peff@peff.net>
Date: Mar 16, 2017, 11:24 UTC
Message-ID: <20170316112447.meosrjddumkjrfiu@sigill.intra.peff.net>
In-Reply-To: <20170316061738.26760-1-lehmacdj@gmail.com>

On Thu, Mar 16, 2017 at 02:17:38AM -0400, Devin Lehmacher wrote:

> If the pwd contains a ' escape it as '\'' so that git will not fail
> while building on a path containing '.

I think this only fixes half the problem...

Show 10 quoted lines

> diff --git a/Makefile b/Makefile
> index ed68700ac..5cf5d8537 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2276,7 +2276,7 @@ all:: $(NO_INSTALL)
>  bin-wrappers/%: wrap-for-bin.sh
>  	@mkdir -p bin-wrappers
>  	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
> -	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
> +	     -e 's|@@BUILD_DIR@@|$(shell pwd | sed -e "s/'/'\\\''/g")|' \

This escapes the single-quote against the shell run by make. But it does a blind text substitution inside the wrap-for-bin script itself, which means it's subject to another round of expansion. E.g.:

  $ git clone git foo\'git
  $ cd foo\'git
  $ make bin-wrappers/git
  GIT_VERSION = 2.12.0.623.g86ec6c963
  /bin/sh: 4: Syntax error: Unterminated quoted string
  Makefile:2296: recipe for target 'bin-wrappers/git' failed
  make: *** [bin-wrappers/git] Error 2

  $ git am /your/patch
  $ make bin-wrappers/git
  GIT_VERSION = 2.12.0.624.gc9787fbf0
      GEN bin-wrappers/git
  $ bin-wrappers/git
  bin-wrappers/git: 16: bin-wrappers/git: Syntax error: "fi" unexpected

So you'd need an extra layer of quoting. And then to top it all off, passing backslashes through sed requires _another_ layer of quoting. Something like this:

diff --git a/Makefile b/Makefile
index 17e381f0c..9c1357a77 100644
--- a/Makefile
+++ b/Makefile
@@ -2292,10 +2292,15 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 all:: $(NO_INSTALL)
 
+PWD = $(shell pwd)
+PWD_SQ = $(subst ','\'',$(PWD))
+PWD_SQ_SED = $(subst \,\\,$(PWD_SQ))
+PWD_SQ_SED_SQ = $(subst ','\'',$(PWD_SQ_SED))
+
 bin-wrappers/%: wrap-for-bin.sh
 	@mkdir -p bin-wrappers
 	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
+	     -e 's|@@BUILD_DIR@@|$(PWD_SQ_SED_SQ)|' \
 	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' < $< > $@ && \
 	chmod +x $@
 

That at least gets me as far as building a runnable git. I wouldn't be
surprised if there are other gotchas, though. Running "make test" dies
in t0060 due to similar substitutions.

I guess if somebody is really determined they can try to fix every spot.
But at some point I wonder if the answer is "if it hurts, don't do it".

-Peff