Re: [PATCH v10 4/8] replay: support empty commit ranges

From: Elijah Newren <newren@gmail.com>
Date: Jan 13, 2026, 06:00 UTC
Message-ID: <CABPp-BGhtPyiVT=32NXz3k8m=+ZgPziXueM4Y8+g4dAUtN9osw@mail.gmail.com>
In-Reply-To: <20260112-b4-pks-history-builtin-v10-4-e3c6aa5b4cec@pks.im>

On Mon, Jan 12, 2026 at 6:17 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 7 quoted lines

>
> In a subsequent commit we're about to introduce a new user of the replay
> subsystem. With that new user, the range of commits that we'll want to
> replay will be identified implicitly via "HEAD". With such implicit
> ranges it becomes likely that the range of revisions that we're asked to
> replay becomes empty. This case does not make sense with git-replay(1),
> but with the new command it will.

I think I know what you were trying to say, but this feels misleading; it could be the commit at the tip of any branch, not just HEAD. Perhaps:

In a subsequent commit we're about to introduce a new user of the replay subsystem. With that new user, the range of commits that we'll want to replay will be identified implicitly via a single commit, and will include all descendants of that commit to any branch. If that commit has no descendants (because it's the tip of some branch), then the range of revisions that we're asked to replay becomes empty. This case does not make sense with git-replay(1), but with the new command it will.

Show 41 quoted lines

> This case is not currently supported by `replay_revisions()` though
> because we zero-initialize `struct merge_result`. This includes its
> `.clean` member, which indicates whether the merge ran into a conflict
> or not. But given that we don't have any revision to replay, we won't
> ever perform any merge at all, and consequently that member will never
> be set to `1`. We thus later think that there's been a merge conflict
> and return an error from `replay_commits()`.
>
> Address this issue by initializing the `.clean` member to `1`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  replay.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/replay.c b/replay.c
> index 1e660171d2..a8e6d5b30b 100644
> --- a/replay.c
> +++ b/replay.c
> @@ -266,7 +266,9 @@ int replay_revisions(struct rev_info *revs,
>         struct commit *commit;
>         struct commit *onto = NULL;
>         struct merge_options merge_opt;
> -       struct merge_result result;
> +       struct merge_result result = {
> +               .clean = 1,
> +       };
>         char *advance;
>         int ret;
>
> @@ -282,7 +284,6 @@ int replay_revisions(struct rev_info *revs,
>         }
>
>         init_basic_merge_options(&merge_opt, revs->repo);
> -       memset(&result, 0, sizeof(result));
>         merge_opt.show_rename_progress = 0;
>         last_commit = onto;
>         replayed_commits = kh_init_oid_map();
>
> --
> 2.52.0.590.g1f87b77810.dirty

Looks good otherwise; thanks for splitting this out.