Subject: Re: [PATCH/RFC 0/5] replay: support replaying 2-parent merges

From: Johannes Schindelin <[email-masked]>
Date: Sun, 17 May 2026 13:33:22 +0200 (CEST)

Hi Ben,

On Thu, 7 May 2026, Ben Knoble wrote:

> > Le 7 mai 2026 à 11:06, Johannes Schindelin <[email-masked]> a écrit :
> > 
> >> On Thu, 7 May 2026, D. Ben Knoble wrote:
> >> 
> >>> On Wed, May 6, 2026 at 6:44 PM Johannes Schindelin via GitGitGadget
> >>> <[email-masked]> wrote:
> >>> 
> >>> [...]
> >>> 
> >>> While I was at it, git history reword had a pre-existing
> >>> silent-success bug: a positive return from replay_revisions() (which
> >>> means "conflict, no updates queued") was treated as success. Obviously
> >>> this should never occur, as a reword simply does not change any of the
> >>> file contents, but bugs do happen. The merge-replay work is complex
> >>> enough to make that class of bugs more likely, therefore I introduce
> >>> error messages for those instances.
> >> 
> >> Fixing this bug sounded interesting; I had a hard time spotting it
> >> while skimming the first 2 patches.
> > 
> > It's this part:
> > 
> > @@ -482,6 +482,9 @@ static int cmd_history_reword(int argc,
> >    if (ret < 0) {
> >        ret = error(_("failed replaying descendants"));
> >        goto out;
> > +    } else if (ret) {
> > +        ret = error(_("conflict during replay; some descendants were not rewritten"));
> > +        goto out;
> >    }
> > 
> >    ret = 0;
> > @@ -721,6 +724,9 @@ static int cmd_history_split(int argc,
> >    if (ret < 0) {
> >        ret = error(_("failed replaying descendants"));
> >        goto out;
> > +    } else if (ret) {
> > +        ret = error(_("conflict during replay; some descendants were not rewritten"));
> > +        goto out;
> >    }
> > 
> >    ret = 0;
> 
> Thanks, super helpful.
> 
> (Perhaps later) if we can say _which_ descendants weren’t rewritten, that might be good.

I am afraid that that particular information is lost at this point, all we
have to work with is an `int ret`.

Ciao,
Johannes

> >> Did I just miss it? Is it worth splitting that fix out to a separate patch?
> > 
> > Well, you _could_ argue that they were not bugs at all: a `git history
> > reword` isn't supposed to be able to result in merge conflicts, nor is
> > `git history split` because they leave the respective commits tree-same
> > (in the `split` case, the second commit).
> 
> I seem to recall Patrick making a similar argument, but don’t let me put
> words in anyone’s mouth. 
> 
> > I could see the point were anybody to suggest using `BUG()` instead of
> > `error()` here, but erred on the "nicer to the user" side.
> > 
> > The only way this _might_ be triggered before this patch series is most
> > likely by playing games with replace objects. Or maybe you cannot trigger
> > it at all.
> > 
> > With the changes in this here patch series, I wasn't so certain that I had
> > covered all the edge cases (an early iteration of the quick short-cut in
> > patch 2/5 keyed only on the parent commits' trees, and forgot to verify
> > the merge _bases_' trees, for example). That's why I think it matters more
> > now than it did before.
> > 
> > Ciao,
> > Johannes
> 
> Makes sense, thanks.