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

From: Junio C Hamano <gitster@pobox.com>
To: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH 10/11] rebase --apply: set ORIG_HEAD correctly
Date: Fri, 01 Oct 2021 14:18:57 -0700	[thread overview]
Message-ID: <xmqqwnmw4f5q.fsf@gitster.g> (raw)
In-Reply-To: <e8884efcc83960fc0ea5d5aee72bb9839b4be397.1633082702.git.gitgitgadget@gmail.com> (Phillip Wood via GitGitGadget's message of "Fri, 01 Oct 2021 10:05:01 +0000")

"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