Re: git rebase -i error message interprets \t in commit message

David Kastrup [off-list ref] writes:

Matthieu Moy [off-list ref] writes:

quoted

quoted

From 7962ac8d8f2cbc556f669fd97487f9d70edc4ea1 Mon Sep 17 00:00:00 2001

From: Matthieu Moy <redacted>
Date: Tue, 6 Aug 2013 19:13:03 +0200
Subject: [PATCH] die_with_status: use "printf '%s\n'", not "echo"

At least GNU echo interprets backslashes in its arguments.

I think that's incorrect in several respects.  For one thing, echo is
never called for most Bourne shells since echo is a builtin (might have
been different for UNIX version 7 or so).  For another, GNU echo would
behave like Bash:

And GNU Bash does not interpret escapes unless you do echo -e.  Escape
sequence interpretation, however, happens for Dash:

dak@lola:/usr/local/tmp/lilypond$ dash -c 'echo "x\tx"'
x	x
dak@lola:/usr/local/tmp/lilypond$ bash -c 'echo "x\tx"'
x\tx
dak@lola:/usr/local/tmp/lilypond$ /bin/echo "x\tx"
x\tx

So replace "GNU echo" in your commit message with "Dash's echo builtin"
and you get closer.

I'll queue the attached.

POSIX makes it an implementation defined behaviour when the first
argument is "-n", or any argument contains a backslas (X/Open System
Interfaces wants to treat "-n" as literal and always interpret the
backslash sequence), so it is definitely safer to avoid running
'echo' on any random string.

Thanks.

Author: Matthieu Moy [off-list ref]
Date:   Tue Aug 6 20:26:44 2013 +0200

    die_with_status: use "printf '%s\n'", not "echo"
    
    Some implementations of 'echo' (e.g. dash's built-in) interpret
    backslash sequences in their arguments.
    
    This triggered at least one bug: the error message of "rebase -i" was
    turning \t in commit messages into actual tabulations. There may be
    others.
    
    Using "printf '%s\n'" instead avoids this bad behavior, and is the form
    used by the "say" function.
    
    Noticed-by: David Kastrup [off-list ref]
    Signed-off-by: Matthieu Moy [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..e15be51 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh

@@ -53,7 +53,7 @@ die () {
 die_with_status () {
 	status=$1
 	shift
-	echo >&2 "$*"
+	printf >&2 '%s\n' "$*"
 	exit "$status"
 }
 

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 49ccb38..074deb1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh

@@ -976,4 +976,17 @@ test_expect_success 'rebase -i with --strategy and -X' '
 	test $(cat file1) = Z
 '
 
+test_expect_success 'rebase -i error on commits with \ in message' '
+	current_head=$(git rev-parse HEAD)
+	test_when_finished "git rebase --abort; git reset --hard $current_head; rm -f error" &&
+	test_commit TO-REMOVE will-conflict old-content &&
+	test_commit "\temp" will-conflict new-content dummy &&
+	(
+	EDITOR=true &&
+	export EDITOR &&
+	test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error
+	) &&
+	grep -v "	" error
+'
+
 test_done