All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] t0300: work around bug in dash 0.5.6
       [not found] <93904081120af9a646b8bda96aa2da8e85cc063d.1330700524.git.git@drmicha.warpmail.net>
@ 2012-03-03  0:37 ` Jeff King
  0 siblings, 0 replies; only message in thread
From: Jeff King @ 2012-03-03  0:37 UTC (permalink / raw)
  To: git; +Cc: Michael J Gruber

From: Michael J Gruber <git@drmicha.warpmail.net>

The construct 'while IFS== read' makes dash 0.5.6 execute
read without changing IFS, which results in test breakages
all over the place in t0300.  Neither dash 0.5.5.1 and older
nor dash 0.5.7 and newer are affected: The problem was
introduded resp. fixed by the commits

  55c46b7 ([BUILTIN] Honor tab as IFS whitespace when
           splitting fields in readcmd, 2009-08-11)

  1d806ac ([VAR] Do not poplocalvars prematurely on regular
           utilities, 2010-05-27)

in http://git.kernel.org/?p=utils/dash/dash.git

Putting 'IFS==' before that line makes all versions of dash
work.

This looks like a dash bug, not a misinterpretation of the
standard. However, it's worth working around for two
reasons. One, this version of dash was released in Fedora
14-16, so the bug is found in the wild. And two, at least
one other shell, Solaris /bin/sh, choked on this by
persisting IFS after the read invocation. That is not a
shell we usually care about, and I think this use of IFS is
acceptable by POSIX (which allows other behavior near
"special builtins", but "read" is not one of those). But it
seems that this may be a subtle, not-well-tested case for
some shells. Given that the workaround is so simple, it's
worth just being defensive.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jeff King <peff@peff.net>
---
Michael and I discussed this off-list. The patch is his, but I've
rewritten much of the commit message to incorporate the recent thread
over this exact same bit of code on Solaris:

  http://thread.gmane.org/gmane.comp.version-control.git/189680

This patch is basically the same as Ben Walton's from that thread, but
we ended up creating and using the write_script function to avoid using
Solaris /bin/sh at all.

 t/t0300-credentials.sh |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 8621ab0..20e28e3 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -8,10 +8,13 @@ test_expect_success 'setup helper scripts' '
 	cat >dump <<-\EOF &&
 	whoami=`echo $0 | sed s/.*git-credential-//`
 	echo >&2 "$whoami: $*"
-	while IFS== read key value; do
+	OIFS=$IFS
+	IFS==
+	while read key value; do
 		echo >&2 "$whoami: $key=$value"
 		eval "$key=$value"
 	done
+	IFS=$OIFS
 	EOF
 
 	write_script git-credential-useless <<-\EOF &&
-- 
1.7.6.6.7.g65e2

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2012-03-03  0:37 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <93904081120af9a646b8bda96aa2da8e85cc063d.1330700524.git.git@drmicha.warpmail.net>
2012-03-03  0:37 ` [PATCH] t0300: work around bug in dash 0.5.6 Jeff King

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.