Re: [PATCH 2/2] Makefile: work around ksh's failure to handle missing list argument to for loop
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 5, 2010, 18:18 UTC
- Message-ID
- <7vy6dpyeqf.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <4C31796E.9050109@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Before the test for emptyness can happen, the complete statement must be > parsed, but ksh finds a syntax error in the statement and, therefore, > cannot even begin to execute the statement. (ksh doesn't follow POSIX in > this regard, where this would not be a syntax error.)
I had to stare at Brandon's patch that was essentially:
- for p in $(FOO); do echo $$p; done + foo=$(FOO); for p in $$foo; do echo $$p; done
and the above two doesn't look like there should be any difference; your explanation makes quite a lot of sense but that's arcane. I doubt I will be able to justify and explain the latter construction without consulting your message I am responsing to, if somebody asks "why do we assign $(FOO) to a shell variable and then iterate over it?" 6 months from now.
It might make sense to use $(foreach) instead of rolling our own loop in the shell to avoid glitches like this.
Makefile | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile index 9aca8a1..8bbb574 100644 --- a/Makefile +++ b/Makefile @@ -2085,13 +2085,13 @@ endif ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \ cp "$$execdir/git$X" "$$execdir/$$p" || exit; \ done; } && \ - { test x"$(REMOTE_CURL_ALIASES)" = x || \ - { for p in $(REMOTE_CURL_ALIASES); do \ - $(RM) "$$execdir/$$p" && \ - ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \ - ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \ - cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; \ - done; } ; } && \ + $(foreach p,$(REMOTE_CURL_ALIASES), \ + { \ + $(RM) "$$execdir/$p" && \ + ln "$$execdir/git-remote-http$X" "$$execdir/$p" 2>/dev/null || \ + ln -s "git-remote-http$X" "$$execdir/$p" 2>/dev/null || \ + cp "$$execdir/git-remote-http$X" "$$execdir/$p" || exit; \ + } && ) : \ ./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X" install-gitweb: