Bug in git rebase --continue in v1.8.4

Bug in git rebase --continue in v1.8.4

[Christoph Mallon]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

Hi,

if I run rebase --continue (e.g. after a conflict resolution), then the rebase always ends with this error message:
	It seems that there is already a rebase-apply directory, and
	I wonder if you are in the middle of another rebase.  If that is the
	case, please try
        	git rebase (--continue | --abort | --skip)
	If that is not the case, please
        	rm -fr "/home/tron/gitRebaseTest/test/.git/rebase-apply"
	and run me again.  I am stopping in case you still have something
	valuable there.

This happens on git v1.8.4 on FreeBSD. It is fine with v1.8.3.4. It seems to be caused by a1549e1049439386b9fd643fae236ad3ba649650, specifically this hunk:
	--- a/git-rebase--am.sh
	+++ b/git-rebase--am.sh
	@@ -7,12 +7,12 @@ case "$action" in
	 continue)
	  git am --resolved --resolvemsg="$resolvemsg" &&
	  move_to_original_branch
	- exit
	+ return
	  ;;
	 skip)
	  git am --skip --resolvemsg="$resolvemsg" &&

Attached is a test script for this problem.

Regards
	Christoph

[-- Attachment #2: gitRebaseTest --]
[-- Type: text/plain, Size: 274 bytes --]

#! /bin/sh
set -eux

git init test
cd test

echo a > file
git add .
git commit -m a

git branch -t test

echo b > file
git add .
git commit -m b

git checkout test
echo c > file
git add .
git commit -m c

! git rebase
git checkout --theirs .
git add .
git rebase --continue

Re: Bug in git rebase --continue in v1.8.4

[Andriy Gapon]

Christoph Mallon said:
> if I run rebase --continue (e.g. after a conflict resolution), then the rebase always ends with this error message:
> 	It seems that there is already a rebase-apply directory, and
> 	I wonder if you are in the middle of another rebase.  If that is the
> 	case, please try
>         	git rebase (--continue | --abort | --skip)
> 	If that is not the case, please
>         	rm -fr "/home/tron/gitRebaseTest/test/.git/rebase-apply"
> 	and run me again.  I am stopping in case you still have something
> 	valuable there.
> 
> This happens on git v1.8.4 on FreeBSD. It is fine with v1.8.3.4.

I observe exactly the same problem.
I also use FreeBSD and the problem started with 1.8.4.

Judging by the lack of followups, could this be a FreeBSD-specific problem?
Any thoughts / suggestions?
Thank you!

> It seems to be caused by
> a1549e1049439386b9fd643fae236ad3ba649650, specifically this hunk:
> 	--- a/git-rebase--am.sh
> 	+++ b/git-rebase--am.sh
> 	 <at>  <at>  -7,12 +7,12  <at>  <at>  case "$action" in
> 	 continue)
> 	  git am --resolved --resolvemsg="$resolvemsg" &&
> 	  move_to_original_branch
> 	- exit
> 	+ return
> 	  ;;
> 	 skip)
> 	  git am --skip --resolvemsg="$resolvemsg" &&

-- 
Andriy Gapon

Re: Bug in git rebase --continue in v1.8.4

[Matthieu Moy]

[ Cc-ing Ram, as he is the author of the possibly guilty commit. ]

Andriy Gapon <avg@FreeBSD.org> writes:

> Christoph Mallon said:
>> if I run rebase --continue (e.g. after a conflict resolution), then the rebase always ends with this error message:
>> 	It seems that there is already a rebase-apply directory, and
>> 	I wonder if you are in the middle of another rebase.  If that is the
>> 	case, please try
>>         	git rebase (--continue | --abort | --skip)
>> 	If that is not the case, please
>>         	rm -fr "/home/tron/gitRebaseTest/test/.git/rebase-apply"
>> 	and run me again.  I am stopping in case you still have something
>> 	valuable there.
>> 
>> This happens on git v1.8.4 on FreeBSD. It is fine with v1.8.3.4.
>
> I observe exactly the same problem.
> I also use FreeBSD and the problem started with 1.8.4.
>
> Judging by the lack of followups, could this be a FreeBSD-specific problem?

I can't reproduce here (Debian GNU/Linux). Do the testsuite pass for
you?

If not, can you write a failing test? A minimalist script outside the
testsuite may help too if you're not familiar with Git's testsuite.

> Any thoughts / suggestions?
> Thank you!
>
>> It seems to be caused by
>> a1549e1049439386b9fd643fae236ad3ba649650, specifically this hunk:
>> 	--- a/git-rebase--am.sh
>> 	+++ b/git-rebase--am.sh
>> 	 <at>  <at>  -7,12 +7,12  <at>  <at>  case "$action" in
>> 	 continue)
>> 	  git am --resolved --resolvemsg="$resolvemsg" &&
>> 	  move_to_original_branch
>> 	- exit
>> 	+ return
>> 	  ;;
>> 	 skip)
>> 	  git am --skip --resolvemsg="$resolvemsg" &&

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Bug in git rebase --continue in v1.8.4

[Andriy Gapon]

on 04/09/2013 11:18 Matthieu Moy said the following:
> [ Cc-ing Ram, as he is the author of the possibly guilty commit. ]
> 
> Andriy Gapon <avg@FreeBSD.org> writes:
>> Judging by the lack of followups, could this be a FreeBSD-specific problem?
> 
> I can't reproduce here (Debian GNU/Linux). Do the testsuite pass for
> you?
> 
> If not, can you write a failing test? A minimalist script outside the
> testsuite may help too if you're not familiar with Git's testsuite.

Thank you for the suggestion.
I've just tried it and it failed here:

*** t3403-rebase-skip.sh ***
ok 1 - setup
ok 2 - rebase with git am -3 (default)
ok 3 - rebase --skip can not be used with other options
not ok 4 - rebase --skip with am -3
#
#               git rebase --skip
#
not ok 5 - rebase moves back to skip-reference
#
#               test refs/heads/skip-reference = $(git symbolic-ref HEAD) &&
#               git branch post-rebase &&
#               git reset --hard pre-rebase &&
#               test_must_fail git rebase master &&
#               echo "hello" > hello &&
#               git add hello &&
#               git rebase --continue &&
#               test refs/heads/skip-reference = $(git symbolic-ref HEAD) &&
#               git reset --hard post-rebase
#
ok 6 - checkout skip-merge
ok 7 - rebase with --merge
not ok 8 - rebase --skip with --merge
#
#               git rebase --skip
#
ok 9 - merge and reference trees equal
not ok 10 - moved back to branch correctly
#
#               test refs/heads/skip-merge = $(git symbolic-ref HEAD)
#
# failed 4 among 10 test(s)


Christoph has also suggested a small test script in his original email that
started this thread:
http://thread.gmane.org/gmane.comp.version-control.git/233516

-- 
Andriy Gapon

Re: Bug in git rebase --continue in v1.8.4

[Andriy Gapon]

This looks like bug in FreeBSD shell indeed.
If a script is sourced within a function scope, then 'return' in the script
causes the function to return.  But it should cause the sourced script to return.

I'll see what we can do about this.
Sorry for distracting you.
And thank you for the help!
-- 
Andriy Gapon

Re: Bug in git rebase --continue in v1.8.4

[Matthieu Moy]

Andriy Gapon <avg@FreeBSD.org> writes:

> This looks like bug in FreeBSD shell indeed.

Yes: the testsuite would have caught it otherwise.

> If a script is sourced within a function scope, then 'return' in the script
> causes the function to return.  But it should cause the sourced script to return.

OK, so this is FreeBSD's fault. But that should not prevent us from
fixing Git to make it work with FreeBSD.

Does the patch below fix the testsuite for you?

--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -167,13 +167,22 @@ You can run "git stash pop" or "git stash drop" at any time.
        rm -rf "$state_dir"
 }
 
-run_specific_rebase () {
+run_specific_rebase_internal () {
        if [ "$interactive_rebase" = implied ]; then
                GIT_EDITOR=:
                export GIT_EDITOR
                autosquash=
        fi
+       # On FreeBSD, the shell's "return" returns from the current
+       # function, not from the current file inclusion.
+       # run_specific_rebase_internal has the file inclusion as a
+       # last statement, so POSIX and FreeBSD's return will do the
+       # same thing.
        . git-rebase--$type
+}
+
+run_specific_rebase () {
+       run_specific_rebase_internal
        ret=$?
        if test $ret -eq 0
        then


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/