Re: [PATCH 1/2] replay: fail gracefully when a merge input is unreadable
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 19, 2026, 18:09 UTC
- Message-ID
- <xmqqfr0augls.fsf@gitster.g>
- In-Reply-To
- <321af575e0a9e0c22c70c1809f6fbf0265b05d4c.1787092446.git.gitgitgadget@gmail.com>
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 36 quoted lines
> From: Elijah Newren <newren@gmail.com>
>
> 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 <newren@gmail.com>
> ---
> 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.
> + # 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'".
Besides, the spelling used by strsignal() is implementation-defined, so you cannot reliably grep for it anyway.
> + ) > +' > + > test_done
Thanks.