Re: [PATCH 5/7] rebase --apply: make reflog messages match rebase --merge

From: Elijah Newren <newren@gmail.com>
Date: Apr 17, 2022, 02:03 UTC
Message-ID: <CABPp-BEqjv8BVe3G5x=3wRbL9GH8GeWe4Yv2wiUse4vjWcd58w@mail.gmail.com>
In-Reply-To: <1c3ec1654225ff090c194fa646f974082bd9c944.1645441854.git.gitgitgadget@gmail.com>

On Mon, Feb 21, 2022 at 3:19 PM Phillip Wood via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 13 quoted lines

>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> The apply backend creates slightly different reflog messages to the
> merge backend when starting and finishing a rebase and when picking
> commits. The choice of backend is really an implementation detail so
> it is confusing to have the same command create different messages
> depending on which backend is selected. Change the apply backend so
> the reflog messages from the two backends match as closely as
> possible. Note that there is still a difference when committing a
> conflict resolution - the merge backend will use "(continue)" rather
> than "(pick)" in that case as it does not know which command created
> the conflict that it is committing.

I've always been slightly annoyed by the various differences in these backends; it's nice to see another one disappearing.

Show 31 quoted lines

>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/rebase.c          | 9 +++++----
>  t/t3406-rebase-message.sh | 2 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index e50361fc2a9..678339c7bf7 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -580,10 +580,10 @@ static int move_to_original_branch(struct rebase_options *opts)
>         if (!opts->onto)
>                 BUG("move_to_original_branch without onto");
>
> -       strbuf_addf(&branch_reflog, "%s finished: %s onto %s",
> +       strbuf_addf(&branch_reflog, "%s (finish): %s onto %s",
>                     getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
>                     opts->head_name, oid_to_hex(&opts->onto->object.oid));
> -       strbuf_addf(&head_reflog, "%s finished: returning to %s",
> +       strbuf_addf(&head_reflog, "%s (finish): returning to %s",
>                     getenv(GIT_REFLOG_ACTION_ENVIRONMENT), opts->head_name);
>         ropts.branch = opts->head_name;
>         ropts.flags = RESET_HEAD_REFS_ONLY;
> @@ -613,7 +613,8 @@ static int run_am(struct rebase_options *opts)
>
>         am.git_cmd = 1;
>         strvec_push(&am.args, "am");
> -
> +       strvec_pushf(&am.env_array, GIT_REFLOG_ACTION_ENVIRONMENT "=%s (pick)",
> +                    getenv(GIT_REFLOG_ACTION_ENVIRONMENT));

Kinda sad that we're still forking subprocesses and passing things via environment variable, but that's outside the scope of this series. Fix looks fine.

Show 27 quoted lines

>         if (opts->action && !strcmp("continue", opts->action)) {
>                 strvec_push(&am.args, "--resolved");
>                 strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg);
> @@ -1763,7 +1764,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>                 printf(_("First, rewinding head to replay your work on top of "
>                          "it...\n"));
>
> -       strbuf_addf(&msg, "%s: checkout %s",
> +       strbuf_addf(&msg, "%s (start): checkout %s",
>                     getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
>         ropts.oid = &options.onto->object.oid;
>         ropts.orig_head = &options.orig_head,
> diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
> index bb2a4949abc..5c6cd9af3bc 100755
> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
> @@ -88,7 +88,7 @@ test_expect_success 'error out early upon -C<n> or --whitespace=<bad>' '
>  write_reflog_expect () {
>         if test $mode = --apply
>         then
> -               sed 's/(finish)/finished/; s/ ([^)]*)//'
> +               sed 's/(continue)/(pick)/'
>         else
>                 cat
>         fi >expect
> --
> gitgitgadget