From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Fri, 25 Sep 2026 13:00:51 GMT
Subject: Re: [PATCH v2 4/4] builtin/stash: merge index in-core
Message-ID: <CALnO6CBhoBcVjLXidvii+o_Ump_k9disW177LeSS0118t3oGKg@mail.gmail.com>
In-Reply-To: <xmqqse2yz4y4.fsf@gitster.g>

On Thu, Sep 24, 2026 at 5:59 PM Junio C Hamano <gitster@pobox.com> 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.