# [BUGS] test failure in t4014-format-patch.sh with master 12 messages from 2008-04-17 to 2008-04-19. Participants: Remi Vanicat, Linus Torvalds, Björn Steinbrink, Rémi Vanicat, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/13155 ## Remi Vanicat, 2008-04-17 09:30 Subject: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <87od88eslq.dlv@maison.homelinux.org> URL: https://gitlist.dev/e/87od88eslq.dlv%40maison.homelinux.org ``` Hello, While trying to wrote a test for git, I discover that there is a test failure in master on my computer: $ ./t4014-format-patch.sh [....] * FAIL 8: replay did not screw up the log message git cat-file commit rebuild-1 | grep "^Side .* with .* backslash-n" It seem that now, somewhere a \n has been interpreted as a newline. Bisecting point the problem to the 2e6e3e82 commit "git-am: cope better with an empty Subject: line" -- Rémi Vanicat ``` ## Linus Torvalds, 2008-04-17 15:11 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.00.0804170806430.2879%40woody.linux-foundation.org In-Reply-To: <87od88eslq.dlv@maison.homelinux.org> ``` On Thu, 17 Apr 2008, Remi Vanicat wrote: > > While trying to wrote a test for git, I discover that there is a test > failure in master on my computer: Hmm. Doesn't fail for me. > $ ./t4014-format-patch.sh > [....] > * FAIL 8: replay did not screw up the log message > > git cat-file commit rebuild-1 | grep "^Side .* with .* backslash-n" > > It seem that now, somewhere a \n has been interpreted as a newline. I wonder if it is some shell-specific variable expansion issue. What's your /bin/sh? (I assume, from your email address, that you're using Debian, and I thought Debian used bash, but maybe that's not true). What does a="hello\nhi" echo $a result in? Linus ``` ## Björn Steinbrink, 2008-04-17 15:34 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <20080417153421.GA13117@atjola.homenet> URL: https://gitlist.dev/e/20080417153421.GA13117%40atjola.homenet In-Reply-To: ``` On 2008.04.17 08:11:52 -0700, Linus Torvalds wrote: > > > On Thu, 17 Apr 2008, Remi Vanicat wrote: > > > > While trying to wrote a test for git, I discover that there is a test > > failure in master on my computer: > > Hmm. Doesn't fail for me. > > > $ ./t4014-format-patch.sh > > [....] > > * FAIL 8: replay did not screw up the log message > > > > git cat-file commit rebuild-1 | grep "^Side .* with .* backslash-n" > > > > It seem that now, somewhere a \n has been interpreted as a newline. > > I wonder if it is some shell-specific variable expansion issue. What's > your /bin/sh? (I assume, from your email address, that you're using > Debian, and I thought Debian used bash, but maybe that's not true). > > What does > > a="hello\nhi" > echo $a > > result in? Debian has its own dash which is supposed to be a lightweight alternative to bash and "for checking POSIX compliance of scripts" (quote from the packages description). I don't happen to know off-hand whether POSIX says that echo should default to -e, but dash seems to do that: $ dash $ a="hello\nhi" $ echo $a hello hi Björn ``` ## Rémi Vanicat, 2008-04-17 15:44 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <6b8a91420804170844t2776e8a7sf735e3f87a95298b@mail.gmail.com> URL: https://gitlist.dev/e/6b8a91420804170844t2776e8a7sf735e3f87a95298b%40mail.gmail.com In-Reply-To: ``` 2008/4/17, Linus Torvalds : > > > On Thu, 17 Apr 2008, Remi Vanicat wrote: [...] > > $ ./t4014-format-patch.sh > > [....] > > * FAIL 8: replay did not screw up the log message > > Hmm. Doesn't fail for me. [...] > I wonder if it is some shell-specific variable expansion issue. What's > your /bin/sh? (I assume, from your email address, that you're using > Debian, and I thought Debian used bash, but maybe that's not true). Debian used bash, but I use dash as /bin/sh > What does > > a="hello\nhi" > echo $a > > result in? here is the problem: $/bin/sh $ a="hello\nhi" $ echo $a hello hi Mmm, I Wonder what posix said about this, but it look like a bug with dash ``` ## Linus Torvalds, 2008-04-17 15:44 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.00.0804170841270.2879%40woody.linux-foundation.org In-Reply-To: <20080417153421.GA13117@atjola.homenet> ``` On Thu, 17 Apr 2008, Björn Steinbrink wrote: > > Debian has its own dash which is supposed to be a lightweight > alternative to bash and "for checking POSIX compliance of scripts" > (quote from the packages description). I don't happen to know off-hand > whether POSIX says that echo should default to -e, but dash seems to do > that: > > $ dash > $ a="hello\nhi" > $ echo $a > hello > hi Ahh. So that "echo" should just be replaced with a 'printf "%s\n"' instead? We have a _lot_ of "echo"s though. I suspect the only ones we'd ever catch are the ones explicitly tested for. I suspect that the dash echo is just broken. Linus ``` ## Björn Steinbrink, 2008-04-17 16:03 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <20080417160327.GA13462@atjola.homenet> URL: https://gitlist.dev/e/20080417160327.GA13462%40atjola.homenet In-Reply-To: ``` On 2008.04.17 08:44:07 -0700, Linus Torvalds wrote: > > > On Thu, 17 Apr 2008, Björn Steinbrink wrote: > > > > Debian has its own dash which is supposed to be a lightweight > > alternative to bash and "for checking POSIX compliance of scripts" > > (quote from the packages description). I don't happen to know off-hand > > whether POSIX says that echo should default to -e, but dash seems to do > > that: > > > > $ dash > > $ a="hello\nhi" > > $ echo $a > > hello > > hi > > Ahh. So that "echo" should just be replaced with a 'printf "%s\n"' > instead? > > We have a _lot_ of "echo"s though. I suspect the only ones we'd ever catch > are the ones explicitly tested for. I suspect that the dash echo is just > broken. Wow, seems that "echo" in itself is "broken". According to POSIX[1] backslashes induce implementation defined behaviour, and there seem to be two historic versions of echo, one that knows about -n and one that interprets escape sequences. So echo is totally non-portable unless you avoid -n as well as backslashes. Ouch. The POSIX docs, as well as random Google results, indeed recommend to use printf instead. But that switch seems painful as well... Björn [1] http://www.opengroup.org/onlinepubs/009695399/utilities/echo.html ``` ## Jeff King, 2008-04-17 16:24 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <20080417162421.GA18318@sigill.intra.peff.net> URL: https://gitlist.dev/e/20080417162421.GA18318%40sigill.intra.peff.net In-Reply-To: ``` On Thu, Apr 17, 2008 at 08:44:07AM -0700, Linus Torvalds wrote: > Ahh. So that "echo" should just be replaced with a 'printf "%s\n"' > instead? > > We have a _lot_ of "echo"s though. I suspect the only ones we'd ever catch > are the ones explicitly tested for. I suspect that the dash echo is just > broken. As Bjorn mentioned, it's POSIX. Junio and I already hunted down quite a few of these, so I think we are OK in general, and this is just a new one that got introduced. -Peff ``` ## Jeff King, 2008-04-17 16:27 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <20080417162700.GB18318@sigill.intra.peff.net> URL: https://gitlist.dev/e/20080417162700.GB18318%40sigill.intra.peff.net In-Reply-To: <20080417162421.GA18318@sigill.intra.peff.net> ``` On Thu, Apr 17, 2008 at 12:24:22PM -0400, Jeff King wrote: > As Bjorn mentioned, it's POSIX. Junio and I already hunted down quite a > few of these, so I think we are OK in general, and this is just a new > one that got introduced. Oops, I meant to mention this thread: http://article.gmane.org/gmane.comp.version-control.git/48419 -Peff ``` ## Junio C Hamano, 2008-04-18 18:30 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <7vskxjj9rb.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vskxjj9rb.fsf%40gitster.siamese.dyndns.org In-Reply-To: <6b8a91420804170844t2776e8a7sf735e3f87a95298b@mail.gmail.com> ``` "Rémi Vanicat" writes: > 2008/4/17, Linus Torvalds : >> >> On Thu, 17 Apr 2008, Remi Vanicat wrote: > [...] >> > $ ./t4014-format-patch.sh >> > [....] >> > * FAIL 8: replay did not screw up the log message >> >> Hmm. Doesn't fail for me. > > [...] > >> I wonder if it is some shell-specific variable expansion issue. What's >> your /bin/sh? (I assume, from your email address, that you're using >> Debian, and I thought Debian used bash, but maybe that's not true). > > Debian used bash, but I use dash as /bin/sh > >> What does >> >> a="hello\nhi" >> echo $a >> >> result in? So perhaps this one is the troublemaker? git-am.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/git-am.sh b/git-am.sh index 646dc45..5b20bbd 100755 --- a/git-am.sh +++ b/git-am.sh @@ -330,7 +330,7 @@ do SUBJECT="$(sed -n '/^Subject/ s/Subject: //p' "$dotest/info")" case "$keep_subject" in -k) SUBJECT="[PATCH] $SUBJECT" ;; esac - (echo "$SUBJECT" ; echo ; cat "$dotest/msg") | + (printf '%s\n' "$SUBJECT" ; echo ; cat "$dotest/msg") | git stripspace > "$dotest/msg-clean" ;; esac ``` ## Linus Torvalds, 2008-04-18 18:37 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0804181135540.2779%40woody.linux-foundation.org In-Reply-To: <7vskxjj9rb.fsf@gitster.siamese.dyndns.org> ``` On Fri, 18 Apr 2008, Junio C Hamano wrote: > > - (echo "$SUBJECT" ; echo ; cat "$dotest/msg") | > + (printf '%s\n' "$SUBJECT" ; echo ; cat "$dotest/msg") | Well, get rid of the superfluous second 'echo' then too, replacing it with another '\n'. IOW: ( printf '%s\n\n' "$SUBJECT" ; cat "$dotest/msg" ) | instead. Linus ``` ## Rémi Vanicat, 2008-04-18 22:13 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <6b8a91420804181513m139baf06x41d8efeb7acd49a3@mail.gmail.com> URL: https://gitlist.dev/e/6b8a91420804181513m139baf06x41d8efeb7acd49a3%40mail.gmail.com In-Reply-To: ``` 2008/4/18, Linus Torvalds : > > > On Fri, 18 Apr 2008, Junio C Hamano wrote: > > > > - (echo "$SUBJECT" ; echo ; cat "$dotest/msg") | > > + (printf '%s\n' "$SUBJECT" ; echo ; cat "$dotest/msg") | > > > Well, get rid of the superfluous second 'echo' then too, replacing it > with another '\n'. IOW: > > ( printf '%s\n\n' "$SUBJECT" ; cat "$dotest/msg" ) | > > instead. This work for me: no more test failure on my computer ``` ## Junio C Hamano, 2008-04-19 06:51 Subject: Re: [BUGS] test failure in t4014-format-patch.sh with master Message-ID: <7vfxtiibhf.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfxtiibhf.fsf%40gitster.siamese.dyndns.org In-Reply-To: <6b8a91420804181513m139baf06x41d8efeb7acd49a3@mail.gmail.com> ``` "Rémi Vanicat" writes: > 2008/4/18, Linus Torvalds : >> >> >> On Fri, 18 Apr 2008, Junio C Hamano wrote: >> > >> > - (echo "$SUBJECT" ; echo ; cat "$dotest/msg") | >> > + (printf '%s\n' "$SUBJECT" ; echo ; cat "$dotest/msg") | >> >> >> Well, get rid of the superfluous second 'echo' then too, replacing it >> with another '\n'. IOW: >> >> ( printf '%s\n\n' "$SUBJECT" ; cat "$dotest/msg" ) | >> >> instead. > > This work for me: no more test failure on my computer Thanks for confirmation. I reproduced the breakage and the fix myself with "make SHELL_PATH=/bin/dash" myself, too. ```