Thread (70 messages) 70 messages, 4 authors, 9h ago

Re: [PATCH v2 4/4] builtin/stash: merge index in-core

From: D. Ben Knoble <hidden>
Date: 2026-09-25 13:01:03

On Thu, Sep 24, 2026 at 5:59 PM Junio C Hamano [off-list ref] wrote:
Ahh, or perhaps the trees are indeed given in a wrong order, but not
in a random wrong order.  merge_ort_nonrecursive(), which is *not*
the function you are using, takes head, merge, and merge_base in
this order, and that order matches what you wrote.

Perhaps the true culprit in this confusion is that the order in
which merge_ort_nonrecursive() takes its three trees (head, merge,
and common) and the order in which merge_incore_nonrecursive() takes
its trees (merge_base, side1, and side2) are different, and if we
fix them to match, it would make it easier to work with?
Indeed, the confusion is that simple ;) Shamefully, we don't have
enough test coverage to catch that regression, so I'm very glad indeed
you spotted it.
The new test in the attached patch will fail with this step but if
we revert the changes to builtin/stash.c in this step, it passes.
Any objection to me adding this test as a preparatory patch? There's
no sign-off, so I don't want to mess up the DCO here.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help