All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brandon Casey <casey@nrlssc.navy.mil>
To: gitster@pobox.com
Cc: git@vger.kernel.org, j.sixt@viscovery.net,
	git@drmicha.warpmail.net, PWalker752@aol.com,
	newsletter@dirk.my1.cc, bruce.stephens@isode.com,
	Brandon Casey <drafnel@gmail.com>
Subject: [PATCH 2/2 v2] Makefile: work around ksh's failure to handle missing list argument to for loop
Date: Tue,  6 Jul 2010 14:56:51 -0700	[thread overview]
Message-ID: <vUheaJEBKF4RpCQidVNBD1qrdnnqrcfK_TfSmMUj8l53PCm3F-faBNN2Pxk97G5Mf5GY3dVoDgwkpzK8xO7nXw@cipher.nrlssc.navy.mil> (raw)
In-Reply-To: <7v630t72vo.fsf@alter.siamese.dyndns.org>

From: Brandon Casey <drafnel@gmail.com>

ksh does not like it when the list argument is missing in a 'for' loop.
This can happen when NO_CURL is set which causes REMOTE_CURL_ALIASES to be
unset.  In this case, the 'for' loop in the Makefile is expanded to look
like this:

   for p in ; do

and ksh complains like this:

   /bin/ksh: syntax error at line 15 : `;' unexpected

The existing attempt to work around this issue, introduced by 70b89f87,
tried to protect the 'for' loop by first testing whether REMOTE_CURL_ALIASES
was empty, but this does not work since, as Johannes Sixt explains, "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.)".

Make's $(foreach) function could be used to avoid this shell glitch, but
since it has already caused a problem once before by generating a command
line that exceeded the maximum argument list length on IRIX, let's adopt
Bruce Stephens's suggestion for working around this issue in the same way
the OpenSSL folks have done it.  This solution first assigns the contents
of the REMOTE_CURL_ALIASES make variable to a shell variable and then
supplies the shell variable as the list argument in the 'for' loop.  This
satisfies ksh and has the expected behavior even if $(REMOTE_CURL_ALIASES)
is empty.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


On 07/05/2010 09:36 PM, Junio C Hamano wrote:
> Well your patch fixes the issue, and I would actually prefer it as long as
> it is explained well ;-).

Heh, ok, hopefully this commit message does a better job.

-Brandon


 Makefile |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 527d872..bc3c570 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 \
+	remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \
+	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; } && \
+	done && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
 install-gitweb:
-- 
1.7.2.rc1

      reply	other threads:[~2010-07-06 21:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-26 13:35 installation issue when building with NO_CURL=YesPlease Paul Walker
2010-05-26 13:58 ` Michael J Gruber
2010-05-26 14:41   ` Paul Walker
2010-05-26 13:58 ` Ramkumar Ramachandra
2010-05-26 14:15   ` Paul Walker
2010-05-26 14:24     ` [PATCH] Makefile: reenable install with NO_CURL Michael J Gruber
2010-05-26 18:27     ` installation issue when building with NO_CURL=YesPlease Dirk Süsserott
2010-05-26 18:45       ` Bruce Stephens
2010-05-26 19:03         ` Dirk Süsserott
2010-07-02 18:50         ` [PATCH 1/2] Makefile: remove some unnecessary curly braces Brandon Casey
2010-07-02 18:50         ` [PATCH 2/2] Makefile: work around ksh's failure to handle missing list argument to for loop Brandon Casey
2010-07-03  6:21           ` Raja R Harinath
2010-07-04 18:37           ` Michael J Gruber
2010-07-05  6:19             ` Johannes Sixt
2010-07-05  8:14               ` Michael J Gruber
2010-07-05  9:15                 ` Johannes Sixt
2010-07-05  9:31                   ` Michael J Gruber
2010-07-05 18:18               ` Junio C Hamano
2010-07-05 20:00                 ` Andreas Schwab
2010-07-05 21:29                 ` Brandon Casey
2010-07-06  2:36                   ` Junio C Hamano
2010-07-06 21:56                     ` Brandon Casey [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=vUheaJEBKF4RpCQidVNBD1qrdnnqrcfK_TfSmMUj8l53PCm3F-faBNN2Pxk97G5Mf5GY3dVoDgwkpzK8xO7nXw@cipher.nrlssc.navy.mil \
    --to=casey@nrlssc.navy.mil \
    --cc=PWalker752@aol.com \
    --cc=bruce.stephens@isode.com \
    --cc=drafnel@gmail.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=newsletter@dirk.my1.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.