On 6/18/24 11:00 PM, Elijah Newren via GitGitGadget wrote:
From: Elijah Newren <newren@xxxxxxxxx>
The calling convention for the merge machinery is
One call to init_merge_options()
One or more calls to merge_incore_[non]recursive()
One call to merge_finalize()
(possibly indirectly via merge_switch_to_result())
Both merge_switch_to_result() and merge_finalize() expect
opt->priv == NULL && result->priv != NULL
which is supposed to be set up by our move_opt_priv_to_result_priv()
function. However, two codepaths dealing with error cases did not
execute this necessary logic, which could result in assertion failures
(or, if assertions were compiled out, could result in segfaults). Fix
the oversight and add a test that would have caught one of these
problems.
"one of these problems" is key here.
@@ -5050,6 +5050,7 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
oid_to_hex(&side1->object.oid),
oid_to_hex(&side2->object.oid));
result->clean = -1;
+ move_opt_priv_to_result_priv(opt, result);
return;
} > trace2_region_leave("merge", "collect_merge_info", opt->repo);
Removing this line does not cause your test script to fail. This is
understandable as this case only happens during a parse failure, so
it would be unreasonable to generate a test case that only fails
for such a reason.
@@ -5080,7 +5081,7 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
/* existence of conflicted entries implies unclean */
result->clean &= strmap_empty(&opt->priv->conflicted);
}
- if (!opt->priv->call_depth)
+ if (!opt->priv->call_depth || result->clean < 0)
move_opt_priv_to_result_priv(opt, result);
}
Removing this change does get caught by the new test.
Thanks,
-Stolee
