Re: [PATCH 1/2] replay: fail gracefully when a merge input is unreadable
From: Junio C Hamano <hidden>
Date: 2026-08-19 18:09:54
"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted hunk ↗ jump to hunk
From: Elijah Newren <redacted> 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 <redacted> --- 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.