Re: [PATCH 1/2] replay: fail gracefully when a merge input is unreadable

On Wed, Aug 19, 2026 at 11:09 AM Junio C Hamano <[email protected]> wrote:
>
> "Elijah Newren via GitGitGadget" <[email protected]> writes:
>
> > From: Elijah Newren <[email protected]>
> >
> > When objects involved in the merge cannot be read, the merge machinery
> > will return early with result.clean = -1, and result.tree left as NULL.
> > pick_regular_commit() tested only "if (!result->clean)", ignoring the
> > case where "clean < 0".  That causes the code to try to use
> > result->tree, resulting in a SIGSEGV.
> >
> > Handle clean < 0 explicitly; the merge machinery will already have printed
> > messages such as "Could not read <object>" and "collecting merge info
> > failed for trees...", so we don't need to add much detail beyond the
> > fact that the merge failed.
> >
> > Signed-off-by: Elijah Newren <[email protected]>
> > ---
> >  replay.c                 |  7 +++++++
> >  t/t3650-replay-basics.sh | 35 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 42 insertions(+)
> >
> > diff --git a/replay.c b/replay.c
> > index 463c900d6c..33e21b2032 100644
> > --- a/replay.c
> > +++ b/replay.c
> > @@ -327,6 +327,13 @@ static struct commit *pick_regular_commit(struct repository *repo,
> >       merge_opt->ancestor = NULL;
> >       merge_opt->branch2 = NULL;
> >
> > +     if (result->clean < 0) {
> > +             error(_("merge of %s onto %s failed"),
> > +                   oid_to_hex(&pickme->object.oid),
> > +                   oid_to_hex(&replayed_base->object.oid));
> > +             return NULL;
> > +     }
> > +
> >       if (!result->clean)
> >               return NULL;
>
> Hmph, so anything but "0 < result->clean" is a failure, but we by
> mistake took any non-zero value as OK?  That is an obvious mistake.
> Well spotted and fixed.

Thanks, but the bug was also caused by me -- e787e664da64 (replay:
introduce pick_regular_commit(), 2023-11-24) -- so not sure I should
get much credit for finding it three years later.

> > +             # Ensure replay gracefully handles the missing object
> > +             test_must_fail git replay --onto onto base..side 2>err &&
> > +             test_grep ! "[Ss]egmentation" err &&
> > +             test_grep "Could not read\|collecting merge info failed" err
>
> "test_must_fail" means "the tested command must fail voluntarily and
> in a controlled way", so a segfaulting git-replay invocation would
> not pass test_must_fail.  Hence, there is no need to separately
> test "test_grep ! '[sS]egmentation'".

Oops, you're right.

You said on 2/2 that I don't need to rebase because you're putting
together an evil merge.  Do you want me to resubmit with this line
removed (without changing the series' base), or would you rather I
avoid that to prevent merging work for you?