Re: [PATCH] test-lib.sh: use printf instead of echo
On Mar 15, Johannes Sixt wrote: - echo $@ | sed -e 's/^/# /' + printf '%s\n' $@ | sed -e 's/^/# /' This should be printf '%s\n' $* | sed -e 's/^/# /' Right, that should be $* to always be one argument for the format pattern. Thanks Uwe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] test-lib.sh: use printf instead of echo
when variables may contain backslash sequences.
Backslash sequences are interpreted as control characters
by the echo command of some shells (e.g. dash).
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
Changed $@ to $* in printf.
Thanks to Johannes Sixt to point that out.
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..3c7cb1d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error Test script did not set test_description.
if test $help = t
then
- echo $test_description
+ printf '%s\n' $test_description
exit 0
fi
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error not ok $test_count - $1
shift
- echo $@ | sed -e 's/^/# /'
+ printf '%s\n' $* | sed -e 's/^/# /'
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] rebase -i: replace an echo command by printf
On Mar 17, Junio C Hamano wrote: Will tentatively queue with the above rewrite, but if you feel strongly, please send an replacement. No need for a replacement, your wording is good. I couldn't do it better. I'll borrow your commit message for my other patch to fix the continued title there too. If you already have queued and rewritten that patch you may keep your version. Thanks Uwe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] test-lib.sh: do not echo externally supplied strings
In some places we echo a string that is supplied by the calling
test script and may contain backslash sequences. The echo command
of some shells, most notably dash, interprets these backslash
sequences (POSIX.1 allows this) which may scramble the test
output.
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
Commit message rewritten to avoid title continuation in the body.
Thanks Junio C Hamano for the hint.
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..3c7cb1d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error Test script did not set test_description.
if test $help = t
then
- echo $test_description
+ printf '%s\n' $test_description
exit 0
fi
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error not ok $test_count - $1
shift
- echo $@ | sed -e 's/^/# /'
+ printf '%s\n' $* | sed -e 's/^/# /'
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Corner case bug caused by shell dependent behavior
On Mar 13, Jonathan Nieder wrote: May we have your sign-off? (See Documentation/SubmittingPatches section Sign your work for what this means. I could have found that myself .. thanks! I'll try to follow it now. :) I'll resend the patch. Hopefully I'll do it right. Would it make sense to add this as a test to e.g. t/t3404-rebase-interactive.sh? It's a rather special case, so I'm not sure if it's worth it. I'll send a patch which adds a test for it. The test works for me, but as I don't understand the test mechanisms already good enough a few questions: - Is it correct to call the fake editor with an empty variable FAKE_LINES when you want it to not change the todo list of a rebase -i and use it as it is (the work is already done by the autosquash option)? I can achieve the same with EDITOR=true. What's the preferred way? Is there an advantage to use the fake editor also in this case? - The tests in t3404-rebase-interactive.sh use their variables a bit differently, some just set the variables, some export the variables and some use a subshell to encapsulate them. Also some of the tests reset their rebase state so that subsequent tests, which also use rebase, do not fail when the rebase fails. Other tests don't do that. What's the expected resp. recommended behavior? While trying to understand the test mechanisms I stumbled over two other problematic uses of echo. These although only affect the test output, not git itself. Uwe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] rebase -i: replace an echo command by printf
to avoid shell dependent behavior.
When your system shell (/bin/sh) is a dash backslash sequences
in strings are interpreted by the echo command. A commit message
which ends with the string '\n' may result in a garbage line in
the todo list of an interactive rebase which causes the rebase
to fail.
To reproduce the behavior (with dash as /bin/sh):
mkdir test cd test git init
echo 1 foo git add foo
git commit -mthis commit message ends with '\n'
echo 2 foo git commit -a --fixup HEAD
git rebase -i --autosquash --root
Now the editor opens with garbage in line 3 which has to be
removed or the rebase fails.
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43c19e0..43631b4 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -739,7 +739,7 @@ rearrange_squash () {
;;
esac
done
- echo $sha1 $action $prefix $rest
+ printf '%s %s %s %s\n' $sha1 $action $prefix
$rest
# if it's a single word, try to resolve to a full sha1
and
# emit a second copy. This allows us to match on both
message
# and on sha1 prefix
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test-lib.sh: use printf instead of echo
when variables may contain backslash sequences.
Backslash sequences are interpreted as control characters
by the echo command of some shells (e.g. dash).
Signed-off-by: Uwe Storbeck u...@ibr.ch
---
t/test-lib.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..8209204 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error Test script did not set test_description.
if test $help = t
then
- echo $test_description
+ printf '%s\n' $test_description
exit 0
fi
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error not ok $test_count - $1
shift
- echo $@ | sed -e 's/^/# /'
+ printf '%s\n' $@ | sed -e 's/^/# /'
test $immediate = || { GIT_EXIT_OK=t; exit 1; }
}
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] t3404: test autosquash for fixup! commits with funny messages
This commit adds a test to verify the correct behavior when rebase -i is used to autosquash fixup! commits where the commit message contains a backslash sequence (\n). When echo is used instead of printf to handle such a commit message the test will fail on shells (e.g. dash) where the echo command interprets backslash sequences as control characters. Signed-off-by: Uwe Storbeck u...@ibr.ch --- t/t3404-rebase-interactive.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 50e22b1..6d32661 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -798,6 +798,18 @@ test_expect_success 'rebase-i history with funny messages' ' test_cmp expect actual ' +test_expect_success 'autosquash fixup! commits with funny messages' ' + test_when_finished git rebase --abort || : + echo file1 + git commit -a -m something that looks like a newline (\n) + echo file1 + git commit -a --fixup HEAD + set_fake_editor + FAKE_LINES= + export FAKE_LINES + git rebase -i --autosquash HEAD~2 +' + test_expect_success 'prepare for rebase -i --exec' ' git checkout master -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Corner case bug caused by shell dependent behavior
Hi
When your system shell (/bin/sh) is a dash control sequences in
strings get interpreted by the echo command. A commit message
which ends with the string '\n' may result in a garbage line in
the todo list of an interactive rebase which causes the rebase
to fail.
To reproduce the behavior (with dash as /bin/sh):
mkdir test cd test git init
echo 1 foo git add foo
git commit -mthis commit message ends with '\n'
echo 2 foo git commit -a --fixup HEAD
git rebase -i --autosquash --root
Now the editor opens with garbage in line 3 which has to be
removed or the rebase fails.
The attached one-line patch fixes the bug. Be free to edit the
commit message when it's too long.
Maybe there are more places where it would be more robust to use
printf instead of echo.
Uwe
From 53262bc8a7a3ec9d9a6b0e8ecaaea598257b87fe Mon Sep 17 00:00:00 2001
From: Uwe Storbeck u...@ibr.ch
Date: Fri, 14 Mar 2014 00:28:33 +0100
Subject: [PATCH] git-rebase--interactive: replace echo by printf
to avoid shell dependent behavior.
When your system shell (/bin/sh) is a dash control sequences in
strings get interpreted by the echo command. A commit message
which ends with the string '\n' may result in a garbage line in
the todo list of an interactive rebase which causes the rebase
to fail.
To reproduce the behavior (with dash as /bin/sh):
mkdir test cd test git init
echo 1 foo git add foo
git commit -mthis commit message ends with '\n'
echo 2 foo git commit -a --fixup HEAD
git rebase -i --autosquash --root
Now the editor opens with garbage in line 3 which has to be
removed or the rebase fails.
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a1adae8..3ffe14c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -749,7 +749,7 @@ rearrange_squash () {
;;
esac
done
- echo $sha1 $action $prefix $rest
+ printf %s %s %s %s\n $sha1 $action $prefix $rest
# if it's a single word, try to resolve to a full sha1 and
# emit a second copy. This allows us to match on both message
# and on sha1 prefix
--
1.9.0
Asymmetric default behavior of git stash
Hi, is there any reason why the default behavior of git stash is asymmetric? When I save my current state with 'git stash save' it saves the worktree changes and the index changes (and resets both). When I restore the state with 'git stash pop' it restores the worktree changes, but not the state of the index. Your work preparing the index is lost. Although this behavior is documented it is kind of unexpected. From a save-restore mechanism I would expect that the default behavior would restore the state as it was before the save. So I would expect it to either save and restore the worktree and leave the index alone or save and restore both, the worktree and the index. Regards Uwe -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
