built-in rebase --autostash: fix regression

It was reported that the new, shiny built-in git rebase has a bug where it would detach the HEAD when it was not even necessary to detach it.

Keeping with my fine tradition to first demonstrate what is the actual problem (and making it easy to verify my claim), this patch series first introduces the regression test, and then the (quite simple) fix.

AEvar, sorry for the ASCII-fication of your name, I still did not find the time to look at the GitGitGadget bug closely where it does the wrong thing when Cc:ing with non-ASCII names.

Johannes Schindelin (2):
built-in rebase: demonstrate regression with --autostash built-in rebase --autostash: leave the current branch alone if
possible

 builtin/rebase.c            | 3 ++-
 t/t3420-rebase-autostash.sh | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-70%2Fdscho%2Ffix-built-in-rebase-autostash-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-70/dscho/fix-built-in-rebase-autostash-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/70

gitgitgadget

From: Johannes Schindelin <[email protected]>

An unnamed colleague of Ævar Arnfjörð Bjarmason reported a breakage where a pull --rebase (which did not really need to do anything but stash, see that nothing was changed, and apply the stash again) also detached the HEAD.

This patch adds a minimal reproducer for this regression.

---
 t/t3420-rebase-autostash.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

index f355c6825a..d4e2520bcb 100755
@@ -361,4 +361,12 @@ test_expect_success 'autostash with dirty submodules' '
 	git rebase -i --autostash HEAD
 '
 
+test_expect_failure 'branch is left alone when possible' '
+	git checkout -b unchanged-branch &&
+	echo changed >file0 &&
+	git rebase --autostash unchanged-branch &&
+	test changed = "$(cat file0)" &&
+	test unchanged-branch = "$(git rev-parse --abbrev-ref HEAD)"
+'
+
 test_done

gitgitgadget

From: Johannes Schindelin <[email protected]>

When we converted a git reset --hard call in the original Unix shell script to built-in code, we asked to reset the worktree and the index and explicitly *not* to detach the HEAD. By mistake, though, we still did. Let's fix this.

---
 builtin/rebase.c            | 3 ++-
 t/t3420-rebase-autostash.sh | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

index 0ee06aa363..4a608d0a78 100644
@@ -613,7 +613,8 @@ static int reset_head(struct object_id *oid, const char *action,
 		reflog_head = msg.buf;
 	}
 	if (!switch_to_branch)
-		ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF,
+		ret = update_ref(reflog_head, "HEAD", oid, orig,
+				 detach_head ? REF_NO_DEREF : 0,
 				 UPDATE_REFS_MSG_ON_ERR);
 	else {
 		ret = create_symref("HEAD", switch_to_branch, msg.buf);

index d4e2520bcb..4c7494cc8f 100755
@@ -361,7 +361,7 @@ test_expect_success 'autostash with dirty submodules' '
 	git rebase -i --autostash HEAD
 '
 
-test_expect_failure 'branch is left alone when possible' '
+test_expect_success 'branch is left alone when possible' '
 	git checkout -b unchanged-branch &&
 	echo changed >file0 &&
 	git rebase --autostash unchanged-branch &&

gitgitgadget