git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 10/10] rebase: dereference tags

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 21, 2021, 10:24 UTC
Message-ID
<55a6250ab38881ba56dfb6e65cf663bd2b631d5f.1632219848.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1033.v3.git.1632219848.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
Previous: Phillip Wood via GitGitGadgetNext: Elijah Newren
Message 54 of 55 in “rebase: dereference tags”
  1. 00/11 rebase: dereference tagsPhillip Wood via GitGitGadget, Sep 8, 2021
  2. 01/11 t3407: run tests in $TEST_DIRECTORYPhillip Wood via GitGitGadget, Sep 8, 2021
  3. Ævar Arnfjörð BjarmasonSep 8, 2021
  4. 02/11 t3407: use test_commitPhillip Wood via GitGitGadget, Sep 8, 2021
  5. Ævar Arnfjörð BjarmasonSep 8, 2021
  6. 03/11 t3407: use test_cmp_revPhillip Wood via GitGitGadget, Sep 8, 2021
  7. Ævar Arnfjörð BjarmasonSep 8, 2021
  8. Phillip WoodSep 8, 2021
  9. 04/11 t3407: rename a variablePhillip Wood via GitGitGadget, Sep 8, 2021
  10. 06/11 t3407: strengthen rebase --abort testsPhillip Wood via GitGitGadget, Sep 8, 2021
  11. Ævar Arnfjörð BjarmasonSep 8, 2021
  12. 05/11 t3407: use test_path_is_missingPhillip Wood via GitGitGadget, Sep 8, 2021
  13. 07/11 t3407: rework rebase --quit testsPhillip Wood via GitGitGadget, Sep 8, 2021
  14. 08/11 rebase: remove redundant strbufPhillip Wood via GitGitGadget, Sep 8, 2021
  15. Johannes SchindelinSep 9, 2021
  16. 09/11 rebase: use our standard error return valuePhillip Wood via GitGitGadget, Sep 8, 2021
  17. 11/11 rebase: dereference tagsPhillip Wood via GitGitGadget, Sep 8, 2021
  18. Ævar Arnfjörð BjarmasonSep 8, 2021
  19. 10/11 rebase: use lookup_commit_reference_by_name()Phillip Wood via GitGitGadget, Sep 8, 2021
  20. 00/11 rebase: dereference tagsPhillip Wood via GitGitGadget, Sep 13, 2021
  21. 01/11 t3407: run tests in $TEST_DIRECTORYPhillip Wood via GitGitGadget, Sep 13, 2021
  22. 02/11 t3407: use test_commitPhillip Wood via GitGitGadget, Sep 13, 2021
  23. 03/11 t3407: use test_cmp_revPhillip Wood via GitGitGadget, Sep 13, 2021
  24. 04/11 t3407: rename a variablePhillip Wood via GitGitGadget, Sep 13, 2021
  25. 05/11 t3407: use test_path_is_missingPhillip Wood via GitGitGadget, Sep 13, 2021
  26. 06/11 t3407: strengthen rebase --abort testsPhillip Wood via GitGitGadget, Sep 13, 2021
  27. 07/11 t3407: rework rebase --quit testsPhillip Wood via GitGitGadget, Sep 13, 2021
  28. 08/11 rebase: remove redundant strbufPhillip Wood via GitGitGadget, Sep 13, 2021
  29. René ScharfeSep 13, 2021
  30. Junio C HamanoSep 13, 2021
  31. Phillip WoodSep 14, 2021
  32. Phillip WoodSep 14, 2021
  33. 09/11 rebase: use our standard error return valuePhillip Wood via GitGitGadget, Sep 13, 2021
  34. 11/11 rebase: dereference tagsPhillip Wood via GitGitGadget, Sep 13, 2021
  35. Junio C HamanoSep 13, 2021
  36. Phillip WoodSep 14, 2021
  37. Phillip WoodSep 14, 2021
  38. Junio C HamanoSep 14, 2021
  39. Elijah NewrenSep 14, 2021
  40. Sergey OrganovSep 14, 2021
  41. Phillip WoodSep 14, 2021
  42. 10/11 rebase: use lookup_commit_reference_by_name()Phillip Wood via GitGitGadget, Sep 13, 2021
  43. Elijah NewrenSep 14, 2021
  44. 00/10 rebase: dereference tagsPhillip Wood via GitGitGadget, Sep 21, 2021
  45. 01/10 t3407: run tests in $TEST_DIRECTORYPhillip Wood via GitGitGadget, Sep 21, 2021
  46. 02/10 t3407: use test_commitPhillip Wood via GitGitGadget, Sep 21, 2021
  47. 03/10 t3407: use test_cmp_revPhillip Wood via GitGitGadget, Sep 21, 2021
  48. 04/10 t3407: rename a variablePhillip Wood via GitGitGadget, Sep 21, 2021
  49. 05/10 t3407: use test_path_is_missingPhillip Wood via GitGitGadget, Sep 21, 2021
  50. 06/10 t3407: strengthen rebase --abort testsPhillip Wood via GitGitGadget, Sep 21, 2021
  51. 07/10 t3407: rework rebase --quit testsPhillip Wood via GitGitGadget, Sep 21, 2021
  52. 08/10 rebase: use our standard error return valuePhillip Wood via GitGitGadget, Sep 21, 2021
  53. 09/10 rebase: use lookup_commit_reference_by_name()Phillip Wood via GitGitGadget, Sep 21, 2021
  54. 10/10 rebase: dereference tagsPhillip Wood via GitGitGadget, Sep 21, 2021
  55. Elijah NewrenSep 24, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.