Re: [PATCH v2 13/14] rebase --apply: set ORIG_HEAD correctly

[Elijah Newren <newren@gmail.com>]

On Wed, Dec 8, 2021 at 6:58 AM Phillip Wood via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> At the start of a rebase ORIG_HEAD is updated to tip of the branch

Could we insert a comma between "rebase" and "ORIG_HEAD"?  Otherwise I
try to read it as "At the start of a `rebase ORIG_HEAD` is updated..."
and get totally lost.  I had to re-read the sentence a few times
before I understood what it was trying to say.

Also, perhaps s/to tip of/to the tip of/ ?

> 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                    |  4 ++++
>  t/t3418-rebase-continue.sh | 26 ++++++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index b55a9cff05d..e942c300f8c 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1769,6 +1769,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 e02915c0f65..448cb3fd785 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -15,6 +15,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", head,
> +                       update_ref(reflog_orig_head, "ORIG_HEAD",
> +                                  orig_head ? orig_head : head,
>                                    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 7ef7e43ea8c..a28f81829d8 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -22,6 +22,10 @@ struct reset_head_opts {
>          * The commit to checkout/reset to. Defaults to HEAD.
>          */
>         const struct object_id *oid;
> +       /*
> +        * Optional value to set ORIG_HEAD. Defaults to HEAD.
> +        */
> +       const struct object_id *orig_head;
>         /*
>          * Optional branch to switch to.
>          */
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 22eca73aa3e..130e2f9b553 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -308,4 +308,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
> --