Possible bug in "git rebase" (non-interactive) with regards to post-rewrite

Hi

Please advise, thank you.

-- 
Mihai Rusu

Junio C Hamano· Dec 23, 2010, 00:02 UTC · re: Mihai Rusu · lore

Re: Possible bug in "git rebase" (non-interactive) with regards to post-rewrite

Mihai Rusu <dizzy@google.com> writes:

> I have found a possible bug in Git. When running "git rebase"
> (non-interactively, ie not "git rebase -i") on code that would
> conflict on the last commit that is being rebased and if that last
> commit is being skipped (git rebase --skip) then after the rebase is
> done the "post-rewrite" hook is not called by "git rebase". If I get a
> conflict and "git rebase --skip" any other commit or if I use "git
> rebase -i" and "git rebase --skip" the last commit when it conflicts
> then it calls post-rewrite just fine. Because this hook is normally
> called only once, at the end of a non-aborted rebase the fact that
> "git rebase" does not call it when the last commit conflicts and is
> skipped means the script is not called at all for that rebase
> operation thus breaking the code that depends on it.
>
> Please advise, thank you.

Perhaps this?  Totally untested...

 git-am.sh |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index cf1f64b..6cdd591 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -577,13 +577,6 @@ then
 	resume=
 fi
 
-if test "$this" -gt "$last"
-then
-	say Nothing to do.
-	rm -fr "$dotest"
-	exit
-fi
-
 while test "$this" -le "$last"
 do
 	msgnum=`printf "%0${prec}d" $this`

Junio C Hamano· Dec 23, 2010, 00:49 UTC · re: Junio C Hamano · lore

Re: Possible bug in "git rebase" (non-interactive) with regards to post-rewrite

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-am.sh                    |    7 -------
 t/t5407-post-rewrite-hook.sh |   18 +++++++++++++++++-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index de116a2..69474e5 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -554,13 +554,6 @@ then
 	resume=
 fi
 
-if test "$this" -gt "$last"
-then
-	say Nothing to do.
-	rm -fr "$dotest"
-	exit
-fi
-
 while test "$this" -le "$last"
 do
 	msgnum=`printf "%0${prec}d" $this`
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index 552da65..baa670c 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -10,7 +10,11 @@ test_expect_success 'setup' '
 	test_commit A foo A &&
 	test_commit B foo B &&
 	test_commit C foo C &&
-	test_commit D foo D
+	test_commit D foo D &&
+	git checkout A^0 &&
+	test_commit E bar E &&
+	test_commit F foo F &&
+	git checkout master
 '
 
 mkdir .git/hooks
@@ -79,6 +83,18 @@ EOF
 	verify_hook_input
 '
 
+test_expect_success 'git rebase --skip the last one' '
+	git reset --hard F &&
+	clear_hook_input &&
+	test_must_fail git rebase --onto D A &&
+	git rebase --skip &&
+	echo rebase >expected.args &&
+	cat >expected.data <<EOF &&
+$(git rev-parse E) $(git rev-parse HEAD)
+EOF
+	verify_hook_input
+'
+
 test_expect_success 'git rebase -m' '
 	git reset --hard D &&
 	clear_hook_input &&

Thomas Rast· Dec 26, 2010, 10:38 UTC · re: Junio C Hamano · lore

Re: Possible bug in "git rebase" (non-interactive) with regards to post-rewrite

Junio C Hamano wrote:

> When "rebase --skip" is used to skip the last patch in the series, the
> code to wrap up the rewrite by copying the notes from old to new commits
> and also by running the post-rewrite hook was bypassed.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

[...]

> -if test "$this" -gt "$last"
> -then
> -	say Nothing to do.
> -	rm -fr "$dotest"
> -	exit
> -fi
> -
>  while test "$this" -le "$last"
>  do
>  	msgnum=`printf "%0${prec}d" $this`

Ack, thanks for patching this.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch