diff mbox series

[10/11] rebase --apply: set ORIG_HEAD correctly

Message ID e8884efcc83960fc0ea5d5aee72bb9839b4be397.1633082702.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series rebase: reset_head() related fixes and improvements | expand

Commit Message

Phillip Wood Oct. 1, 2021, 10:05 a.m. UTC 
From: Phillip Wood <phillip.wood@dunelm.org.uk>

At the start of a rebase ORIG_HEAD is updated to tip of the branch
being rebased. Unfortunately reset_head() always uses the current
value of HEAD for this which is incorrect if the rebase is started
with 'git rebase <upstream> <branch>' as in that case ORIG_HEAD should
be updated to <branch>. This only affects the "apply" backend as the
"merge" backend does not yet use reset_head() for the initial
checkout. Fix this by passing in orig_head when calling reset_head()
and add some regression tests.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/rebase.c           |  1 +
 reset.c                    |  4 +++-
 reset.h                    |  2 ++
 t/t3418-rebase-continue.sh | 26 ++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

Comments

Junio C Hamano Oct. 1, 2021, 9:18 p.m. UTC | #1 
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> At the start of a rebase ORIG_HEAD is updated to tip of the branch
> being rebased. Unfortunately reset_head() always uses the current
> value of HEAD for this which is incorrect if the rebase is started
> with 'git rebase <upstream> <branch>' as in that case ORIG_HEAD should
> be updated to <branch>. This only affects the "apply" backend as the
> "merge" backend does not yet use reset_head() for the initial
> checkout. Fix this by passing in orig_head when calling reset_head()
> and add some regression tests.

All correct.  It is somewhat surprising that this wasn't caught and
reported as an issue for this long X-<.

> +	const struct object_id *orig_head;
>  	 /* Optional branch to switch to */
>  	const char *branch;
>  	/* Flags defined above */
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 738fbae9b29..be63456c5b9 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -323,4 +323,30 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
>  	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
>  '
>  
> +test_orig_head_helper() {

Need SP before "()" in addition to after it.  Ditto for the other one.

> +	test_when_finished 'git rebase --abort &&
> +		git checkout topic &&
> +		git reset --hard commit-new-file-F2-on-topic-branch' &&
> +	git update-ref -d ORIG_HEAD &&
> +	test_must_fail git rebase "$@" &&
> +	test_cmp_rev ORIG_HEAD commit-new-file-F2-on-topic-branch
> +}
> +
> +test_orig_head() {
> +	type=$1
> +	test_expect_success "rebase $type sets ORIG_HEAD correctly" '
> +		git checkout topic &&
> +		git reset --hard commit-new-file-F2-on-topic-branch &&
> +		test_orig_head_helper $type main
> +	'
> +
> +	test_expect_success "rebase $type <upstream> <branch> sets ORIG_HEAD correctly" '
> +		git checkout main &&
> +		test_orig_head_helper $type main topic
> +	'
> +}
> +
> +test_orig_head --apply
> +test_orig_head --merge
> +
>  test_done
diff mbox series

Patch

diff --git a/builtin/rebase.c b/builtin/rebase.c
index c40a8b843f4..c36a8a10e9b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -2074,6 +2074,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.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
 			RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
 	ropts.head_msg = msg.buf;
diff --git a/reset.c b/reset.c
index 2c32600234d..82bcf2dcb8b 100644
--- a/reset.c
+++ b/reset.c
@@ -14,6 +14,7 @@  static int update_refs(const struct reset_head_opts *opts,
 	unsigned detach_head = opts->flags & RESET_HEAD_DETACH;
 	unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
 	unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
+	const struct object_id *orig_head = opts->orig_head;
 	const char *switch_to_branch = opts->branch;
 	const char *reflog_branch = opts->branch_msg;
 	const char *reflog_head = opts->head_msg;
@@ -43,7 +44,8 @@  static int update_refs(const struct reset_head_opts *opts,
 				strbuf_addstr(&msg, "updating ORIG_HEAD");
 				reflog_orig_head = msg.buf;
 			}
-			update_ref(reflog_orig_head, "ORIG_HEAD", orig,
+			update_ref(reflog_orig_head, "ORIG_HEAD",
+				   orig_head ? orig_head : orig,
 				   old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
 		} else if (old_orig)
 			delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
diff --git a/reset.h b/reset.h
index c0f4e99a3be..91a868c5ba8 100644
--- a/reset.h
+++ b/reset.h
@@ -15,6 +15,8 @@ 
 struct reset_head_opts {
 	/* The oid of the commit to checkout/reset to. Defaults to HEAD */
 	const struct object_id *oid;
+	/* Optional commit when setting ORIG_HEAD. Defaults to HEAD */
+	const struct object_id *orig_head;
 	 /* Optional branch to switch to */
 	const char *branch;
 	/* Flags defined above */
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 738fbae9b29..be63456c5b9 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -323,4 +323,30 @@  test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_orig_head_helper() {
+	test_when_finished 'git rebase --abort &&
+		git checkout topic &&
+		git reset --hard commit-new-file-F2-on-topic-branch' &&
+	git update-ref -d ORIG_HEAD &&
+	test_must_fail git rebase "$@" &&
+	test_cmp_rev ORIG_HEAD commit-new-file-F2-on-topic-branch
+}
+
+test_orig_head() {
+	type=$1
+	test_expect_success "rebase $type sets ORIG_HEAD correctly" '
+		git checkout topic &&
+		git reset --hard commit-new-file-F2-on-topic-branch &&
+		test_orig_head_helper $type main
+	'
+
+	test_expect_success "rebase $type <upstream> <branch> sets ORIG_HEAD correctly" '
+		git checkout main &&
+		test_orig_head_helper $type main topic
+	'
+}
+
+test_orig_head --apply
+test_orig_head --merge
+
 test_done