On 03.01.13 00:22, Jeff King wrote:
> On Thu, Jan 03, 2013 at 12:14:32AM +0100, Torsten Bögershausen wrote:
>
>>> 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.
>
>> - 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?
At least on my system the following combination works:
git diff
diff --git a/t/Makefile b/t/Makefile
index f8f8c54..391a5ca 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -8,7 +8,7 @@
#GIT_TEST_OPTS = --verbose --debug
SHELL_PATH ?= $(SHELL)
-PERL_PATH ?= /usr/bin/perl
+PERL_PATH = "/Users/tb/projects/git/tb/pe rl"
TAR ?= $(TAR)
RM ?= rm -f
PROVE ?= prove
~/projects/git/tb/t> ls -l "/Users/tb/projects/git/tb/pe rl"
lrwxr-xr-x 1 tb staff 13 Jan 3 00:33 /Users/tb/projects/git/tb/pe rl -> /usr/bin/perl
====================================================
And this works as well, is that what you intended?
Note: "single Dollar"
====================================================
git diff
diff --git a/t/Makefile b/t/Makefile
index f8f8c54..f624f95 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -8,7 +8,7 @@
#GIT_TEST_OPTS = --verbose --debug
SHELL_PATH ?= $(SHELL)
-PERL_PATH ?= /usr/bin/perl
+PERL_PATH = /Users/tb/projects/git/tb/pe rl
TAR ?= $(TAR)
RM ?= rm -f
PROVE ?= prove
@@ -57,7 +57,7 @@ test-lint-executable:
echo >&2 "non-executable tests:" $$bad; exit 1; }
test-lint-shell-syntax:
- $(PERL_PATH) check-non-portable-shell.pl $(T)
+ "$(PERL_PATH)" check-non-portable-shell.pl $(T)
> 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
I followed these lines as an example:
test-results/git-smoke.tar.gz: test-results
$(PERL_PATH) ./harness \
--archive="test-results/git-smoke.tar.gz" \
$(T)
(and make smoke did not work, as we don't have ./harness :-(
Do we need some cleanup/improvements here as well?
/Torsten
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html