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

[PATCH v3 4/8] rebase: store orig_head as a commit

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Oct 13, 2022, 08:42 UTC
Message-ID
<dc056b13ed5c62729f97bd2cedbe769c531f4bc4.1665650564.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1323.v3.git.1665650564.gitgitgadget@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Using a struct commit rather than a struct oid to hold orig_head means that we error out straight away if the branch being rebased does not point to a commit. It also simplifies the code that handles finding the merge base and fork point as it no longer has to convert from an oid to a commit.

To avoid changing the behavior of "git rebase <upstream> <branch>" we keep the existing call to read_ref() and use lookup_commit_reference() on the oid returned by that rather than calling lookup_commit_reference_by_name() which applies the ref dwim rules to its argument. lookup_commit_reference() will dereference tag objects but we do not expect the branch being rebased to be pointing to a tag object.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/rebase.c | 67 ++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 76f83a42f49..7e6ce374c59 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -68,7 +68,7 @@ struct rebase_options {
 	const char *upstream_name;
 	const char *upstream_arg;
 	char *head_name;
-	struct object_id orig_head;
+	struct commit *orig_head;
 	struct commit *onto;
 	const char *onto_name;
 	const char *revisions;
@@ -261,13 +261,13 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
 	struct replay_opts replay = get_replay_opts(opts);
 	struct string_list commands = STRING_LIST_INIT_DUP;
 
-	if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head,
+	if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head->object.oid,
 				&revisions, &shortrevisions))
 		return -1;
 
 	if (init_basic_state(&replay,
 			     opts->head_name ? opts->head_name : "detached HEAD",
-			     opts->onto, &opts->orig_head)) {
+			     opts->onto, &opts->orig_head->object.oid)) {
 		free(revisions);
 		free(shortrevisions);
 
@@ -298,9 +298,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
 		split_exec_commands(opts->cmd, &commands);
 		ret = complete_action(the_repository, &replay, flags,
 			shortrevisions, opts->onto_name, opts->onto,
-			&opts->orig_head, &commands, opts->autosquash,
-			opts->update_refs,
-			&todo_list);
+			&opts->orig_head->object.oid, &commands,
+			opts->autosquash, opts->update_refs, &todo_list);
 	}
 
 	string_list_clear(&commands, 0);
@@ -448,7 +447,8 @@ static int read_basic_state(struct rebase_options *opts)
 	} else if (!read_oneliner(&buf, state_dir_path("head", opts),
 				  READ_ONELINER_WARN_MISSING))
 		return -1;
-	if (get_oid_hex(buf.buf, &opts->orig_head))
+	if (get_oid_hex(buf.buf, &oid) ||
+	    !(opts->orig_head = lookup_commit_reference(the_repository, &oid)))
 		return error(_("invalid orig-head: '%s'"), buf.buf);
 
 	if (file_exists(state_dir_path("quiet", opts)))
@@ -517,7 +517,7 @@ static int rebase_write_basic_state(struct rebase_options *opts)
 	write_file(state_dir_path("onto", opts), "%s",
 		   opts->onto ? oid_to_hex(&opts->onto->object.oid) : "");
 	write_file(state_dir_path("orig-head", opts), "%s",
-		   oid_to_hex(&opts->orig_head));
+		   oid_to_hex(&opts->orig_head->object.oid));
 	if (!(opts->flags & REBASE_NO_QUIET))
 		write_file(state_dir_path("quiet", opts), "%s", "");
 	if (opts->flags & REBASE_VERBOSE)
@@ -646,7 +646,7 @@ static int run_am(struct rebase_options *opts)
 			       /* this is now equivalent to !opts->upstream */
 			       &opts->onto->object.oid :
 			       &opts->upstream->object.oid),
-		    oid_to_hex(&opts->orig_head));
+		    oid_to_hex(&opts->orig_head->object.oid));
 
 	rebased_patches = xstrdup(git_path("rebased-patches"));
 	format_patch.out = open(rebased_patches,
@@ -680,7 +680,7 @@ static int run_am(struct rebase_options *opts)
 		free(rebased_patches);
 		strvec_clear(&am.args);
 
-		ropts.oid = &opts->orig_head;
+		ropts.oid = &opts->orig_head->object.oid;
 		ropts.branch = opts->head_name;
 		ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
 		reset_head(the_repository, &ropts);
@@ -833,7 +833,7 @@ static int checkout_up_to_date(struct rebase_options *options)
 	strbuf_addf(&buf, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
 		    options->switch_to);
-	ropts.oid = &options->orig_head;
+	ropts.oid = &options->orig_head->object.oid;
 	ropts.branch = options->head_name;
 	ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
 	if (!ropts.branch)
@@ -866,15 +866,11 @@ static int is_linear_history(struct commit *from, struct commit *to)
 
 static int can_fast_forward(struct commit *onto, struct commit *upstream,
 			    struct commit *restrict_revision,
-			    struct object_id *head_oid, struct object_id *merge_base)
+			    struct commit *head, struct object_id *merge_base)
 {
-	struct commit *head = lookup_commit(the_repository, head_oid);
 	struct commit_list *merge_bases = NULL;
 	int res = 0;
 
-	if (!head)
-		goto done;
-
 	merge_bases = get_merge_bases(onto, head);
 	if (!merge_bases || merge_bases->next) {
 		oidcpy(merge_base, null_oid());
@@ -1312,13 +1308,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 		if (read_basic_state(&options))
 			exit(1);
-		ropts.oid = &options.orig_head;
+		ropts.oid = &options.orig_head->object.oid;
 		ropts.branch = options.head_name;
 		ropts.flags = RESET_HEAD_HARD;
 		ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
 		if (reset_head(the_repository, &ropts) < 0)
 			die(_("could not move back to %s"),
-			    oid_to_hex(&options.orig_head));
+			    oid_to_hex(&options.orig_head->object.oid));
 		remove_branch_state(the_repository, 0);
 		ret = finish_rebase(&options);
 		goto cleanup;
@@ -1604,25 +1600,27 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	 */
 	if (argc == 1) {
 		/* Is it "rebase other branchname" or "rebase other commit"? */
+		struct object_id branch_oid;
 		branch_name = argv[0];
 		options.switch_to = argv[0];
 
 		/* Is it a local branch? */
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "refs/heads/%s", branch_name);
-		if (!read_ref(buf.buf, &options.orig_head)) {
+		if (!read_ref(buf.buf, &branch_oid)) {
 			die_if_checked_out(buf.buf, 1);
 			options.head_name = xstrdup(buf.buf);
+			options.orig_head =
+				lookup_commit_reference(the_repository,
+							&branch_oid);
 		/* If not is it a valid ref (branch or commit)? */
 		} else {
-			struct commit *commit =
+			options.orig_head =
 				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;
 		}
+		if (!options.orig_head)
+			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 =
@@ -1639,8 +1637,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			FREE_AND_NULL(options.head_name);
 			branch_name = "HEAD";
 		}
-		if (get_oid("HEAD", &options.orig_head))
-			die(_("Could not resolve HEAD to a revision"));
+		options.orig_head = lookup_commit_reference_by_name("HEAD");
+		if (!options.orig_head)
+			die(_("Could not resolve HEAD to a commit"));
 	} else
 		BUG("unexpected number of arguments left to parse");
 
@@ -1672,13 +1671,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 				options.onto_name);
 	}
 
-	if (options.fork_point > 0) {
-		struct commit *head =
-			lookup_commit_reference(the_repository,
-						&options.orig_head);
+	if (options.fork_point > 0)
 		options.restrict_revision =
-			get_fork_point(options.upstream_name, head);
-	}
+			get_fork_point(options.upstream_name, options.orig_head);
 
 	if (repo_read_index(the_repository) < 0)
 		die(_("could not read index"));
@@ -1708,7 +1703,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	 * call it before checking allow_preemptive_ff.
 	 */
 	if (can_fast_forward(options.onto, options.upstream, options.restrict_revision,
-		    &options.orig_head, &merge_base) &&
+		    options.orig_head, &merge_base) &&
 	    allow_preemptive_ff) {
 		int flag;
 
@@ -1785,7 +1780,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&msg, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
 	ropts.oid = &options.onto->object.oid;
-	ropts.orig_head = &options.orig_head,
+	ropts.orig_head = &options.orig_head->object.oid,
 	ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
 			RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
 	ropts.head_msg = msg.buf;
@@ -1799,7 +1794,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	 * we just fast-forwarded.
 	 */
 	strbuf_reset(&msg);
-	if (oideq(&merge_base, &options.orig_head)) {
+	if (oideq(&merge_base, &options.orig_head->object.oid)) {
 		printf(_("Fast-forwarded %s to %s.\n"),
 			branch_name, options.onto_name);
 		strbuf_addf(&msg, "rebase finished: %s onto %s",
@@ -1820,7 +1815,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		    (options.restrict_revision ?
 		     oid_to_hex(&options.restrict_revision->object.oid) :
 		     oid_to_hex(&options.upstream->object.oid)),
-		    oid_to_hex(&options.orig_head));
+		    oid_to_hex(&options.orig_head->object.oid));
 
 	options.revisions = revisions.buf;
 

-- 
gitgitgadget
Previous: Junio C HamanoNext: Junio C Hamano
Message 55 of 82 in “rebase --keep-base: imply --reapply-cherry-picks and --no-fork-point”
  1. 0/5 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Aug 15, 2022
  2. 1/5 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Aug 15, 2022
  3. Junio C HamanoAug 15, 2022
  4. Phillip WoodAug 16, 2022
  5. Jonathan TanAug 24, 2022
  6. Phillip WoodAug 30, 2022
  7. 2/5 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Aug 15, 2022
  8. Junio C HamanoAug 15, 2022
  9. Johannes SchindelinAug 16, 2022
  10. Elijah NewrenAug 18, 2022
  11. 3/5 rebase: factor out merge_base calculationPhillip Wood via GitGitGadget, Aug 15, 2022
  12. Junio C HamanoAug 15, 2022
  13. Johannes SchindelinAug 16, 2022
  14. Junio C HamanoAug 16, 2022
  15. Phillip WoodAug 16, 2022
  16. Junio C HamanoAug 16, 2022
  17. Elijah NewrenAug 18, 2022
  18. Jonathan TanAug 24, 2022
  19. Phillip WoodAug 30, 2022
  20. 4/5 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Aug 15, 2022
  21. Junio C HamanoAug 15, 2022
  22. Jonathan TanAug 24, 2022
  23. Phillip WoodAug 30, 2022
  24. Philippe BlainAug 25, 2022
  25. Phillip WoodSep 5, 2022
  26. 5/5 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Aug 15, 2022
  27. Junio C HamanoAug 15, 2022
  28. Jonathan TanAug 24, 2022
  29. Phillip WoodSep 5, 2022
  30. Johannes SchindelinAug 16, 2022
  31. Jonathan TanAug 24, 2022
  32. 0/7 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Sep 7, 2022
  33. 1/7 t3416: tighten two testsPhillip Wood via GitGitGadget, Sep 7, 2022
  34. Junio C HamanoSep 7, 2022
  35. 2/7 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Sep 7, 2022
  36. Junio C HamanoSep 7, 2022
  37. 3/7 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Sep 7, 2022
  38. Junio C HamanoSep 7, 2022
  39. Phillip WoodSep 8, 2022
  40. 5/7 rebase: factor out branch_base calculationPhillip Wood via GitGitGadget, Sep 7, 2022
  41. Junio C HamanoSep 7, 2022
  42. 4/7 rebase: rename merge_base to branch_basePhillip Wood via GitGitGadget, Sep 7, 2022
  43. Junio C HamanoSep 7, 2022
  44. 6/7 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Sep 7, 2022
  45. 7/7 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Sep 7, 2022
  46. Denton LiuSep 8, 2022
  47. Phillip WoodSep 8, 2022
  48. 0/8 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Oct 13, 2022
  49. 1/8 t3416: tighten two testsPhillip Wood via GitGitGadget, Oct 13, 2022
  50. 2/8 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Oct 13, 2022
  51. 3/8 rebase: be stricter when reading state files containing oidsPhillip Wood via GitGitGadget, Oct 13, 2022
  52. Junio C HamanoOct 13, 2022
  53. Ævar Arnfjörð BjarmasonOct 13, 2022
  54. Junio C HamanoOct 13, 2022
  55. 4/8 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Oct 13, 2022
  56. Junio C HamanoOct 13, 2022
  57. Phillip WoodOct 13, 2022
  58. Junio C HamanoOct 13, 2022
  59. 6/8 rebase: factor out branch_base calculationPhillip Wood via GitGitGadget, Oct 13, 2022
  60. Ævar Arnfjörð BjarmasonOct 13, 2022
  61. Phillip WoodOct 17, 2022
  62. Ævar Arnfjörð BjarmasonOct 17, 2022
  63. 5/8 rebase: rename merge_base to branch_basePhillip Wood via GitGitGadget, Oct 13, 2022
  64. Ævar Arnfjörð BjarmasonOct 13, 2022
  65. Phillip WoodOct 17, 2022
  66. Ævar Arnfjörð BjarmasonOct 17, 2022
  67. Phillip WoodOct 17, 2022
  68. Ævar Arnfjörð BjarmasonOct 17, 2022
  69. Phillip WoodOct 19, 2022
  70. 7/8 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Oct 13, 2022
  71. 8/8 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Oct 13, 2022
  72. 0/8 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Oct 17, 2022
  73. 1/8 t3416: tighten two testsPhillip Wood via GitGitGadget, Oct 17, 2022
  74. 2/8 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Oct 17, 2022
  75. 3/8 rebase: be stricter when reading state files containing oidsPhillip Wood via GitGitGadget, Oct 17, 2022
  76. Junio C HamanoOct 17, 2022
  77. Phillip WoodOct 19, 2022
  78. 4/8 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Oct 17, 2022
  79. 6/8 rebase: factor out branch_base calculationPhillip Wood via GitGitGadget, Oct 17, 2022
  80. 5/8 rebase: rename merge_base to branch_basePhillip Wood via GitGitGadget, Oct 17, 2022
  81. 7/8 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Oct 17, 2022
  82. 8/8 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Oct 17, 2022

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.