From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Wed, 30 Sep 2026 01:29:25 GMT
Subject: Re: [PATCH v4 5/5] builtin/stash: merge index in-core
Message-ID: <CALnO6CDDAomqb+MqRw10Kj048gL5+k+3k_4kVxbjTB1YrK3fXg@mail.gmail.com>
In-Reply-To: <xmqq4if7g6u1.fsf@gitster.g>

On Tue, Sep 29, 2026 at 4:07 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
>
> > +                     merge_incore_nonrecursive(&o, merge_base, head, merge,
> > +                                               &result);
>
> In a hard error from merge_incore_nonrecursive(), result->clean is
> set to -1, which means that ...
>
> > +                     if (!result.clean) {
>
> ... "result.clean is false" is not true here, so we will ...
>
> > +                             merge_finalize(&o, &result);
> >                               return error(_("conflicts in index. "
> >                                              "Try without --index."));
> > +                     } else {
>
> ... come here to access result.tree member, no?
>
> > +                             oidcpy(&index_tree, &result.tree->object.oid);
> > +                             merge_finalize(&o, &result);
> > +                     }
>
> IOW, shouldn't it be more like three-way check,

Yep. Missed that when looking at the result struct. Will fix.

>
>                         if (result.clean < 0) {
>                                 merge_finalize(&o, &result);
>                                 return error(_("index merge failed."));
>                         } else if (!result.clean) {
>                                 merge_finalize(&o, &result);
>                                 return error(_("conflict in index merge."));
>                         } else {
>                                 ... happy path ...
>                         }
>
> or something like that?


-- 
D. Ben Knoble