Re: [PATCH 1/4] test: Add target test-lint-shell-syntax

From: Jeff King <peff@peff.net>
Date: Jan 2, 2013, 23:22 UTC
Message-ID: <20130102232239.GA27952@sigill.intra.peff.net>
In-Reply-To: <50E4BF58.4090808@web.de>

On Thu, Jan 03, 2013 at 12:14:32AM +0100, Torsten Bögershausen wrote:

Show 8 quoted lines

> > This is wrong if $(PERL_PATH) contains spaces, no? Doing "$(PERL_PATH)"
> > is also wrong, because the expansion happens in 'make', and a
> > $(PERL_PATH) with double-quotes would fool the shell. Since we export
> > $PERL_PATH, I think doing:
> > 
> >   "$$PERL_PATH"" check-non-portable-shell.pl $(T)
> Thanks, but:             
> - The double "" after PERL_PATH makes the string un-terminated.

Yeah, sorry, typo on my part.

> - Using "$$PERL_PATH" expands from make into "$PERL_PATH" on the command line

Right. That's what I intended.

Show 8 quoted lines

> - If the Makefile looks like this:
> PERL_PATH = "/Users/tb/projects/git/tb/pe rl"
> [snip]
> $(PERL_PATH) check-non-portable-shell.pl $(T)
> The command line will look like this:
> "/Users/tb/projects/git/tb/pe rl" check-non-portable-shell.pl t0000-basic.sh ...
> 
> So I think that PERL_PATH should be quoted when it is defined in the Makefile.

Does a $PERL_PATH with quotes actually work?

Usually in our runtime environment, commands that are handed to git are assumed to be passed directly to the shell, and you need to quote. E.g., setting diff.external to:

  [diff]
  external = "foo --bar"

will let the shell split the argument out; if you have a space, you would want to set it like:

  [diff]
  external = "'command with space'"

This is the most flexible way to do it.

However, for Makefile variables, I think we do not (and cannot) follow the same rule. Notice that all of the uses of $PERL_PATH in the test suite enclose it in quotes. Having extra quotes would break those invocations. And the value of $PERL_PATH will be put on the #!-line, which cannot not be quoted.

-Peff