From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 13 Jan 2026 07:31:15 GMT
Subject: Re: [PATCH v9 2/7] builtin/replay: move core logic into "libgit.a"
Message-ID: <aWX0wyQ_2Mn9ZOEJ@pks.im>
In-Reply-To: <CABPp-BGM4AxoedD3uUnS+12n5c0egd8pw-=cRsO64oDs+G9RkA@mail.gmail.com>

On Mon, Jan 12, 2026 at 10:00:16PM -0800, Elijah Newren wrote:
> On Mon, Jan 12, 2026 at 5:02 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Fri, Jan 09, 2026 at 05:16:41PM -0800, Elijah Newren wrote:
> [...]
> > > It feels duplicative to have replay_result include a merge_conflict
> > > field and to have replay_revisions() return an int which signifies
> > > whether there's a conflict.  Can we remove one of the two?  (Perhaps
> > > the merge_conflict field was only a workaround to the weird ret
> > > setting from the previous patch?)
> >
> > The idea here is that we have a generic error code that tells the caller
> > that _something_ happened, whereas `struct replay_result` gives the
> > caller a bit more context around what exactly has happened. This allows
> > callers to handle merge conflicts differently from any other error and
> > makes the different failure modes a lot more explicit.
> >
> > Some context: at GitLab we actually have the use case to surface more
> > information around what commits have conflicted, so there will be a
> 
> Interesting, but doesn't answering that question presume first-class
> conflict handling?  How do you determine which commits conflict
> without that?  Or, is the first commit we process that hits a conflict
> sufficient information and you don't really need the commits that
> conflict, just one of them?  Or, do you presume that all unprocessed
> commits after the first one that conflicted would have also conflicted
> (even if it touched files that are conflict-free so far, so that
> commit would not have contributed to the conflicts)?  If this is done
> without first-class conflict handling, there may also be an assumption
> here about linear single-branch history, or else some kind of attempt
> to continue processing whichever commits don't have an ancestor that
> has conflicted, so that we can enumerate "commits [which] have
> conflicted".

Yup, knowing about the first commit that conflicts is sufficient for our
use case.

> > follow-up patch series that extends `struct replay_result` to return
> > more information about the actual conflict. I'm already planning ahead a
> > bit in this patch series.
> 
> Wait, above you said you wanted to know the "commits [which] have
> conflicted", here you seem to be suggesting you want to know about
> "the actual conflict" which might mean you only care about the first
> commit that conflicted but you want details about what conflicted
> within it.  Or is it perhaps the set of files that would have had
> conflicts across replaying the whole sequence of patches (in which
> case a bunch of the previous questions are still valid)?

I was simply being inaccurate there, we only care about the first
conflicting commit, no plural.

Thanks!

Patrick