[PATCH v3 10/10] rebase: dereference tags

["Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

A rebase started with 'git rebase <A> <B>' is conceptually to first
checkout <B> and run 'git rebase <A>' starting from that state.  'git
rebase --abort' in the middle of such a rebase should take us back to
the state we checked out <B>.

This used to work, even when <B> is a tag that points at a commit,
until Git 2.20.0 when the command was reimplemented in C.  The command
now complains that the tag object itself cannot be checked out, which
may be technically correct but is not what the user asked to do.

Fix this old regression by using lookup_commit_reference_by_name()
when parsing <B>. The scripted version did not need to peel the tag
because the commands it passed the tag to (e.g 'git reset') peeled the
tag themselves.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
---
 builtin/rebase.c        | 14 ++++++++------
 t/t3407-rebase-abort.sh | 18 ++++++++++++++----
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ace9e0a8ec..b4433ee7978 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1904,13 +1904,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			die_if_checked_out(buf.buf, 1);
 			options.head_name = xstrdup(buf.buf);
 		/* If not is it a valid ref (branch or commit)? */
-		} else if (!get_oid(branch_name, &options.orig_head) &&
-			   lookup_commit_reference(the_repository,
-						   &options.orig_head))
+		} else {
+			struct commit *commit =
+				lookup_commit_reference_by_name(branch_name);
+			if (!commit)
+				die(_("no such branch/commit '%s'"),
+				    branch_name);
+			oidcpy(&options.orig_head, &commit->object.oid);
 			options.head_name = NULL;
-		else
-			die(_("no such branch/commit '%s'"),
-			    branch_name);
+		}
 	} else if (argc == 0) {
 		/* Do not need to switch branches, we are already on it. */
 		options.head_name =
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index 162112ba5ea..ebbaed147a6 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -11,18 +11,18 @@ test_expect_success setup '
 	test_commit a a a &&
 	git branch to-rebase &&
 
-	test_commit b a b &&
-	test_commit c a c &&
+	test_commit --annotate b a b &&
+	test_commit --annotate c a c &&
 
 	git checkout to-rebase &&
 	test_commit "merge should fail on this" a d d &&
-	test_commit "merge should fail on this, too" a e pre-rebase
+	test_commit --annotate "merge should fail on this, too" a e pre-rebase
 '
 
 # Check that HEAD is equal to "pre-rebase" and the current branch is
 # "to-rebase"
 check_head() {
-	test_cmp_rev HEAD pre-rebase &&
+	test_cmp_rev HEAD pre-rebase^{commit} &&
 	test "$(git symbolic-ref HEAD)" = refs/heads/to-rebase
 }
 
@@ -67,6 +67,16 @@ testrebase() {
 		test_path_is_missing "$state_dir"
 	'
 
+	test_expect_success "rebase$type --abort when checking out a tag" '
+		test_when_finished "git symbolic-ref HEAD refs/heads/to-rebase" &&
+		git reset --hard a -- &&
+		test_must_fail git rebase$type --onto b c pre-rebase &&
+		test_cmp_rev HEAD b^{commit} &&
+		git rebase --abort &&
+		test_cmp_rev HEAD pre-rebase^{commit} &&
+		! git symbolic-ref HEAD
+	'
+
 	test_expect_success "rebase$type --abort does not update reflog" '
 		# Clean up the state from the previous one
 		git reset --hard pre-rebase &&
-- 
gitgitgadget