From: Johannes Schindelin <Johannes.Schindelin@gmx•de>
To: Ben Knoble <ben.knoble@gmail•com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org, Elijah Newren <newren@gmail•com>,
Patrick Steinhardt <ps@pks•im>
Subject: Re: [PATCH/RFC 0/5] replay: support replaying 2-parent merges
Date: Sun, 17 May 2026 13:33:22 +0200 (CEST) [thread overview]
Message-ID: <c594ed5c-5c81-c6b7-c660-11b2ce1bb3b5@gmx.de> (raw)
In-Reply-To: <21A507D3-1B0D-4404-8AF5-9485B01E63A6@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]
Hi Ben,
On Thu, 7 May 2026, Ben Knoble wrote:
> > Le 7 mai 2026 à 11:06, Johannes Schindelin <johannes.schindelin@gmx•de> a écrit :
> >
> >> On Thu, 7 May 2026, D. Ben Knoble wrote:
> >>
> >>> On Wed, May 6, 2026 at 6:44 PM Johannes Schindelin via GitGitGadget
> >>> <gitgitgadget@gmail•com> 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.
prev parent reply other threads:[~2026-05-17 11:34 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-06 22:43 [PATCH/RFC 0/5] replay: support replaying 2-parent merges Johannes Schindelin via GitGitGadget
2026-05-06 22:43 ` [PATCH/RFC 1/5] " Johannes Schindelin via GitGitGadget
2026-05-08 9:36 ` Phillip Wood
2026-05-08 10:05 ` Phillip Wood
2026-05-17 14:33 ` Johannes Schindelin
2026-05-17 14:32 ` Johannes Schindelin
2026-05-26 21:15 ` Kristoffer Haugsbakk
2026-05-06 22:43 ` [PATCH/RFC 2/5] replay: short-circuit merge replay when parent and base trees are unchanged Johannes Schindelin via GitGitGadget
2026-05-06 22:43 ` [PATCH/RFC 3/5] history.adoc: describe merge-replay support and its limits Johannes Schindelin via GitGitGadget
2026-05-06 22:43 ` [PATCH/RFC 4/5] test-tool: add a "historian" subcommand for building merge fixtures Johannes Schindelin via GitGitGadget
2026-05-12 10:54 ` Toon Claes
2026-05-17 11:40 ` Johannes Schindelin
2026-05-06 22:43 ` [PATCH/RFC 5/5] t3454: cover merge-replay scenarios with the historian helper Johannes Schindelin via GitGitGadget
2026-05-07 14:14 ` [PATCH/RFC 0/5] replay: support replaying 2-parent merges D. Ben Knoble
2026-05-07 15:06 ` Johannes Schindelin
2026-05-07 15:39 ` Ben Knoble
2026-05-17 11:33 ` Johannes Schindelin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c594ed5c-5c81-c6b7-c660-11b2ce1bb3b5@gmx.de \
--to=johannes.schindelin@gmx$(echo .)de \
--cc=ben.knoble@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox