Re: [PATCH v9 2/7] builtin/replay: move core logic into "libgit.a"
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 13, 2026, 07:31 UTC
- 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:
Show 32 quoted lines
> 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.
Show 11 quoted lines
> > 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