replay: support replaying 2-parent merges#2106
Draft
dscho wants to merge 11 commits into
Draft
Conversation
dscho
force-pushed
the
support-merge-commits-in-git-history
branch
4 times, most recently
from
dd9b8e1 to
2dec28b
Compare
Member
Author
|
/submit |
|
Submitted as pull.2106.git.1778107405.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
"D. Ben Knoble" wrote on the Git mailing list (how to reply to this email): Hi Dscho,
On Wed, May 6, 2026 at 6:44 PM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> git history, the new history-rewriting builtin in v2.54, dies on any merge
> in the rewrite path with replaying merge commits is not supported yet!. That
> makes it not very useful for the workflows I actually have, where almost
> every interesting branch contains at least one merge of a feature topic. The
> natural fallback, git rebase --rebase-merges, is interactive and stops to
> ask for re-resolution even when no re-resolution is needed.
>
> This series lifts that limitation for the common 2-parent case. The
> algorithm itself is not new: Elijah Newren wrote it down in his replay
> design notes
> [https://github.com/newren/git/blob/replay/replay-design-notes.txt] and
> prototyped it in a 2022 work-in-progress sketch
> [https://github.com/newren/git/commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45].
> What is new is wiring it into the replay_revisions() API that backs both git
> replay and git history, plus three specific tweaks that make the trickier
> cases work where the WIP sketch bailed out: identical conflict-marker labels
> for the inner remerges of the original and the rewritten parents (so their
> conflict-markered trees compare equal in the regions the user did not
> touch), tolerating result.clean == 0 from those inner merges (their
> well-defined conflict-markered trees are valid inputs to the outer 3-way
> merge), and self-fallback for both merge parents combined with mapping the
> rev-range boundary commits to the onto commit.
>
> Octopus merges and revert-of-merge are surfaced as explicit errors at the
> dispatch point. The split sub-command of git history continues to refuse
> when its target is a merge: split semantics simply do not apply there. The
> xdiff special mode for matching conflict-marker hunks across inner remerges,
> the XDL_MERGE_FAVOR_BASE variant, and the modify/delete and binary-file
> specials that the design notes flag as future work all remain future work.
>
> 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.
Did I just miss it? Is it worth splitting that fix out to a separate patch?
Best,
--
D. Ben Knoble |
|
User |
|
Johannes Schindelin wrote on the Git mailing list (how to reply to this email): Hi Ben,
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;
> 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 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 |
|
Ben Knoble wrote on the Git mailing list (how to reply to this email): >
> Le 7 mai 2026 à 11:06, Johannes Schindelin <johannes.schindelin@gmx.de> a écrit :
>
> Hi Ben,
>
>> 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.
>> 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. |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt, const char *value, int uns | |||
| return 0; | |||
There was a problem hiding this comment.
Phillip Wood wrote on the Git mailing list (how to reply to this email):
Hi Johannes
On 06/05/2026 23:43, Johannes Schindelin via GitGitGadget wrote:
> > Elijah Newren spelled out a way to lift this limitation in his
> replay-design-notes [1] and prototyped it in a 2022
> work-in-progress sketch [2]. The idea is that a merge commit M on
> parents (P1, P2) records both an automatic merge of those parents
> AND any manual layer the author put on top of that automatic merge
> (textual conflict resolution and any semantic edit outside conflict
> markers). Replaying M onto rewritten parents (P1', P2') must
> preserve that manual layer, but the rewritten parents change the
> automatic merge, so a simple cherry-pick is wrong: the manual layer
> would be re-introduced on top of stale auto-merge text.
> > What works instead is a three-way merge of three trees the existing
> infrastructure already knows how to compute. Let R be the recursive
> auto-merge of (P1, P2), O be M's actual tree and N be the recursive
> auto-merge of (P1', P2'). Then `git diff R O` is morally
> `git show --remerge-diff M`: it captures exactly what the author
> added on top of the automatic merge. A non-recursive 3-way merge
> with R as the merge base, O as side 1 and N as side 2 layers that
> manual contribution onto the freshly auto-merged rewritten parents
> (N) and produces the replayed tree.
So we cherry-pick the difference between the user's conflict resolution O and the auto-merge M of the original parents onto the auto-merge N of the replayed parents. If we have a topology that looks like
|
A
/|\
/ B \
E | D
C /
|/
O
then running
git replay --onto E --ancestry-path B..O
will replay C and O onto E. If the changes in E and D conflict but those conflicts do not overlap with the conflicts in M that were resolved to create O then the replayed version of O will contain conflict markers from the conflicting changes in E and D. Because the previous conflict resolution applies to N without conflicts we do not recognize that there are still conflicts in N that need to be resolved.
Having realized this I went to look at Elijah's notes and they recognize this possibility and suggest extending the xdiff merge code to detect when N has conflicts that do not correspond to the conflicts in M. That sounds like quite a lot of work. I've not put much effort into coming up with a counterexample but think that because "git replay" and "git history" do not yet allow the commits in the merged branches to be edited we may be able to safely use the implementation proposed in this series if both merge parents have been rebased (or we might want all the merge bases of the new merge to be a descendants of "--onto"). In the example above if both the parents were rebased onto E then any new conflicts would happen when picking D rather than when recreating the merge.
Thanks
Phillip
> Implement `pick_merge_commit()` along those lines and dispatch to it
> from `replay_revisions()` when the commit being replayed has exactly
> two parents. Two specific points (learned the hard way) keep
> non-trivial cases working where the WIP sketch [2] bailed out.
> First, R and N use identical `merge_options.branch1` and `branch2`
> labels ("ours"/"theirs"). When the original parents conflicted on a
> region of a file, both R and N produce textually identical conflict
> markers; the outer non-recursive merge then sees N == R in that
> region and the user's manual resolution from O wins cleanly. Without
> this, the conflict-marker text would differ between R and N (because
> the inner merges would label the conflicts differently), and the
> outer merge would itself be unclean even when the user did supply a
> clean resolution. Second, an unclean inner merge
> (`result.clean == 0`) is _not_ fatal: the tree merge-ort produces in
> that case still has well-defined contents (with conflict markers in
> the conflicted files) and is a valid input to the outer
> non-recursive merge. Only a real error (`< 0`) propagates as
> failure.
> > The replay propagates the textual diffs the user actually made in M;
> it does _not_ extrapolate symbol-level intent. If rewriting the
> parents pulls in genuinely new content (for example, a brand-new
> caller of a function that the merge renamed), that new content stays
> as the rewritten parents have it. Symbol-aware refactoring is out of
> scope here, just as it is for plain rebase.
> > Octopus merges (more than two parents) and revert-of-merge are not
> supported and are surfaced as explicit errors at the dispatch point.
> The "split" sub-command of `git history` continues to refuse when
> the targeted commit is itself a merge: split semantics do not apply
> to merges. The pre-walk gate in `builtin/history.c` that previously
> rejected any merge in the rewrite path now only rejects octopus
> merges; rename it accordingly.
> > A small refactor in `create_commit()` makes the merge case possible:
> the helper now takes a `struct commit_list *parents` rather than a
> single parent pointer and takes ownership of the list. The single
> existing caller in `pick_regular_commit()` builds and passes a
> one-element list; the new `pick_merge_commit()` builds a two-element
> list, with the order of the `from` and `merge` parents preserved.
> > Update the negative expectations in t3451, t3452 and t3650 that were
> asserting the now-retired "not supported yet" message, replacing
> them with positive coverage where it fits. Octopus rejection and
> revert-of-merge rejection are covered by new positive tests in
> t3650. A dedicated test script with merge-replay scenarios driven by
> a new test-tool fixture builder will follow in a subsequent commit.
> > [1] https://github.com/newren/git/blob/replay/replay-design-notes.txt
> [2] https://github.com/newren/git/commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45
> > Helped-by: Elijah Newren <newren@gmail.com>
> Assisted-by: Claude Opus 4.7
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/history.c | 16 ++-
> replay.c | 209 ++++++++++++++++++++++++++++++++++++--
> t/t3451-history-reword.sh | 21 ++--
> t/t3452-history-split.sh | 6 +-
> t/t3650-replay-basics.sh | 46 ++++++++-
> 5 files changed, 269 insertions(+), 29 deletions(-)
> > diff --git a/builtin/history.c b/builtin/history.c
> index 9526938085..00097b2226 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt, const char *value, int uns
> return 0;
> }
> > -static int revwalk_contains_merges(struct repository *repo,
> - const struct strvec *revwalk_args)
> +static int revwalk_contains_octopus_merges(struct repository *repo,
> + const struct strvec *revwalk_args)
> {
> struct strvec args = STRVEC_INIT;
> struct rev_info revs;
> int ret;
> > strvec_pushv(&args, revwalk_args->v);
> - strvec_push(&args, "--min-parents=2");
> + strvec_push(&args, "--min-parents=3");
> > repo_init_revisions(repo, &revs, NULL);
> > @@ -217,7 +217,7 @@ static int revwalk_contains_merges(struct repository *repo,
> }
> > if (get_revision(&revs)) {
> - ret = error(_("replaying merge commits is not supported yet!"));
> + ret = error(_("replaying octopus merges is not supported"));
> goto out;
> }
> > @@ -289,7 +289,7 @@ static int setup_revwalk(struct repository *repo,
> strvec_push(&args, "HEAD");
> }
> > - ret = revwalk_contains_merges(repo, &args);
> + ret = revwalk_contains_octopus_merges(repo, &args);
> if (ret < 0)
> goto out;
> > @@ -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;
> diff --git a/replay.c b/replay.c
> index f96f1f6551..3dbce095f9 100644
> --- a/replay.c
> +++ b/replay.c
> @@ -1,6 +1,7 @@
> #define USE_THE_REPOSITORY_VARIABLE
> > #include "git-compat-util.h"
> +#include "commit-reach.h"
> #include "environment.h"
> #include "hex.h"
> #include "merge-ort.h"
> @@ -77,15 +78,21 @@ static void generate_revert_message(struct strbuf *msg,
> repo_unuse_commit_buffer(repo, commit, message);
> }
> > +/*
> + * Build a new commit with the given tree and parent list, copying author,
> + * extra headers and (for pick mode) the commit message from `based_on`.
> + *
> + * Takes ownership of `parents`: it will be freed before returning, even on
> + * error. Parent order is preserved as supplied by the caller.
> + */
> static struct commit *create_commit(struct repository *repo,
> struct tree *tree,
> struct commit *based_on,
> - struct commit *parent,
> + struct commit_list *parents,
> enum replay_mode mode)
> {
> struct object_id ret;
> struct object *obj = NULL;
> - struct commit_list *parents = NULL;
> char *author = NULL;
> char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
> struct commit_extra_header *extra = NULL;
> @@ -96,7 +103,6 @@ static struct commit *create_commit(struct repository *repo,
> const char *orig_message = NULL;
> const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
> > - commit_list_insert(parent, &parents);
> extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> if (mode == REPLAY_MODE_REVERT) {
> generate_revert_message(&msg, based_on, repo);
> @@ -273,6 +279,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
> {
> struct commit *base, *replayed_base;
> struct tree *pickme_tree, *base_tree, *replayed_base_tree;
> + struct commit_list *parents = NULL;
> > if (pickme->parents) {
> base = pickme->parents->item;
> @@ -327,7 +334,143 @@ static struct commit *pick_regular_commit(struct repository *repo,
> if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
> !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
> return replayed_base;
> - return create_commit(repo, result->tree, pickme, replayed_base, mode);
> + commit_list_insert(replayed_base, &parents);
> + return create_commit(repo, result->tree, pickme, parents, mode);
> +}
> +
> +/*
> + * Replay a 2-parent merge commit by composing three calls into merge-ort:
> + *
> + * R = recursive merge of pickme's two original parents (auto-remerge of
> + * the original merge, accepting any conflicts)
> + * N = recursive merge of the (possibly rewritten) parents
> + * O = pickme's tree (the user's actual merge, including any manual
> + * resolutions)
> + *
> + * The picked tree comes from a non-recursive merge using R as the base,
> + * O as side1 and N as side2. `git diff R O` is morally `git show
> + * --remerge-diff $oldmerge`, so this layers the user's original manual
> + * resolution on top of the freshly auto-merged rewritten parents (see
> + * `replay-design-notes.txt` on the `replay` branch of newren/git).
> + *
> + * If the outer 3-way merge is unclean, propagate the conflict status to
> + * the caller via `result->clean = 0` and return NULL. The two inner
> + * merges (R and N) being unclean is _not_ fatal: the conflict-markered
> + * trees they produce are valid inputs to the outer merge, and using
> + * identical labels for both inner merges keeps the marker text
> + * byte-equal between R and N so the user's resolution recorded in O
> + * collapses the conflict cleanly there. Octopus merges (more than two
> + * parents) and revert-of-merge are rejected by the caller before this
> + * function is invoked.
> + */
> +static struct commit *pick_merge_commit(struct repository *repo,
> + struct commit *pickme,
> + kh_oid_map_t *replayed_commits,
> + struct merge_options *merge_opt,
> + struct merge_result *result)
> +{
> + struct commit *parent1, *parent2;
> + struct commit *replayed_par1, *replayed_par2;
> + struct tree *pickme_tree;
> + struct merge_options remerge_opt = { 0 };
> + struct merge_options new_merge_opt = { 0 };
> + struct merge_result remerge_res = { 0 };
> + struct merge_result new_merge_res = { 0 };
> + struct commit_list *parent_bases = NULL;
> + struct commit_list *replayed_bases = NULL;
> + struct commit_list *parents;
> + struct commit *picked = NULL;
> + char *ancestor_name = NULL;
> +
> + parent1 = pickme->parents->item;
> + parent2 = pickme->parents->next->item;
> +
> + /*
> + * Map the merge's parents to their replayed counterparts. With the
> + * boundary commits pre-seeded into `replayed_commits`, every parent
> + * either has an explicit mapping (rewritten or boundary -> onto) or
> + * sits outside the rewrite range entirely; the latter must stay at
> + * the original parent commit, so use `parent` itself as the fallback
> + * for both sides.
> + */
> + replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
> + replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
> +
> + /*
> + * R: auto-remerge of the original parents.
> + *
> + * Use the same branch labels for the inner merges that compute R
> + * and N so conflict markers (if any) are textually identical
> + * between the two; the outer non-recursive merge can then collapse
> + * the manual resolution from O against them.
> + */
> + init_basic_merge_options(&remerge_opt, repo);
> + remerge_opt.show_rename_progress = 0;
> + remerge_opt.branch1 = "ours";
> + remerge_opt.branch2 = "theirs";
> + if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0) {
> + result->clean = -1;
> + goto out;
> + }
> + merge_incore_recursive(&remerge_opt, parent_bases,
> + parent1, parent2, &remerge_res);
> + parent_bases = NULL; /* consumed by merge_incore_recursive */
> + if (remerge_res.clean < 0) {
> + result->clean = remerge_res.clean;
> + goto out;
> + }
> +
> + /* N: fresh merge of the (possibly rewritten) parents. */
> + init_basic_merge_options(&new_merge_opt, repo);
> + new_merge_opt.show_rename_progress = 0;
> + new_merge_opt.branch1 = "ours";
> + new_merge_opt.branch2 = "theirs";
> + if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
> + &replayed_bases) < 0) {
> + result->clean = -1;
> + goto out;
> + }
> + merge_incore_recursive(&new_merge_opt, replayed_bases,
> + replayed_par1, replayed_par2, &new_merge_res);
> + replayed_bases = NULL; /* consumed by merge_incore_recursive */
> + if (new_merge_res.clean < 0) {
> + result->clean = new_merge_res.clean;
> + goto out;
> + }
> +
> + /*
> + * Outer non-recursive merge: base=R, side1=O (pickme), side2=N.
> + */
> + pickme_tree = repo_get_commit_tree(repo, pickme);
> + ancestor_name = xstrfmt("auto-remerge of %s",
> + oid_to_hex(&pickme->object.oid));
> + merge_opt->ancestor = ancestor_name;
> + merge_opt->branch1 = short_commit_name(repo, pickme);
> + merge_opt->branch2 = "merge of replayed parents";
> + merge_incore_nonrecursive(merge_opt,
> + remerge_res.tree,
> + pickme_tree,
> + new_merge_res.tree,
> + result);
> + merge_opt->ancestor = NULL;
> + merge_opt->branch1 = NULL;
> + merge_opt->branch2 = NULL;
> + if (!result->clean)
> + goto out;
> +
> + parents = NULL;
> + commit_list_insert(replayed_par2, &parents);
> + commit_list_insert(replayed_par1, &parents);
> + picked = create_commit(repo, result->tree, pickme, parents,
> + REPLAY_MODE_PICK);
> +
> +out:
> + free(ancestor_name);
> + free_commit_list(parent_bases);
> + free_commit_list(replayed_bases);
> + merge_finalize(&remerge_opt, &remerge_res);
> + merge_finalize(&new_merge_opt, &new_merge_res);
> + return picked;
> }
> > void replay_result_release(struct replay_result *result)
> @@ -407,17 +550,63 @@ int replay_revisions(struct rev_info *revs,
> merge_opt.show_rename_progress = 0;
> last_commit = onto;
> replayed_commits = kh_init_oid_map();
> +
> + /*
> + * Seed the rewritten-commit map with each negative-side ("BOTTOM")
> + * cmdline entry pointing at `onto`. This matters for merge replay:
> + * a 2-parent merge whose first parent is the boundary (e.g. the
> + * commit being reworded) must replay onto the rewritten boundary,
> + * yet pick_merge_commit uses a self fallback so the second parent
> + * (a side branch outside the rewrite range) is preserved as-is.
> + * Pre-seeding the boundary disambiguates the two: in the map ->
> + * rewritten, missing -> kept as-is.
> + *
> + * Only do this for the pick path; revert mode chains reverts
> + * through last_commit and a pre-seeded boundary would short-circuit
> + * that chain.
> + */
> + if (mode == REPLAY_MODE_PICK) {
> + for (size_t i = 0; i < revs->cmdline.nr; i++) {
> + struct rev_cmdline_entry *e = &revs->cmdline.rev[i];
> + struct commit *boundary;
> + khint_t pos;
> + int hr;
> +
> + if (!(e->flags & BOTTOM))
> + continue;
> + boundary = lookup_commit_reference_gently(revs->repo,
> + &e->item->oid, 1);
> + if (!boundary)
> + continue;
> + pos = kh_put_oid_map(replayed_commits,
> + boundary->object.oid, &hr);
> + if (hr != 0)
> + kh_value(replayed_commits, pos) = onto;
> + }
> + }
> +
> while ((commit = get_revision(revs))) {
> const struct name_decoration *decoration;
> khint_t pos;
> int hr;
> > - if (commit->parents && commit->parents->next)
> - die(_("replaying merge commits is not supported yet!"));
> -
> - last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
> - mode == REPLAY_MODE_REVERT ? last_commit : onto,
> - &merge_opt, &result, mode);
> + if (commit->parents && commit->parents->next) {
> + if (commit->parents->next->next) {
> + ret = error(_("replaying octopus merges is not supported"));
> + goto out;
> + }
> + if (mode == REPLAY_MODE_REVERT) {
> + ret = error(_("reverting merge commits is not supported"));
> + goto out;
> + }
> + last_commit = pick_merge_commit(revs->repo, commit,
> + replayed_commits,
> + &merge_opt, &result);
> + } else {
> + last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
> + mode == REPLAY_MODE_REVERT ? last_commit : onto,
> + &merge_opt, &result, mode);
> + }
> if (!last_commit)
> break;
> > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> index de7b357685..d103f866a2 100755
> --- a/t/t3451-history-reword.sh
> +++ b/t/t3451-history-reword.sh
> @@ -201,12 +201,21 @@ test_expect_success 'can reword a merge commit' '
> git switch - &&
> git merge theirs &&
> > - # It is not possible to replay merge commits embedded in the
> - # history (yet).
> - test_must_fail git -c core.editor=false history reword HEAD~ 2>err &&
> - test_grep "replaying merge commits is not supported yet" err &&
> + # Reword a non-merge commit whose descendants include the
> + # merge: replay carries the merge through.
> + reword_with_message HEAD~ <<-EOF &&
> + ours reworded
> + EOF
> + expect_graph <<-EOF &&
> + * Merge tag ${SQ}theirs${SQ}
> + |\\
> + | * theirs
> + * | ours reworded
> + |/
> + * base
> + EOF
> > - # But it is possible to reword a merge commit directly.
> + # And reword a merge commit directly.
> reword_with_message HEAD <<-EOF &&
> Reworded merge commit
> EOF
> @@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' '
> * Reworded merge commit
> |\
> | * theirs
> - * | ours
> + * | ours reworded
> |/
> * base
> EOF
> diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
> index 8ed0cebb50..ad6309f98b 100755
> --- a/t/t3452-history-split.sh
> +++ b/t/t3452-history-split.sh
> @@ -36,7 +36,7 @@ expect_tree_entries () {
> test_cmp expect actual
> }
> > -test_expect_success 'refuses to work with merge commits' '
> +test_expect_success 'refuses to split a merge commit' '
> test_when_finished "rm -rf repo" &&
> git init repo &&
> (
> @@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge commits' '
> git switch - &&
> git merge theirs &&
> test_must_fail git history split HEAD 2>err &&
> - test_grep "cannot split up merge commit" err &&
> - test_must_fail git history split HEAD~ 2>err &&
> - test_grep "replaying merge commits is not supported yet" err
> + test_grep "cannot split up merge commit" err
> )
> '
> > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> index 3353bc4a4d..368b1b0f9a 100755
> --- a/t/t3650-replay-basics.sh
> +++ b/t/t3650-replay-basics.sh
> @@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ... ordering would be ill-defined' '
> test_cmp expect actual
> '
> > -test_expect_success 'replaying merge commits is not supported yet' '
> - echo "fatal: replaying merge commits is not supported yet!" >expect &&
> - test_must_fail git replay --advance=main main..topic-with-merge 2>actual &&
> - test_cmp expect actual
> +test_expect_success 'using replay to rebase a 2-parent merge' '
> + # main..topic-with-merge contains a 2-parent merge (P) introduced
> + # via test_merge. Use --ref-action=print so this test does not
> + # mutate state for subsequent tests in this file.
> + git replay --ref-action=print --onto main main..topic-with-merge >result &&
> + test_line_count = 1 result &&
> +
> + new_tip=$(cut -f 3 -d " " result) &&
> +
> + # Result is still a 2-parent merge.
> + git cat-file -p $new_tip >cat &&
> + grep -c "^parent " cat >count &&
> + echo 2 >expect &&
> + test_cmp expect count &&
> +
> + # Merge subject is preserved.
> + echo P >expect &&
> + git log -1 --format=%s $new_tip >actual &&
> + test_cmp expect actual &&
> +
> + # The replayed merge sits on top of main: walking back via the
> + # first-parent chain reaches main.
> + git merge-base --is-ancestor main $new_tip
> +'
> +
> +test_expect_success 'replaying an octopus merge is rejected' '
> + # Build an octopus side-branch so the rest of the test state stays
> + # untouched.
> + test_when_finished "git update-ref -d refs/heads/octopus-tip" &&
> + octopus_tip=$(git commit-tree -p topic4 -p topic1 -p topic3 \
> + -m "octopus" $(git rev-parse topic4^{tree})) &&
> + git update-ref refs/heads/octopus-tip "$octopus_tip" &&
> +
> + test_must_fail git replay --ref-action=print --onto main \
> + topic4..octopus-tip 2>actual &&
> + test_grep "octopus merges" actual
> +'
> +
> +test_expect_success 'reverting a merge commit is rejected' '
> + test_must_fail git replay --ref-action=print --revert=topic-with-merge \
> + topic4..topic-with-merge 2>actual &&
> + test_grep "reverting merge commits" actual
> '
> > test_expect_success 'using replay to rebase two branches, one on top of other' 'There was a problem hiding this comment.
Phillip Wood wrote on the Git mailing list (how to reply to this email):
On 08/05/2026 10:36, Phillip Wood wrote:
> Hi Johannes
> > On 06/05/2026 23:43, Johannes Schindelin via GitGitGadget wrote:
>>
>> Elijah Newren spelled out a way to lift this limitation in his
>> replay-design-notes [1] and prototyped it in a 2022
>> work-in-progress sketch [2]. The idea is that a merge commit M on
>> parents (P1, P2) records both an automatic merge of those parents
>> AND any manual layer the author put on top of that automatic merge
>> (textual conflict resolution and any semantic edit outside conflict
>> markers). Replaying M onto rewritten parents (P1', P2') must
>> preserve that manual layer, but the rewritten parents change the
>> automatic merge, so a simple cherry-pick is wrong: the manual layer
>> would be re-introduced on top of stale auto-merge text.
>>
>> What works instead is a three-way merge of three trees the existing
>> infrastructure already knows how to compute. Let R be the recursive
>> auto-merge of (P1, P2), O be M's actual tree and N be the recursive
>> auto-merge of (P1', P2'). Then `git diff R O` is morally
>> `git show --remerge-diff M`: it captures exactly what the author
>> added on top of the automatic merge. A non-recursive 3-way merge
>> with R as the merge base, O as side 1 and N as side 2 layers that
>> manual contribution onto the freshly auto-merged rewritten parents
>> (N) and produces the replayed tree.
> > So we cherry-pick the difference between the user's conflict resolution > O and the auto-merge M of the original parents onto the auto-merge N of > the replayed parents. If we have a topology that looks like
> > |
> A
> /|\
> / B \
> E | D
> C /
> |/
> O
> > then running
> > git replay --onto E --ancestry-path B..O
> > will replay C and O onto E. If the changes in E and D conflict but those > conflicts do not overlap with the conflicts in M that were resolved to > create O then the replayed version of O will contain conflict markers > from the conflicting changes in E and D. Because the previous conflict > resolution applies to N without conflicts we do not recognize that there > are still conflicts in N that need to be resolved.
> > Having realized this I went to look at Elijah's notes and they recognize > this possibility and suggest extending the xdiff merge code to detect > when N has conflicts that do not correspond to the conflicts in M. That > sounds like quite a lot of work. I've not put much effort into coming up > with a counterexample but think that because "git replay" and "git > history" do not yet allow the commits in the merged branches to be > edited we may be able to safely use the implementation proposed in this > series if both merge parents have been rebased (or we might want all the > merge bases of the new merge to be a descendants of "--onto"). In the > example above if both the parents were rebased onto E then any new > conflicts would happen when picking D rather than when recreating the > merge.
One further thought - if only one of the parents has been rebased (i.e. we're replaying O with parents P1' and P2) then can we just cherry-pick the merge - instead of merging P1' and P2, use P1 as the merge-base with O and P1' as the merge heads?
Thanks
Phillip
> Thanks
> > Phillip
> >> Implement `pick_merge_commit()` along those lines and dispatch to it
>> from `replay_revisions()` when the commit being replayed has exactly
>> two parents. Two specific points (learned the hard way) keep
>> non-trivial cases working where the WIP sketch [2] bailed out.
>> First, R and N use identical `merge_options.branch1` and `branch2`
>> labels ("ours"/"theirs"). When the original parents conflicted on a
>> region of a file, both R and N produce textually identical conflict
>> markers; the outer non-recursive merge then sees N == R in that
>> region and the user's manual resolution from O wins cleanly. Without
>> this, the conflict-marker text would differ between R and N (because
>> the inner merges would label the conflicts differently), and the
>> outer merge would itself be unclean even when the user did supply a
>> clean resolution. Second, an unclean inner merge
>> (`result.clean == 0`) is _not_ fatal: the tree merge-ort produces in
>> that case still has well-defined contents (with conflict markers in
>> the conflicted files) and is a valid input to the outer
>> non-recursive merge. Only a real error (`< 0`) propagates as
>> failure.
>>
>> The replay propagates the textual diffs the user actually made in M;
>> it does _not_ extrapolate symbol-level intent. If rewriting the
>> parents pulls in genuinely new content (for example, a brand-new
>> caller of a function that the merge renamed), that new content stays
>> as the rewritten parents have it. Symbol-aware refactoring is out of
>> scope here, just as it is for plain rebase.
>>
>> Octopus merges (more than two parents) and revert-of-merge are not
>> supported and are surfaced as explicit errors at the dispatch point.
>> The "split" sub-command of `git history` continues to refuse when
>> the targeted commit is itself a merge: split semantics do not apply
>> to merges. The pre-walk gate in `builtin/history.c` that previously
>> rejected any merge in the rewrite path now only rejects octopus
>> merges; rename it accordingly.
>>
>> A small refactor in `create_commit()` makes the merge case possible:
>> the helper now takes a `struct commit_list *parents` rather than a
>> single parent pointer and takes ownership of the list. The single
>> existing caller in `pick_regular_commit()` builds and passes a
>> one-element list; the new `pick_merge_commit()` builds a two-element
>> list, with the order of the `from` and `merge` parents preserved.
>>
>> Update the negative expectations in t3451, t3452 and t3650 that were
>> asserting the now-retired "not supported yet" message, replacing
>> them with positive coverage where it fits. Octopus rejection and
>> revert-of-merge rejection are covered by new positive tests in
>> t3650. A dedicated test script with merge-replay scenarios driven by
>> a new test-tool fixture builder will follow in a subsequent commit.
>>
>> [1] https://github.com/newren/git/blob/replay/replay-design-notes.txt
>> [2] https://github.com/newren/git/ >> commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45
>>
>> Helped-by: Elijah Newren <newren@gmail.com>
>> Assisted-by: Claude Opus 4.7
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>> builtin/history.c | 16 ++-
>> replay.c | 209 ++++++++++++++++++++++++++++++++++++--
>> t/t3451-history-reword.sh | 21 ++--
>> t/t3452-history-split.sh | 6 +-
>> t/t3650-replay-basics.sh | 46 ++++++++-
>> 5 files changed, 269 insertions(+), 29 deletions(-)
>>
>> diff --git a/builtin/history.c b/builtin/history.c
>> index 9526938085..00097b2226 100644
>> --- a/builtin/history.c
>> +++ b/builtin/history.c
>> @@ -195,15 +195,15 @@ static int parse_ref_action(const struct option >> *opt, const char *value, int uns
>> return 0;
>> }
>> -static int revwalk_contains_merges(struct repository *repo,
>> - const struct strvec *revwalk_args)
>> +static int revwalk_contains_octopus_merges(struct repository *repo,
>> + const struct strvec *revwalk_args)
>> {
>> struct strvec args = STRVEC_INIT;
>> struct rev_info revs;
>> int ret;
>> strvec_pushv(&args, revwalk_args->v);
>> - strvec_push(&args, "--min-parents=2");
>> + strvec_push(&args, "--min-parents=3");
>> repo_init_revisions(repo, &revs, NULL);
>> @@ -217,7 +217,7 @@ static int revwalk_contains_merges(struct >> repository *repo,
>> }
>> if (get_revision(&revs)) {
>> - ret = error(_("replaying merge commits is not supported yet!"));
>> + ret = error(_("replaying octopus merges is not supported"));
>> goto out;
>> }
>> @@ -289,7 +289,7 @@ static int setup_revwalk(struct repository *repo,
>> strvec_push(&args, "HEAD");
>> }
>> - ret = revwalk_contains_merges(repo, &args);
>> + ret = revwalk_contains_octopus_merges(repo, &args);
>> if (ret < 0)
>> goto out;
>> @@ -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;
>> diff --git a/replay.c b/replay.c
>> index f96f1f6551..3dbce095f9 100644
>> --- a/replay.c
>> +++ b/replay.c
>> @@ -1,6 +1,7 @@
>> #define USE_THE_REPOSITORY_VARIABLE
>> #include "git-compat-util.h"
>> +#include "commit-reach.h"
>> #include "environment.h"
>> #include "hex.h"
>> #include "merge-ort.h"
>> @@ -77,15 +78,21 @@ static void generate_revert_message(struct strbuf >> *msg,
>> repo_unuse_commit_buffer(repo, commit, message);
>> }
>> +/*
>> + * Build a new commit with the given tree and parent list, copying >> author,
>> + * extra headers and (for pick mode) the commit message from `based_on`.
>> + *
>> + * Takes ownership of `parents`: it will be freed before returning, >> even on
>> + * error. Parent order is preserved as supplied by the caller.
>> + */
>> static struct commit *create_commit(struct repository *repo,
>> struct tree *tree,
>> struct commit *based_on,
>> - struct commit *parent,
>> + struct commit_list *parents,
>> enum replay_mode mode)
>> {
>> struct object_id ret;
>> struct object *obj = NULL;
>> - struct commit_list *parents = NULL;
>> char *author = NULL;
>> char *sign_commit = NULL; /* FIXME: cli users might want to sign >> again */
>> struct commit_extra_header *extra = NULL;
>> @@ -96,7 +103,6 @@ static struct commit *create_commit(struct >> repository *repo,
>> const char *orig_message = NULL;
>> const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
>> - commit_list_insert(parent, &parents);
>> extra = read_commit_extra_headers(based_on, exclude_gpgsig);
>> if (mode == REPLAY_MODE_REVERT) {
>> generate_revert_message(&msg, based_on, repo);
>> @@ -273,6 +279,7 @@ static struct commit *pick_regular_commit(struct >> repository *repo,
>> {
>> struct commit *base, *replayed_base;
>> struct tree *pickme_tree, *base_tree, *replayed_base_tree;
>> + struct commit_list *parents = NULL;
>> if (pickme->parents) {
>> base = pickme->parents->item;
>> @@ -327,7 +334,143 @@ static struct commit *pick_regular_commit(struct >> repository *repo,
>> if (oideq(&replayed_base_tree->object.oid, &result->tree- >> >object.oid) &&
>> !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
>> return replayed_base;
>> - return create_commit(repo, result->tree, pickme, replayed_base, >> mode);
>> + commit_list_insert(replayed_base, &parents);
>> + return create_commit(repo, result->tree, pickme, parents, mode);
>> +}
>> +
>> +/*
>> + * Replay a 2-parent merge commit by composing three calls into >> merge-ort:
>> + *
>> + * R = recursive merge of pickme's two original parents (auto- >> remerge of
>> + * the original merge, accepting any conflicts)
>> + * N = recursive merge of the (possibly rewritten) parents
>> + * O = pickme's tree (the user's actual merge, including any manual
>> + * resolutions)
>> + *
>> + * The picked tree comes from a non-recursive merge using R as the base,
>> + * O as side1 and N as side2. `git diff R O` is morally `git show
>> + * --remerge-diff $oldmerge`, so this layers the user's original manual
>> + * resolution on top of the freshly auto-merged rewritten parents (see
>> + * `replay-design-notes.txt` on the `replay` branch of newren/git).
>> + *
>> + * If the outer 3-way merge is unclean, propagate the conflict status to
>> + * the caller via `result->clean = 0` and return NULL. The two inner
>> + * merges (R and N) being unclean is _not_ fatal: the conflict-markered
>> + * trees they produce are valid inputs to the outer merge, and using
>> + * identical labels for both inner merges keeps the marker text
>> + * byte-equal between R and N so the user's resolution recorded in O
>> + * collapses the conflict cleanly there. Octopus merges (more than two
>> + * parents) and revert-of-merge are rejected by the caller before this
>> + * function is invoked.
>> + */
>> +static struct commit *pick_merge_commit(struct repository *repo,
>> + struct commit *pickme,
>> + kh_oid_map_t *replayed_commits,
>> + struct merge_options *merge_opt,
>> + struct merge_result *result)
>> +{
>> + struct commit *parent1, *parent2;
>> + struct commit *replayed_par1, *replayed_par2;
>> + struct tree *pickme_tree;
>> + struct merge_options remerge_opt = { 0 };
>> + struct merge_options new_merge_opt = { 0 };
>> + struct merge_result remerge_res = { 0 };
>> + struct merge_result new_merge_res = { 0 };
>> + struct commit_list *parent_bases = NULL;
>> + struct commit_list *replayed_bases = NULL;
>> + struct commit_list *parents;
>> + struct commit *picked = NULL;
>> + char *ancestor_name = NULL;
>> +
>> + parent1 = pickme->parents->item;
>> + parent2 = pickme->parents->next->item;
>> +
>> + /*
>> + * Map the merge's parents to their replayed counterparts. With the
>> + * boundary commits pre-seeded into `replayed_commits`, every parent
>> + * either has an explicit mapping (rewritten or boundary -> onto) or
>> + * sits outside the rewrite range entirely; the latter must stay at
>> + * the original parent commit, so use `parent` itself as the >> fallback
>> + * for both sides.
>> + */
>> + replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
>> + replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
>> +
>> + /*
>> + * R: auto-remerge of the original parents.
>> + *
>> + * Use the same branch labels for the inner merges that compute R
>> + * and N so conflict markers (if any) are textually identical
>> + * between the two; the outer non-recursive merge can then collapse
>> + * the manual resolution from O against them.
>> + */
>> + init_basic_merge_options(&remerge_opt, repo);
>> + remerge_opt.show_rename_progress = 0;
>> + remerge_opt.branch1 = "ours";
>> + remerge_opt.branch2 = "theirs";
>> + if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < >> 0) {
>> + result->clean = -1;
>> + goto out;
>> + }
>> + merge_incore_recursive(&remerge_opt, parent_bases,
>> + parent1, parent2, &remerge_res);
>> + parent_bases = NULL; /* consumed by merge_incore_recursive */
>> + if (remerge_res.clean < 0) {
>> + result->clean = remerge_res.clean;
>> + goto out;
>> + }
>> +
>> + /* N: fresh merge of the (possibly rewritten) parents. */
>> + init_basic_merge_options(&new_merge_opt, repo);
>> + new_merge_opt.show_rename_progress = 0;
>> + new_merge_opt.branch1 = "ours";
>> + new_merge_opt.branch2 = "theirs";
>> + if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
>> + &replayed_bases) < 0) {
>> + result->clean = -1;
>> + goto out;
>> + }
>> + merge_incore_recursive(&new_merge_opt, replayed_bases,
>> + replayed_par1, replayed_par2, &new_merge_res);
>> + replayed_bases = NULL; /* consumed by merge_incore_recursive */
>> + if (new_merge_res.clean < 0) {
>> + result->clean = new_merge_res.clean;
>> + goto out;
>> + }
>> +
>> + /*
>> + * Outer non-recursive merge: base=R, side1=O (pickme), side2=N.
>> + */
>> + pickme_tree = repo_get_commit_tree(repo, pickme);
>> + ancestor_name = xstrfmt("auto-remerge of %s",
>> + oid_to_hex(&pickme->object.oid));
>> + merge_opt->ancestor = ancestor_name;
>> + merge_opt->branch1 = short_commit_name(repo, pickme);
>> + merge_opt->branch2 = "merge of replayed parents";
>> + merge_incore_nonrecursive(merge_opt,
>> + remerge_res.tree,
>> + pickme_tree,
>> + new_merge_res.tree,
>> + result);
>> + merge_opt->ancestor = NULL;
>> + merge_opt->branch1 = NULL;
>> + merge_opt->branch2 = NULL;
>> + if (!result->clean)
>> + goto out;
>> +
>> + parents = NULL;
>> + commit_list_insert(replayed_par2, &parents);
>> + commit_list_insert(replayed_par1, &parents);
>> + picked = create_commit(repo, result->tree, pickme, parents,
>> + REPLAY_MODE_PICK);
>> +
>> +out:
>> + free(ancestor_name);
>> + free_commit_list(parent_bases);
>> + free_commit_list(replayed_bases);
>> + merge_finalize(&remerge_opt, &remerge_res);
>> + merge_finalize(&new_merge_opt, &new_merge_res);
>> + return picked;
>> }
>> void replay_result_release(struct replay_result *result)
>> @@ -407,17 +550,63 @@ int replay_revisions(struct rev_info *revs,
>> merge_opt.show_rename_progress = 0;
>> last_commit = onto;
>> replayed_commits = kh_init_oid_map();
>> +
>> + /*
>> + * Seed the rewritten-commit map with each negative-side ("BOTTOM")
>> + * cmdline entry pointing at `onto`. This matters for merge replay:
>> + * a 2-parent merge whose first parent is the boundary (e.g. the
>> + * commit being reworded) must replay onto the rewritten boundary,
>> + * yet pick_merge_commit uses a self fallback so the second parent
>> + * (a side branch outside the rewrite range) is preserved as-is.
>> + * Pre-seeding the boundary disambiguates the two: in the map ->
>> + * rewritten, missing -> kept as-is.
>> + *
>> + * Only do this for the pick path; revert mode chains reverts
>> + * through last_commit and a pre-seeded boundary would short-circuit
>> + * that chain.
>> + */
>> + if (mode == REPLAY_MODE_PICK) {
>> + for (size_t i = 0; i < revs->cmdline.nr; i++) {
>> + struct rev_cmdline_entry *e = &revs->cmdline.rev[i];
>> + struct commit *boundary;
>> + khint_t pos;
>> + int hr;
>> +
>> + if (!(e->flags & BOTTOM))
>> + continue;
>> + boundary = lookup_commit_reference_gently(revs->repo,
>> + &e->item->oid, 1);
>> + if (!boundary)
>> + continue;
>> + pos = kh_put_oid_map(replayed_commits,
>> + boundary->object.oid, &hr);
>> + if (hr != 0)
>> + kh_value(replayed_commits, pos) = onto;
>> + }
>> + }
>> +
>> while ((commit = get_revision(revs))) {
>> const struct name_decoration *decoration;
>> khint_t pos;
>> int hr;
>> - if (commit->parents && commit->parents->next)
>> - die(_("replaying merge commits is not supported yet!"));
>> -
>> - last_commit = pick_regular_commit(revs->repo, commit, >> replayed_commits,
>> - mode == REPLAY_MODE_REVERT ? last_commit : >> onto,
>> - &merge_opt, &result, mode);
>> + if (commit->parents && commit->parents->next) {
>> + if (commit->parents->next->next) {
>> + ret = error(_("replaying octopus merges is not >> supported"));
>> + goto out;
>> + }
>> + if (mode == REPLAY_MODE_REVERT) {
>> + ret = error(_("reverting merge commits is not >> supported"));
>> + goto out;
>> + }
>> + last_commit = pick_merge_commit(revs->repo, commit,
>> + replayed_commits,
>> + &merge_opt, &result);
>> + } else {
>> + last_commit = pick_regular_commit(revs->repo, commit, >> replayed_commits,
>> + mode == REPLAY_MODE_REVERT ? >> last_commit : onto,
>> + &merge_opt, &result, mode);
>> + }
>> if (!last_commit)
>> break;
>> diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
>> index de7b357685..d103f866a2 100755
>> --- a/t/t3451-history-reword.sh
>> +++ b/t/t3451-history-reword.sh
>> @@ -201,12 +201,21 @@ test_expect_success 'can reword a merge commit' '
>> git switch - &&
>> git merge theirs &&
>> - # It is not possible to replay merge commits embedded in the
>> - # history (yet).
>> - test_must_fail git -c core.editor=false history reword HEAD~ >> 2>err &&
>> - test_grep "replaying merge commits is not supported yet" err &&
>> + # Reword a non-merge commit whose descendants include the
>> + # merge: replay carries the merge through.
>> + reword_with_message HEAD~ <<-EOF &&
>> + ours reworded
>> + EOF
>> + expect_graph <<-EOF &&
>> + * Merge tag ${SQ}theirs${SQ}
>> + |\\
>> + | * theirs
>> + * | ours reworded
>> + |/
>> + * base
>> + EOF
>> - # But it is possible to reword a merge commit directly.
>> + # And reword a merge commit directly.
>> reword_with_message HEAD <<-EOF &&
>> Reworded merge commit
>> EOF
>> @@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' '
>> * Reworded merge commit
>> |\
>> | * theirs
>> - * | ours
>> + * | ours reworded
>> |/
>> * base
>> EOF
>> diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
>> index 8ed0cebb50..ad6309f98b 100755
>> --- a/t/t3452-history-split.sh
>> +++ b/t/t3452-history-split.sh
>> @@ -36,7 +36,7 @@ expect_tree_entries () {
>> test_cmp expect actual
>> }
>> -test_expect_success 'refuses to work with merge commits' '
>> +test_expect_success 'refuses to split a merge commit' '
>> test_when_finished "rm -rf repo" &&
>> git init repo &&
>> (
>> @@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge >> commits' '
>> git switch - &&
>> git merge theirs &&
>> test_must_fail git history split HEAD 2>err &&
>> - test_grep "cannot split up merge commit" err &&
>> - test_must_fail git history split HEAD~ 2>err &&
>> - test_grep "replaying merge commits is not supported yet" err
>> + test_grep "cannot split up merge commit" err
>> )
>> '
>> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
>> index 3353bc4a4d..368b1b0f9a 100755
>> --- a/t/t3650-replay-basics.sh
>> +++ b/t/t3650-replay-basics.sh
>> @@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ... >> ordering would be ill-defined' '
>> test_cmp expect actual
>> '
>> -test_expect_success 'replaying merge commits is not supported yet' '
>> - echo "fatal: replaying merge commits is not supported yet!" >> >expect &&
>> - test_must_fail git replay --advance=main main..topic-with-merge >> 2>actual &&
>> - test_cmp expect actual
>> +test_expect_success 'using replay to rebase a 2-parent merge' '
>> + # main..topic-with-merge contains a 2-parent merge (P) introduced
>> + # via test_merge. Use --ref-action=print so this test does not
>> + # mutate state for subsequent tests in this file.
>> + git replay --ref-action=print --onto main main..topic-with-merge >> >result &&
>> + test_line_count = 1 result &&
>> +
>> + new_tip=$(cut -f 3 -d " " result) &&
>> +
>> + # Result is still a 2-parent merge.
>> + git cat-file -p $new_tip >cat &&
>> + grep -c "^parent " cat >count &&
>> + echo 2 >expect &&
>> + test_cmp expect count &&
>> +
>> + # Merge subject is preserved.
>> + echo P >expect &&
>> + git log -1 --format=%s $new_tip >actual &&
>> + test_cmp expect actual &&
>> +
>> + # The replayed merge sits on top of main: walking back via the
>> + # first-parent chain reaches main.
>> + git merge-base --is-ancestor main $new_tip
>> +'
>> +
>> +test_expect_success 'replaying an octopus merge is rejected' '
>> + # Build an octopus side-branch so the rest of the test state stays
>> + # untouched.
>> + test_when_finished "git update-ref -d refs/heads/octopus-tip" &&
>> + octopus_tip=$(git commit-tree -p topic4 -p topic1 -p topic3 \
>> + -m "octopus" $(git rev-parse topic4^{tree})) &&
>> + git update-ref refs/heads/octopus-tip "$octopus_tip" &&
>> +
>> + test_must_fail git replay --ref-action=print --onto main \
>> + topic4..octopus-tip 2>actual &&
>> + test_grep "octopus merges" actual
>> +'
>> +
>> +test_expect_success 'reverting a merge commit is rejected' '
>> + test_must_fail git replay --ref-action=print --revert=topic-with- >> merge \
>> + topic4..topic-with-merge 2>actual &&
>> + test_grep "reverting merge commits" actual
>> '
>> test_expect_success 'using replay to rebase two branches, one on top >> of other' '
> There was a problem hiding this comment.
Johannes Schindelin wrote on the Git mailing list (how to reply to this email):
Hi Phillip,
On Fri, 8 May 2026, Phillip Wood wrote:
> On 06/05/2026 23:43, Johannes Schindelin via GitGitGadget wrote:
> >
> > Elijah Newren spelled out a way to lift this limitation in his
> > replay-design-notes [1] and prototyped it in a 2022
> > work-in-progress sketch [2]. The idea is that a merge commit M on
> > parents (P1, P2) records both an automatic merge of those parents
> > AND any manual layer the author put on top of that automatic merge
> > (textual conflict resolution and any semantic edit outside conflict
> > markers). Replaying M onto rewritten parents (P1', P2') must
> > preserve that manual layer, but the rewritten parents change the
> > automatic merge, so a simple cherry-pick is wrong: the manual layer
> > would be re-introduced on top of stale auto-merge text.
> >
> > What works instead is a three-way merge of three trees the existing
> > infrastructure already knows how to compute. Let R be the recursive
> > auto-merge of (P1, P2), O be M's actual tree and N be the recursive
> > auto-merge of (P1', P2'). Then `git diff R O` is morally
> > `git show --remerge-diff M`: it captures exactly what the author
> > added on top of the automatic merge. A non-recursive 3-way merge
> > with R as the merge base, O as side 1 and N as side 2 layers that
> > manual contribution onto the freshly auto-merged rewritten parents
> > (N) and produces the replayed tree.
>
> So we cherry-pick the difference between the user's conflict resolution O and
> the auto-merge M of the original parents onto the auto-merge N of the replayed
> parents. If we have a topology that looks like
>
> |
> A
> /|\
> / B \
> E | D
> C /
> |/
> O
>
> then running
>
> git replay --onto E --ancestry-path B..O
>
> will replay C and O onto E. If the changes in E and D conflict but those
> conflicts do not overlap with the conflicts in M that were resolved to create
> O then the replayed version of O will contain conflict markers from the
> conflicting changes in E and D. Because the previous conflict resolution
> applies to N without conflicts we do not recognize that there are still
> conflicts in N that need to be resolved.
Very good point, and exactly the kind of feedback I was hoping for when I
marked this as an RFC. Thank you!
> Having realized this I went to look at Elijah's notes and they recognize
> this possibility and suggest extending the xdiff merge code to detect
> when N has conflicts that do not correspond to the conflicts in M. That
> sounds like quite a lot of work. I've not put much effort into coming up
> with a counterexample but think that because "git replay" and "git
> history" do not yet allow the commits in the merged branches to be
> edited we may be able to safely use the implementation proposed in this
> series if both merge parents have been rebased (or we might want all the
> merge bases of the new merge to be a descendants of "--onto"). In the
> example above if both the parents were rebased onto E then any new
> conflicts would happen when picking D rather than when recreating the
> merge.
Right. I have to admit that I missed this corner-case when I looked at the
original notes.
And while `git history`'s `reword` and `split` subcommands won't be
affected, the upcoming `fixup` subcommand _will_ be affected.
I am reworking the patches as we speak, loosely following Elijah's notes.
So far, I'm confident that this will address that problem.
What I am not confident at all so far (because I'm still trying to get the
actual algorithm to work, and haven't had a chance to test this on
real-world scenarios) is that the _conflict output_ is helpful. That is,
whether the conflict markers in case of corner-cases (merge conflicts in
R overlapping with merge conflicts in N, but not being identical, for
example) are clear enough to act upon, or will only lead to despair in the
keen reader.
For example, I noticed that a merge conflict resolution in O that is no
longer necessary in N leads to a quite unhelpful output...
I know that `git replay` is not designed as an interactive tool, but `git
history` is, and will ultimately _have_ to find ways to surface such merge
conflicts and help the user resolve them and then continue the replay.
For now, however, I do agree that we need to capture the error modes
correctly.
Ciao,
Johannes
>
> Thanks
>
> Phillip
>
> > Implement `pick_merge_commit()` along those lines and dispatch to it
> > from `replay_revisions()` when the commit being replayed has exactly
> > two parents. Two specific points (learned the hard way) keep
> > non-trivial cases working where the WIP sketch [2] bailed out.
> > First, R and N use identical `merge_options.branch1` and `branch2`
> > labels ("ours"/"theirs"). When the original parents conflicted on a
> > region of a file, both R and N produce textually identical conflict
> > markers; the outer non-recursive merge then sees N == R in that
> > region and the user's manual resolution from O wins cleanly. Without
> > this, the conflict-marker text would differ between R and N (because
> > the inner merges would label the conflicts differently), and the
> > outer merge would itself be unclean even when the user did supply a
> > clean resolution. Second, an unclean inner merge
> > (`result.clean == 0`) is _not_ fatal: the tree merge-ort produces in
> > that case still has well-defined contents (with conflict markers in
> > the conflicted files) and is a valid input to the outer
> > non-recursive merge. Only a real error (`< 0`) propagates as
> > failure.
> >
> > The replay propagates the textual diffs the user actually made in M;
> > it does _not_ extrapolate symbol-level intent. If rewriting the
> > parents pulls in genuinely new content (for example, a brand-new
> > caller of a function that the merge renamed), that new content stays
> > as the rewritten parents have it. Symbol-aware refactoring is out of
> > scope here, just as it is for plain rebase.
> >
> > Octopus merges (more than two parents) and revert-of-merge are not
> > supported and are surfaced as explicit errors at the dispatch point.
> > The "split" sub-command of `git history` continues to refuse when
> > the targeted commit is itself a merge: split semantics do not apply
> > to merges. The pre-walk gate in `builtin/history.c` that previously
> > rejected any merge in the rewrite path now only rejects octopus
> > merges; rename it accordingly.
> >
> > A small refactor in `create_commit()` makes the merge case possible:
> > the helper now takes a `struct commit_list *parents` rather than a
> > single parent pointer and takes ownership of the list. The single
> > existing caller in `pick_regular_commit()` builds and passes a
> > one-element list; the new `pick_merge_commit()` builds a two-element
> > list, with the order of the `from` and `merge` parents preserved.
> >
> > Update the negative expectations in t3451, t3452 and t3650 that were
> > asserting the now-retired "not supported yet" message, replacing
> > them with positive coverage where it fits. Octopus rejection and
> > revert-of-merge rejection are covered by new positive tests in
> > t3650. A dedicated test script with merge-replay scenarios driven by
> > a new test-tool fixture builder will follow in a subsequent commit.
> >
> > [1] https://github.com/newren/git/blob/replay/replay-design-notes.txt
> > [2]
> > https://github.com/newren/git/commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45
> >
> > Helped-by: Elijah Newren <newren@gmail.com>
> > Assisted-by: Claude Opus 4.7
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/history.c | 16 ++-
> > replay.c | 209 ++++++++++++++++++++++++++++++++++++--
> > t/t3451-history-reword.sh | 21 ++--
> > t/t3452-history-split.sh | 6 +-
> > t/t3650-replay-basics.sh | 46 ++++++++-
> > 5 files changed, 269 insertions(+), 29 deletions(-)
> >
> > diff --git a/builtin/history.c b/builtin/history.c
> > index 9526938085..00097b2226 100644
> > --- a/builtin/history.c
> > +++ b/builtin/history.c
> > @@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt,
> > const char *value, int uns
> > return 0;
> > }
> >
> > -static int revwalk_contains_merges(struct repository *repo,
> > - const struct strvec *revwalk_args)
> > +static int revwalk_contains_octopus_merges(struct repository *repo,
> > + const struct strvec *revwalk_args)
> > {
> > struct strvec args = STRVEC_INIT;
> > struct rev_info revs;
> > int ret;
> >
> > strvec_pushv(&args, revwalk_args->v);
> > - strvec_push(&args, "--min-parents=2");
> > + strvec_push(&args, "--min-parents=3");
> >
> > repo_init_revisions(repo, &revs, NULL);
> > @@ -217,7 +217,7 @@ static int revwalk_contains_merges(struct repository
> > *repo,
> > }
> >
> > if (get_revision(&revs)) {
> > - ret = error(_("replaying merge commits is not supported
> > yet!"));
> > + ret = error(_("replaying octopus merges is not supported"));
> > goto out;
> > }
> > @@ -289,7 +289,7 @@ static int setup_revwalk(struct repository *repo,
> > strvec_push(&args, "HEAD");
> > }
> > - ret = revwalk_contains_merges(repo, &args);
> > + ret = revwalk_contains_octopus_merges(repo, &args);
> > if (ret < 0)
> > goto out;
> > @@ -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;
> > diff --git a/replay.c b/replay.c
> > index f96f1f6551..3dbce095f9 100644
> > --- a/replay.c
> > +++ b/replay.c
> > @@ -1,6 +1,7 @@
> > #define USE_THE_REPOSITORY_VARIABLE
> >
> > #include "git-compat-util.h"
> > +#include "commit-reach.h"
> > #include "environment.h"
> > #include "hex.h"
> > #include "merge-ort.h"
> > @@ -77,15 +78,21 @@ static void generate_revert_message(struct strbuf *msg,
> > repo_unuse_commit_buffer(repo, commit, message);
> > }
> >
> > +/*
> > + * Build a new commit with the given tree and parent list, copying author,
> > + * extra headers and (for pick mode) the commit message from `based_on`.
> > + *
> > + * Takes ownership of `parents`: it will be freed before returning, even on
> > + * error. Parent order is preserved as supplied by the caller.
> > + */
> > static struct commit *create_commit(struct repository *repo,
> > struct tree *tree,
> > struct commit *based_on,
> > - struct commit *parent,
> > + struct commit_list *parents,
> > enum replay_mode mode)
> > {
> > struct object_id ret;
> > struct object *obj = NULL;
> > - struct commit_list *parents = NULL;
> > char *author = NULL;
> > char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
> > struct commit_extra_header *extra = NULL;
> > @@ -96,7 +103,6 @@ static struct commit *create_commit(struct repository
> > *repo,
> > const char *orig_message = NULL;
> > const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
> > - commit_list_insert(parent, &parents);
> > extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> > if (mode == REPLAY_MODE_REVERT) {
> > generate_revert_message(&msg, based_on, repo);
> > @@ -273,6 +279,7 @@ static struct commit *pick_regular_commit(struct
> > repository *repo,
> > {
> > struct commit *base, *replayed_base;
> > struct tree *pickme_tree, *base_tree, *replayed_base_tree;
> > + struct commit_list *parents = NULL;
> >
> > if (pickme->parents) {
> > base = pickme->parents->item;
> > @@ -327,7 +334,143 @@ static struct commit *pick_regular_commit(struct
> > repository *repo,
> > if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
> > !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
> > return replayed_base;
> > - return create_commit(repo, result->tree, pickme, replayed_base, mode);
> > + commit_list_insert(replayed_base, &parents);
> > + return create_commit(repo, result->tree, pickme, parents, mode);
> > +}
> > +
> > +/*
> > + * Replay a 2-parent merge commit by composing three calls into merge-ort:
> > + *
> > + * R = recursive merge of pickme's two original parents (auto-remerge of
> > + * the original merge, accepting any conflicts)
> > + * N = recursive merge of the (possibly rewritten) parents
> > + * O = pickme's tree (the user's actual merge, including any manual
> > + * resolutions)
> > + *
> > + * The picked tree comes from a non-recursive merge using R as the base,
> > + * O as side1 and N as side2. `git diff R O` is morally `git show
> > + * --remerge-diff $oldmerge`, so this layers the user's original manual
> > + * resolution on top of the freshly auto-merged rewritten parents (see
> > + * `replay-design-notes.txt` on the `replay` branch of newren/git).
> > + *
> > + * If the outer 3-way merge is unclean, propagate the conflict status to
> > + * the caller via `result->clean = 0` and return NULL. The two inner
> > + * merges (R and N) being unclean is _not_ fatal: the conflict-markered
> > + * trees they produce are valid inputs to the outer merge, and using
> > + * identical labels for both inner merges keeps the marker text
> > + * byte-equal between R and N so the user's resolution recorded in O
> > + * collapses the conflict cleanly there. Octopus merges (more than two
> > + * parents) and revert-of-merge are rejected by the caller before this
> > + * function is invoked.
> > + */
> > +static struct commit *pick_merge_commit(struct repository *repo,
> > + struct commit *pickme,
> > + kh_oid_map_t *replayed_commits,
> > + struct merge_options *merge_opt,
> > + struct merge_result *result)
> > +{
> > + struct commit *parent1, *parent2;
> > + struct commit *replayed_par1, *replayed_par2;
> > + struct tree *pickme_tree;
> > + struct merge_options remerge_opt = { 0 };
> > + struct merge_options new_merge_opt = { 0 };
> > + struct merge_result remerge_res = { 0 };
> > + struct merge_result new_merge_res = { 0 };
> > + struct commit_list *parent_bases = NULL;
> > + struct commit_list *replayed_bases = NULL;
> > + struct commit_list *parents;
> > + struct commit *picked = NULL;
> > + char *ancestor_name = NULL;
> > +
> > + parent1 = pickme->parents->item;
> > + parent2 = pickme->parents->next->item;
> > +
> > + /*
> > + * Map the merge's parents to their replayed counterparts. With the
> > + * boundary commits pre-seeded into `replayed_commits`, every parent
> > + * either has an explicit mapping (rewritten or boundary -> onto) or
> > + * sits outside the rewrite range entirely; the latter must stay at
> > + * the original parent commit, so use `parent` itself as the fallback
> > + * for both sides.
> > + */
> > + replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
> > + replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
> > +
> > + /*
> > + * R: auto-remerge of the original parents.
> > + *
> > + * Use the same branch labels for the inner merges that compute R
> > + * and N so conflict markers (if any) are textually identical
> > + * between the two; the outer non-recursive merge can then collapse
> > + * the manual resolution from O against them.
> > + */
> > + init_basic_merge_options(&remerge_opt, repo);
> > + remerge_opt.show_rename_progress = 0;
> > + remerge_opt.branch1 = "ours";
> > + remerge_opt.branch2 = "theirs";
> > + if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0) {
> > + result->clean = -1;
> > + goto out;
> > + }
> > + merge_incore_recursive(&remerge_opt, parent_bases,
> > + parent1, parent2, &remerge_res);
> > + parent_bases = NULL; /* consumed by merge_incore_recursive */
> > + if (remerge_res.clean < 0) {
> > + result->clean = remerge_res.clean;
> > + goto out;
> > + }
> > +
> > + /* N: fresh merge of the (possibly rewritten) parents. */
> > + init_basic_merge_options(&new_merge_opt, repo);
> > + new_merge_opt.show_rename_progress = 0;
> > + new_merge_opt.branch1 = "ours";
> > + new_merge_opt.branch2 = "theirs";
> > + if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
> > + &replayed_bases) < 0) {
> > + result->clean = -1;
> > + goto out;
> > + }
> > + merge_incore_recursive(&new_merge_opt, replayed_bases,
> > + replayed_par1, replayed_par2, &new_merge_res);
> > + replayed_bases = NULL; /* consumed by merge_incore_recursive */
> > + if (new_merge_res.clean < 0) {
> > + result->clean = new_merge_res.clean;
> > + goto out;
> > + }
> > +
> > + /*
> > + * Outer non-recursive merge: base=R, side1=O (pickme), side2=N.
> > + */
> > + pickme_tree = repo_get_commit_tree(repo, pickme);
> > + ancestor_name = xstrfmt("auto-remerge of %s",
> > + oid_to_hex(&pickme->object.oid));
> > + merge_opt->ancestor = ancestor_name;
> > + merge_opt->branch1 = short_commit_name(repo, pickme);
> > + merge_opt->branch2 = "merge of replayed parents";
> > + merge_incore_nonrecursive(merge_opt,
> > + remerge_res.tree,
> > + pickme_tree,
> > + new_merge_res.tree,
> > + result);
> > + merge_opt->ancestor = NULL;
> > + merge_opt->branch1 = NULL;
> > + merge_opt->branch2 = NULL;
> > + if (!result->clean)
> > + goto out;
> > +
> > + parents = NULL;
> > + commit_list_insert(replayed_par2, &parents);
> > + commit_list_insert(replayed_par1, &parents);
> > + picked = create_commit(repo, result->tree, pickme, parents,
> > + REPLAY_MODE_PICK);
> > +
> > +out:
> > + free(ancestor_name);
> > + free_commit_list(parent_bases);
> > + free_commit_list(replayed_bases);
> > + merge_finalize(&remerge_opt, &remerge_res);
> > + merge_finalize(&new_merge_opt, &new_merge_res);
> > + return picked;
> > }
> >
> > void replay_result_release(struct replay_result *result)
> > @@ -407,17 +550,63 @@ int replay_revisions(struct rev_info *revs,
> > merge_opt.show_rename_progress = 0;
> > last_commit = onto;
> > replayed_commits = kh_init_oid_map();
> > +
> > + /*
> > + * Seed the rewritten-commit map with each negative-side ("BOTTOM")
> > + * cmdline entry pointing at `onto`. This matters for merge replay:
> > + * a 2-parent merge whose first parent is the boundary (e.g. the
> > + * commit being reworded) must replay onto the rewritten boundary,
> > + * yet pick_merge_commit uses a self fallback so the second parent
> > + * (a side branch outside the rewrite range) is preserved as-is.
> > + * Pre-seeding the boundary disambiguates the two: in the map ->
> > + * rewritten, missing -> kept as-is.
> > + *
> > + * Only do this for the pick path; revert mode chains reverts
> > + * through last_commit and a pre-seeded boundary would short-circuit
> > + * that chain.
> > + */
> > + if (mode == REPLAY_MODE_PICK) {
> > + for (size_t i = 0; i < revs->cmdline.nr; i++) {
> > + struct rev_cmdline_entry *e = &revs->cmdline.rev[i];
> > + struct commit *boundary;
> > + khint_t pos;
> > + int hr;
> > +
> > + if (!(e->flags & BOTTOM))
> > + continue;
> > + boundary = lookup_commit_reference_gently(revs->repo,
> > +
> > &e->item->oid, 1);
> > + if (!boundary)
> > + continue;
> > + pos = kh_put_oid_map(replayed_commits,
> > + boundary->object.oid, &hr);
> > + if (hr != 0)
> > + kh_value(replayed_commits, pos) = onto;
> > + }
> > + }
> > +
> > while ((commit = get_revision(revs))) {
> > const struct name_decoration *decoration;
> > khint_t pos;
> > int hr;
> > - if (commit->parents && commit->parents->next)
> > - die(_("replaying merge commits is not supported
> > yet!"));
> > -
> > - last_commit = pick_regular_commit(revs->repo, commit,
> > replayed_commits,
> > - mode == REPLAY_MODE_REVERT ?
> > last_commit : onto,
> > - &merge_opt, &result, mode);
> > + if (commit->parents && commit->parents->next) {
> > + if (commit->parents->next->next) {
> > + ret = error(_("replaying octopus merges is not
> > supported"));
> > + goto out;
> > + }
> > + if (mode == REPLAY_MODE_REVERT) {
> > + ret = error(_("reverting merge commits is not
> > supported"));
> > + goto out;
> > + }
> > + last_commit = pick_merge_commit(revs->repo, commit,
> > + replayed_commits,
> > + &merge_opt, &result);
> > + } else {
> > + last_commit = pick_regular_commit(revs->repo, commit,
> > replayed_commits,
> > + mode ==
> > REPLAY_MODE_REVERT ? last_commit : onto,
> > + &merge_opt, &result,
> > mode);
> > + }
> > if (!last_commit)
> > break;
> > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> > index de7b357685..d103f866a2 100755
> > --- a/t/t3451-history-reword.sh
> > +++ b/t/t3451-history-reword.sh
> > @@ -201,12 +201,21 @@ test_expect_success 'can reword a merge commit' '
> > git switch - &&
> > git merge theirs &&
> > - # It is not possible to replay merge commits embedded in the
> > - # history (yet).
> > - test_must_fail git -c core.editor=false history reword HEAD~
> > 2>err &&
> > - test_grep "replaying merge commits is not supported yet" err
> > &&
> > + # Reword a non-merge commit whose descendants include the
> > + # merge: replay carries the merge through.
> > + reword_with_message HEAD~ <<-EOF &&
> > + ours reworded
> > + EOF
> > + expect_graph <<-EOF &&
> > + * Merge tag ${SQ}theirs${SQ}
> > + |\\
> > + | * theirs
> > + * | ours reworded
> > + |/
> > + * base
> > + EOF
> > - # But it is possible to reword a merge commit directly.
> > + # And reword a merge commit directly.
> > reword_with_message HEAD <<-EOF &&
> > Reworded merge commit
> > EOF
> > @@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' '
> > * Reworded merge commit
> > |\
> > | * theirs
> > - * | ours
> > + * | ours reworded
> > |/
> > * base
> > EOF
> > diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
> > index 8ed0cebb50..ad6309f98b 100755
> > --- a/t/t3452-history-split.sh
> > +++ b/t/t3452-history-split.sh
> > @@ -36,7 +36,7 @@ expect_tree_entries () {
> > test_cmp expect actual
> > }
> >
> > -test_expect_success 'refuses to work with merge commits' '
> > +test_expect_success 'refuses to split a merge commit' '
> > test_when_finished "rm -rf repo" &&
> > git init repo &&
> > (
> > @@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge commits' '
> > git switch - &&
> > git merge theirs &&
> > test_must_fail git history split HEAD 2>err &&
> > - test_grep "cannot split up merge commit" err &&
> > - test_must_fail git history split HEAD~ 2>err &&
> > - test_grep "replaying merge commits is not supported yet" err
> > + test_grep "cannot split up merge commit" err
> > )
> > '
> >
> > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> > index 3353bc4a4d..368b1b0f9a 100755
> > --- a/t/t3650-replay-basics.sh
> > +++ b/t/t3650-replay-basics.sh
> > @@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ...
> > ordering would be ill-defined' '
> > test_cmp expect actual
> > '
> >
> > -test_expect_success 'replaying merge commits is not supported yet' '
> > - echo "fatal: replaying merge commits is not supported yet!" >expect &&
> > - test_must_fail git replay --advance=main main..topic-with-merge
> > 2>actual &&
> > - test_cmp expect actual
> > +test_expect_success 'using replay to rebase a 2-parent merge' '
> > + # main..topic-with-merge contains a 2-parent merge (P) introduced
> > + # via test_merge. Use --ref-action=print so this test does not
> > + # mutate state for subsequent tests in this file.
> > + git replay --ref-action=print --onto main main..topic-with-merge
> > >result &&
> > + test_line_count = 1 result &&
> > +
> > + new_tip=$(cut -f 3 -d " " result) &&
> > +
> > + # Result is still a 2-parent merge.
> > + git cat-file -p $new_tip >cat &&
> > + grep -c "^parent " cat >count &&
> > + echo 2 >expect &&
> > + test_cmp expect count &&
> > +
> > + # Merge subject is preserved.
> > + echo P >expect &&
> > + git log -1 --format=%s $new_tip >actual &&
> > + test_cmp expect actual &&
> > +
> > + # The replayed merge sits on top of main: walking back via the
> > + # first-parent chain reaches main.
> > + git merge-base --is-ancestor main $new_tip
> > +'
> > +
> > +test_expect_success 'replaying an octopus merge is rejected' '
> > + # Build an octopus side-branch so the rest of the test state stays
> > + # untouched.
> > + test_when_finished "git update-ref -d refs/heads/octopus-tip" &&
> > + octopus_tip=$(git commit-tree -p topic4 -p topic1 -p topic3 \
> > + -m "octopus" $(git rev-parse topic4^{tree})) &&
> > + git update-ref refs/heads/octopus-tip "$octopus_tip" &&
> > +
> > + test_must_fail git replay --ref-action=print --onto main \
> > + topic4..octopus-tip 2>actual &&
> > + test_grep "octopus merges" actual
> > +'
> > +
> > +test_expect_success 'reverting a merge commit is rejected' '
> > + test_must_fail git replay --ref-action=print --revert=topic-with-merge
> > \
> > + topic4..topic-with-merge 2>actual &&
> > + test_grep "reverting merge commits" actual
> > '
> >
> > test_expect_success 'using replay to rebase two branches, one on top of
> > other' '
>
> There was a problem hiding this comment.
Johannes Schindelin wrote on the Git mailing list (how to reply to this email):
Hi Phillip,
On Fri, 8 May 2026, Phillip Wood wrote:
> On 08/05/2026 10:36, Phillip Wood wrote:
> >
> > On 06/05/2026 23:43, Johannes Schindelin via GitGitGadget wrote:
> > >
> > > Elijah Newren spelled out a way to lift this limitation in his
> > > replay-design-notes [1] and prototyped it in a 2022
> > > work-in-progress sketch [2]. The idea is that a merge commit M on
> > > parents (P1, P2) records both an automatic merge of those parents
> > > AND any manual layer the author put on top of that automatic merge
> > > (textual conflict resolution and any semantic edit outside conflict
> > > markers). Replaying M onto rewritten parents (P1', P2') must
> > > preserve that manual layer, but the rewritten parents change the
> > > automatic merge, so a simple cherry-pick is wrong: the manual layer
> > > would be re-introduced on top of stale auto-merge text.
> > >
> > > What works instead is a three-way merge of three trees the existing
> > > infrastructure already knows how to compute. Let R be the recursive
> > > auto-merge of (P1, P2), O be M's actual tree and N be the recursive
> > > auto-merge of (P1', P2'). Then `git diff R O` is morally
> > > `git show --remerge-diff M`: it captures exactly what the author
> > > added on top of the automatic merge. A non-recursive 3-way merge
> > > with R as the merge base, O as side 1 and N as side 2 layers that
> > > manual contribution onto the freshly auto-merged rewritten parents
> > > (N) and produces the replayed tree.
> >
> > So we cherry-pick the difference between the user's conflict resolution O
> > and the auto-merge M of the original parents onto the auto-merge N of the
> > replayed parents. If we have a topology that looks like
> >
> > |
> > A
> > /|\
> > / B \
> > E | D
> > C /
> > |/
> > O
> >
> > then running
> >
> > git replay --onto E --ancestry-path B..O
> >
> > will replay C and O onto E. If the changes in E and D conflict but those
> > conflicts do not overlap with the conflicts in M that were resolved to
> > create O then the replayed version of O will contain conflict markers from
> > the conflicting changes in E and D. Because the previous conflict resolution
> > applies to N without conflicts we do not recognize that there are still
> > conflicts in N that need to be resolved.
> >
> > Having realized this I went to look at Elijah's notes and they recognize
> > this possibility and suggest extending the xdiff merge code to detect when N
> > has conflicts that do not correspond to the conflicts in M. That sounds like
> > quite a lot of work. I've not put much effort into coming up with a
> > counterexample but think that because "git replay" and "git history" do not
> > yet allow the commits in the merged branches to be edited we may be able to
> > safely use the implementation proposed in this series if both merge parents
> > have been rebased (or we might want all the merge bases of the new merge to
> > be a descendants of "--onto"). In the example above if both the parents were
> > rebased onto E then any new conflicts would happen when picking D rather
> > than when recreating the merge.
>
> One further thought - if only one of the parents has been rebased (i.e. we're
> replaying O with parents P1' and P2) then can we just cherry-pick the merge -
> instead of merging P1' and P2, use P1 as the merge-base with O and P1' as the
> merge heads?
That's a really good idea! That should _especially_ work well for the
conflict markers in case of conflicts.
Ciao,
Johannes
>
> Thanks
>
> Phillip
>
> > Thanks
> >
> > Phillip
> >
> > > Implement `pick_merge_commit()` along those lines and dispatch to it
> > > from `replay_revisions()` when the commit being replayed has exactly
> > > two parents. Two specific points (learned the hard way) keep
> > > non-trivial cases working where the WIP sketch [2] bailed out.
> > > First, R and N use identical `merge_options.branch1` and `branch2`
> > > labels ("ours"/"theirs"). When the original parents conflicted on a
> > > region of a file, both R and N produce textually identical conflict
> > > markers; the outer non-recursive merge then sees N == R in that
> > > region and the user's manual resolution from O wins cleanly. Without
> > > this, the conflict-marker text would differ between R and N (because
> > > the inner merges would label the conflicts differently), and the
> > > outer merge would itself be unclean even when the user did supply a
> > > clean resolution. Second, an unclean inner merge
> > > (`result.clean == 0`) is _not_ fatal: the tree merge-ort produces in
> > > that case still has well-defined contents (with conflict markers in
> > > the conflicted files) and is a valid input to the outer
> > > non-recursive merge. Only a real error (`< 0`) propagates as
> > > failure.
> > >
> > > The replay propagates the textual diffs the user actually made in M;
> > > it does _not_ extrapolate symbol-level intent. If rewriting the
> > > parents pulls in genuinely new content (for example, a brand-new
> > > caller of a function that the merge renamed), that new content stays
> > > as the rewritten parents have it. Symbol-aware refactoring is out of
> > > scope here, just as it is for plain rebase.
> > >
> > > Octopus merges (more than two parents) and revert-of-merge are not
> > > supported and are surfaced as explicit errors at the dispatch point.
> > > The "split" sub-command of `git history` continues to refuse when
> > > the targeted commit is itself a merge: split semantics do not apply
> > > to merges. The pre-walk gate in `builtin/history.c` that previously
> > > rejected any merge in the rewrite path now only rejects octopus
> > > merges; rename it accordingly.
> > >
> > > A small refactor in `create_commit()` makes the merge case possible:
> > > the helper now takes a `struct commit_list *parents` rather than a
> > > single parent pointer and takes ownership of the list. The single
> > > existing caller in `pick_regular_commit()` builds and passes a
> > > one-element list; the new `pick_merge_commit()` builds a two-element
> > > list, with the order of the `from` and `merge` parents preserved.
> > >
> > > Update the negative expectations in t3451, t3452 and t3650 that were
> > > asserting the now-retired "not supported yet" message, replacing
> > > them with positive coverage where it fits. Octopus rejection and
> > > revert-of-merge rejection are covered by new positive tests in
> > > t3650. A dedicated test script with merge-replay scenarios driven by
> > > a new test-tool fixture builder will follow in a subsequent commit.
> > >
> > > [1] https://github.com/newren/git/blob/replay/replay-design-notes.txt
> > > [2] https://github.com/newren/git/
> > > commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45
> > >
> > > Helped-by: Elijah Newren <newren@gmail.com>
> > > Assisted-by: Claude Opus 4.7
> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > ---
> > > builtin/history.c | 16 ++-
> > > replay.c | 209 ++++++++++++++++++++++++++++++++++++--
> > > t/t3451-history-reword.sh | 21 ++--
> > > t/t3452-history-split.sh | 6 +-
> > > t/t3650-replay-basics.sh | 46 ++++++++-
> > > 5 files changed, 269 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/builtin/history.c b/builtin/history.c
> > > index 9526938085..00097b2226 100644
> > > --- a/builtin/history.c
> > > +++ b/builtin/history.c
> > > @@ -195,15 +195,15 @@ static int parse_ref_action(const struct option
> > > *opt, const char *value, int uns
> > > return 0;
> > > }
> > > -static int revwalk_contains_merges(struct repository *repo,
> > > - const struct strvec *revwalk_args)
> > > +static int revwalk_contains_octopus_merges(struct repository *repo,
> > > + const struct strvec *revwalk_args)
> > > {
> > > struct strvec args = STRVEC_INIT;
> > > struct rev_info revs;
> > > int ret;
> > > strvec_pushv(&args, revwalk_args->v);
> > > - strvec_push(&args, "--min-parents=2");
> > > + strvec_push(&args, "--min-parents=3");
> > > repo_init_revisions(repo, &revs, NULL);
> > > @@ -217,7 +217,7 @@ static int revwalk_contains_merges(struct repository
> > > *repo,
> > > }
> > > if (get_revision(&revs)) {
> > > - ret = error(_("replaying merge commits is not supported yet!"));
> > > + ret = error(_("replaying octopus merges is not supported"));
> > > goto out;
> > > }
> > > @@ -289,7 +289,7 @@ static int setup_revwalk(struct repository *repo,
> > > strvec_push(&args, "HEAD");
> > > }
> > > - ret = revwalk_contains_merges(repo, &args);
> > > + ret = revwalk_contains_octopus_merges(repo, &args);
> > > if (ret < 0)
> > > goto out;
> > > @@ -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;
> > > diff --git a/replay.c b/replay.c
> > > index f96f1f6551..3dbce095f9 100644
> > > --- a/replay.c
> > > +++ b/replay.c
> > > @@ -1,6 +1,7 @@
> > > #define USE_THE_REPOSITORY_VARIABLE
> > > #include "git-compat-util.h"
> > > +#include "commit-reach.h"
> > > #include "environment.h"
> > > #include "hex.h"
> > > #include "merge-ort.h"
> > > @@ -77,15 +78,21 @@ static void generate_revert_message(struct strbuf
> > > *msg,
> > > repo_unuse_commit_buffer(repo, commit, message);
> > > }
> > > +/*
> > > + * Build a new commit with the given tree and parent list, copying
> > > author,
> > > + * extra headers and (for pick mode) the commit message from `based_on`.
> > > + *
> > > + * Takes ownership of `parents`: it will be freed before returning, even
> > > on
> > > + * error. Parent order is preserved as supplied by the caller.
> > > + */
> > > static struct commit *create_commit(struct repository *repo,
> > > struct tree *tree,
> > > struct commit *based_on,
> > > - struct commit *parent,
> > > + struct commit_list *parents,
> > > enum replay_mode mode)
> > > {
> > > struct object_id ret;
> > > struct object *obj = NULL;
> > > - struct commit_list *parents = NULL;
> > > char *author = NULL;
> > > char *sign_commit = NULL; /* FIXME: cli users might want to sign
> > > again */
> > > struct commit_extra_header *extra = NULL;
> > > @@ -96,7 +103,6 @@ static struct commit *create_commit(struct repository
> > > *repo,
> > > const char *orig_message = NULL;
> > > const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
> > > - commit_list_insert(parent, &parents);
> > > extra = read_commit_extra_headers(based_on, exclude_gpgsig);
> > > if (mode == REPLAY_MODE_REVERT) {
> > > generate_revert_message(&msg, based_on, repo);
> > > @@ -273,6 +279,7 @@ static struct commit *pick_regular_commit(struct
> > > repository *repo,
> > > {
> > > struct commit *base, *replayed_base;
> > > struct tree *pickme_tree, *base_tree, *replayed_base_tree;
> > > + struct commit_list *parents = NULL;
> > > if (pickme->parents) {
> > > base = pickme->parents->item;
> > > @@ -327,7 +334,143 @@ static struct commit *pick_regular_commit(struct
> > > repository *repo,
> > > if (oideq(&replayed_base_tree->object.oid, &result->tree-
> > > >object.oid) &&
> > > !oideq(&pickme_tree->object.oid, &base_tree->object.oid))
> > > return replayed_base;
> > > - return create_commit(repo, result->tree, pickme, replayed_base,
> > > mode);
> > > + commit_list_insert(replayed_base, &parents);
> > > + return create_commit(repo, result->tree, pickme, parents, mode);
> > > +}
> > > +
> > > +/*
> > > + * Replay a 2-parent merge commit by composing three calls into
> > > merge-ort:
> > > + *
> > > + * R = recursive merge of pickme's two original parents (auto- remerge
> > > of
> > > + * the original merge, accepting any conflicts)
> > > + * N = recursive merge of the (possibly rewritten) parents
> > > + * O = pickme's tree (the user's actual merge, including any manual
> > > + * resolutions)
> > > + *
> > > + * The picked tree comes from a non-recursive merge using R as the base,
> > > + * O as side1 and N as side2. `git diff R O` is morally `git show
> > > + * --remerge-diff $oldmerge`, so this layers the user's original manual
> > > + * resolution on top of the freshly auto-merged rewritten parents (see
> > > + * `replay-design-notes.txt` on the `replay` branch of newren/git).
> > > + *
> > > + * If the outer 3-way merge is unclean, propagate the conflict status to
> > > + * the caller via `result->clean = 0` and return NULL. The two inner
> > > + * merges (R and N) being unclean is _not_ fatal: the conflict-markered
> > > + * trees they produce are valid inputs to the outer merge, and using
> > > + * identical labels for both inner merges keeps the marker text
> > > + * byte-equal between R and N so the user's resolution recorded in O
> > > + * collapses the conflict cleanly there. Octopus merges (more than two
> > > + * parents) and revert-of-merge are rejected by the caller before this
> > > + * function is invoked.
> > > + */
> > > +static struct commit *pick_merge_commit(struct repository *repo,
> > > + struct commit *pickme,
> > > + kh_oid_map_t *replayed_commits,
> > > + struct merge_options *merge_opt,
> > > + struct merge_result *result)
> > > +{
> > > + struct commit *parent1, *parent2;
> > > + struct commit *replayed_par1, *replayed_par2;
> > > + struct tree *pickme_tree;
> > > + struct merge_options remerge_opt = { 0 };
> > > + struct merge_options new_merge_opt = { 0 };
> > > + struct merge_result remerge_res = { 0 };
> > > + struct merge_result new_merge_res = { 0 };
> > > + struct commit_list *parent_bases = NULL;
> > > + struct commit_list *replayed_bases = NULL;
> > > + struct commit_list *parents;
> > > + struct commit *picked = NULL;
> > > + char *ancestor_name = NULL;
> > > +
> > > + parent1 = pickme->parents->item;
> > > + parent2 = pickme->parents->next->item;
> > > +
> > > + /*
> > > + * Map the merge's parents to their replayed counterparts. With the
> > > + * boundary commits pre-seeded into `replayed_commits`, every parent
> > > + * either has an explicit mapping (rewritten or boundary -> onto) or
> > > + * sits outside the rewrite range entirely; the latter must stay at
> > > + * the original parent commit, so use `parent` itself as the fallback
> > > + * for both sides.
> > > + */
> > > + replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
> > > + replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
> > > +
> > > + /*
> > > + * R: auto-remerge of the original parents.
> > > + *
> > > + * Use the same branch labels for the inner merges that compute R
> > > + * and N so conflict markers (if any) are textually identical
> > > + * between the two; the outer non-recursive merge can then collapse
> > > + * the manual resolution from O against them.
> > > + */
> > > + init_basic_merge_options(&remerge_opt, repo);
> > > + remerge_opt.show_rename_progress = 0;
> > > + remerge_opt.branch1 = "ours";
> > > + remerge_opt.branch2 = "theirs";
> > > + if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0)
> > > {
> > > + result->clean = -1;
> > > + goto out;
> > > + }
> > > + merge_incore_recursive(&remerge_opt, parent_bases,
> > > + parent1, parent2, &remerge_res);
> > > + parent_bases = NULL; /* consumed by merge_incore_recursive */
> > > + if (remerge_res.clean < 0) {
> > > + result->clean = remerge_res.clean;
> > > + goto out;
> > > + }
> > > +
> > > + /* N: fresh merge of the (possibly rewritten) parents. */
> > > + init_basic_merge_options(&new_merge_opt, repo);
> > > + new_merge_opt.show_rename_progress = 0;
> > > + new_merge_opt.branch1 = "ours";
> > > + new_merge_opt.branch2 = "theirs";
> > > + if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
> > > + &replayed_bases) < 0) {
> > > + result->clean = -1;
> > > + goto out;
> > > + }
> > > + merge_incore_recursive(&new_merge_opt, replayed_bases,
> > > + replayed_par1, replayed_par2, &new_merge_res);
> > > + replayed_bases = NULL; /* consumed by merge_incore_recursive */
> > > + if (new_merge_res.clean < 0) {
> > > + result->clean = new_merge_res.clean;
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > + * Outer non-recursive merge: base=R, side1=O (pickme), side2=N.
> > > + */
> > > + pickme_tree = repo_get_commit_tree(repo, pickme);
> > > + ancestor_name = xstrfmt("auto-remerge of %s",
> > > + oid_to_hex(&pickme->object.oid));
> > > + merge_opt->ancestor = ancestor_name;
> > > + merge_opt->branch1 = short_commit_name(repo, pickme);
> > > + merge_opt->branch2 = "merge of replayed parents";
> > > + merge_incore_nonrecursive(merge_opt,
> > > + remerge_res.tree,
> > > + pickme_tree,
> > > + new_merge_res.tree,
> > > + result);
> > > + merge_opt->ancestor = NULL;
> > > + merge_opt->branch1 = NULL;
> > > + merge_opt->branch2 = NULL;
> > > + if (!result->clean)
> > > + goto out;
> > > +
> > > + parents = NULL;
> > > + commit_list_insert(replayed_par2, &parents);
> > > + commit_list_insert(replayed_par1, &parents);
> > > + picked = create_commit(repo, result->tree, pickme, parents,
> > > + REPLAY_MODE_PICK);
> > > +
> > > +out:
> > > + free(ancestor_name);
> > > + free_commit_list(parent_bases);
> > > + free_commit_list(replayed_bases);
> > > + merge_finalize(&remerge_opt, &remerge_res);
> > > + merge_finalize(&new_merge_opt, &new_merge_res);
> > > + return picked;
> > > }
> > > void replay_result_release(struct replay_result *result)
> > > @@ -407,17 +550,63 @@ int replay_revisions(struct rev_info *revs,
> > > merge_opt.show_rename_progress = 0;
> > > last_commit = onto;
> > > replayed_commits = kh_init_oid_map();
> > > +
> > > + /*
> > > + * Seed the rewritten-commit map with each negative-side ("BOTTOM")
> > > + * cmdline entry pointing at `onto`. This matters for merge replay:
> > > + * a 2-parent merge whose first parent is the boundary (e.g. the
> > > + * commit being reworded) must replay onto the rewritten boundary,
> > > + * yet pick_merge_commit uses a self fallback so the second parent
> > > + * (a side branch outside the rewrite range) is preserved as-is.
> > > + * Pre-seeding the boundary disambiguates the two: in the map ->
> > > + * rewritten, missing -> kept as-is.
> > > + *
> > > + * Only do this for the pick path; revert mode chains reverts
> > > + * through last_commit and a pre-seeded boundary would short-circuit
> > > + * that chain.
> > > + */
> > > + if (mode == REPLAY_MODE_PICK) {
> > > + for (size_t i = 0; i < revs->cmdline.nr; i++) {
> > > + struct rev_cmdline_entry *e = &revs->cmdline.rev[i];
> > > + struct commit *boundary;
> > > + khint_t pos;
> > > + int hr;
> > > +
> > > + if (!(e->flags & BOTTOM))
> > > + continue;
> > > + boundary = lookup_commit_reference_gently(revs->repo,
> > > + &e->item->oid, 1);
> > > + if (!boundary)
> > > + continue;
> > > + pos = kh_put_oid_map(replayed_commits,
> > > + boundary->object.oid, &hr);
> > > + if (hr != 0)
> > > + kh_value(replayed_commits, pos) = onto;
> > > + }
> > > + }
> > > +
> > > while ((commit = get_revision(revs))) {
> > > const struct name_decoration *decoration;
> > > khint_t pos;
> > > int hr;
> > > - if (commit->parents && commit->parents->next)
> > > - die(_("replaying merge commits is not supported yet!"));
> > > -
> > > - last_commit = pick_regular_commit(revs->repo, commit,
> > > replayed_commits,
> > > - mode == REPLAY_MODE_REVERT ? last_commit :
> > > onto,
> > > - &merge_opt, &result, mode);
> > > + if (commit->parents && commit->parents->next) {
> > > + if (commit->parents->next->next) {
> > > + ret = error(_("replaying octopus merges is not
> > > supported"));
> > > + goto out;
> > > + }
> > > + if (mode == REPLAY_MODE_REVERT) {
> > > + ret = error(_("reverting merge commits is not
> > > supported"));
> > > + goto out;
> > > + }
> > > + last_commit = pick_merge_commit(revs->repo, commit,
> > > + replayed_commits,
> > > + &merge_opt, &result);
> > > + } else {
> > > + last_commit = pick_regular_commit(revs->repo, commit,
> > > replayed_commits,
> > > + mode == REPLAY_MODE_REVERT ? last_commit :
> > > onto,
> > > + &merge_opt, &result, mode);
> > > + }
> > > if (!last_commit)
> > > break;
> > > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> > > index de7b357685..d103f866a2 100755
> > > --- a/t/t3451-history-reword.sh
> > > +++ b/t/t3451-history-reword.sh
> > > @@ -201,12 +201,21 @@ test_expect_success 'can reword a merge commit' '
> > > git switch - &&
> > > git merge theirs &&
> > > - # It is not possible to replay merge commits embedded in the
> > > - # history (yet).
> > > - test_must_fail git -c core.editor=false history reword HEAD~
> > > 2>err &&
> > > - test_grep "replaying merge commits is not supported yet" err &&
> > > + # Reword a non-merge commit whose descendants include the
> > > + # merge: replay carries the merge through.
> > > + reword_with_message HEAD~ <<-EOF &&
> > > + ours reworded
> > > + EOF
> > > + expect_graph <<-EOF &&
> > > + * Merge tag ${SQ}theirs${SQ}
> > > + |\\
> > > + | * theirs
> > > + * | ours reworded
> > > + |/
> > > + * base
> > > + EOF
> > > - # But it is possible to reword a merge commit directly.
> > > + # And reword a merge commit directly.
> > > reword_with_message HEAD <<-EOF &&
> > > Reworded merge commit
> > > EOF
> > > @@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' '
> > > * Reworded merge commit
> > > |\
> > > | * theirs
> > > - * | ours
> > > + * | ours reworded
> > > |/
> > > * base
> > > EOF
> > > diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
> > > index 8ed0cebb50..ad6309f98b 100755
> > > --- a/t/t3452-history-split.sh
> > > +++ b/t/t3452-history-split.sh
> > > @@ -36,7 +36,7 @@ expect_tree_entries () {
> > > test_cmp expect actual
> > > }
> > > -test_expect_success 'refuses to work with merge commits' '
> > > +test_expect_success 'refuses to split a merge commit' '
> > > test_when_finished "rm -rf repo" &&
> > > git init repo &&
> > > (
> > > @@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge commits'
> > > '
> > > git switch - &&
> > > git merge theirs &&
> > > test_must_fail git history split HEAD 2>err &&
> > > - test_grep "cannot split up merge commit" err &&
> > > - test_must_fail git history split HEAD~ 2>err &&
> > > - test_grep "replaying merge commits is not supported yet" err
> > > + test_grep "cannot split up merge commit" err
> > > )
> > > '
> > > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> > > index 3353bc4a4d..368b1b0f9a 100755
> > > --- a/t/t3650-replay-basics.sh
> > > +++ b/t/t3650-replay-basics.sh
> > > @@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ...
> > > ordering would be ill-defined' '
> > > test_cmp expect actual
> > > '
> > > -test_expect_success 'replaying merge commits is not supported yet' '
> > > - echo "fatal: replaying merge commits is not supported yet!"
> > > >expect &&
> > > - test_must_fail git replay --advance=main main..topic-with-merge
> > > 2>actual &&
> > > - test_cmp expect actual
> > > +test_expect_success 'using replay to rebase a 2-parent merge' '
> > > + # main..topic-with-merge contains a 2-parent merge (P) introduced
> > > + # via test_merge. Use --ref-action=print so this test does not
> > > + # mutate state for subsequent tests in this file.
> > > + git replay --ref-action=print --onto main main..topic-with-merge
> > > >result &&
> > > + test_line_count = 1 result &&
> > > +
> > > + new_tip=$(cut -f 3 -d " " result) &&
> > > +
> > > + # Result is still a 2-parent merge.
> > > + git cat-file -p $new_tip >cat &&
> > > + grep -c "^parent " cat >count &&
> > > + echo 2 >expect &&
> > > + test_cmp expect count &&
> > > +
> > > + # Merge subject is preserved.
> > > + echo P >expect &&
> > > + git log -1 --format=%s $new_tip >actual &&
> > > + test_cmp expect actual &&
> > > +
> > > + # The replayed merge sits on top of main: walking back via the
> > > + # first-parent chain reaches main.
> > > + git merge-base --is-ancestor main $new_tip
> > > +'
> > > +
> > > +test_expect_success 'replaying an octopus merge is rejected' '
> > > + # Build an octopus side-branch so the rest of the test state stays
> > > + # untouched.
> > > + test_when_finished "git update-ref -d refs/heads/octopus-tip" &&
> > > + octopus_tip=$(git commit-tree -p topic4 -p topic1 -p topic3 \
> > > + -m "octopus" $(git rev-parse topic4^{tree})) &&
> > > + git update-ref refs/heads/octopus-tip "$octopus_tip" &&
> > > +
> > > + test_must_fail git replay --ref-action=print --onto main \
> > > + topic4..octopus-tip 2>actual &&
> > > + test_grep "octopus merges" actual
> > > +'
> > > +
> > > +test_expect_success 'reverting a merge commit is rejected' '
> > > + test_must_fail git replay --ref-action=print --revert=topic-with-
> > > merge \
> > > + topic4..topic-with-merge 2>actual &&
> > > + test_grep "reverting merge commits" actual
> > > '
> > > test_expect_success 'using replay to rebase two branches, one on top of
> > > other' '
> >
>
>
>
|
User |
| @@ -832,6 +832,7 @@ TEST_BUILTINS_OBJS += test-hash-speed.o | |||
| TEST_BUILTINS_OBJS += test-hash.o | |||
There was a problem hiding this comment.
Toon Claes wrote on the Git mailing list (how to reply to this email):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The merge-replay tests added in a follow-up commit need a way to set
> up specific topologies with full control over blob contents, parent
> order, and per-side trees. Sequencing plumbing commands or driving
> plain `git fast-import` from shell quickly becomes unreadable for
> the kinds of scenarios that exercise non-trivial merge resolution
> (textual conflicts, semantic edits outside the conflict region,
> intentional limitations such as new content on one side).
>
> Add a small `test-tool historian` subcommand that reads a tight,
> shell-quoted, one-line-per-object DSL and feeds an equivalent stream
> to a `git fast-import` child process. Each blob and commit is given
> a logical name; the helper allocates fast-import marks on first use
> and emits a lightweight tag for every commit so tests can refer to
> the resulting object via `refs/tags/<name>`.
>
> The DSL has just two directives:
>
> blob NAME LINE...
> commit NAME BRANCH SUBJECT [from=NAME] [merge=NAME]... [PATH=BLOB]...
>
> A blob's content is the listed lines joined with `\n` (and a final
> `\n`); a commit's tree is exactly the listed PATH=BLOB pairs (the
> helper emits a `deleteall` so nothing leaks in from the implicit
> parent). Token splitting is delegated to `split_cmdline()` so quoted
> arguments work as in shell. Marks for parent references and file
> contents go through the same `strintmap`-backed name resolver, which
> keeps the helper itself trivially small: blob writing, tree
> construction, commit creation and merge-base computation are all
> handled by `git fast-import`.
>
> Note that the DSL reserves the names `from` and `merge` (with a
> trailing `=`) for parent specification; a tree path called `from` or
> `merge` cannot be expressed via this helper. That is acceptable here
> because every input is a tightly controlled test fixture and the
> filenames are chosen by the test author.
>
> The helper trusts its caller: malformed input results in a
> fast-import error rather than a friendly diagnostic.
>
> Wire the new subcommand into the Makefile and meson build, register
> it in `t/helper/test-tool.{c,h}`.
>
> Assisted-by: Claude Opus 4.7
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Makefile | 1 +
> t/helper/meson.build | 1 +
> t/helper/test-historian.c | 189 ++++++++++++++++++++++++++++++++++++++
> t/helper/test-tool.c | 1 +
> t/helper/test-tool.h | 1 +
> 5 files changed, 193 insertions(+)
> create mode 100644 t/helper/test-historian.c
>
> diff --git a/Makefile b/Makefile
> index cedc234173..b38678b484 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -832,6 +832,7 @@ TEST_BUILTINS_OBJS += test-hash-speed.o
> TEST_BUILTINS_OBJS += test-hash.o
> TEST_BUILTINS_OBJS += test-hashmap.o
> TEST_BUILTINS_OBJS += test-hexdump.o
> +TEST_BUILTINS_OBJS += test-historian.o
> TEST_BUILTINS_OBJS += test-json-writer.o
> TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
> TEST_BUILTINS_OBJS += test-match-trees.o
> diff --git a/t/helper/meson.build b/t/helper/meson.build
> index 675e64c010..704edd1e1f 100644
> --- a/t/helper/meson.build
> +++ b/t/helper/meson.build
> @@ -29,6 +29,7 @@ test_tool_sources = [
> 'test-hash.c',
> 'test-hashmap.c',
> 'test-hexdump.c',
> + 'test-historian.c',
> 'test-json-writer.c',
> 'test-lazy-init-name-hash.c',
> 'test-match-trees.c',
> diff --git a/t/helper/test-historian.c b/t/helper/test-historian.c
> new file mode 100644
> index 0000000000..2250d420c0
> --- /dev/null
> +++ b/t/helper/test-historian.c
> @@ -0,0 +1,189 @@
> +/*
> + * Build a small history out of a tiny declarative input. Used by tests
> + * that need specific merge topologies without long sequences of
> + * plumbing commands or fragile shell helpers.
> + *
> + * The historian reads stdin line by line and emits an equivalent
> + * stream to a `git fast-import` child process. It also allocates marks
> + * for named objects so tests can refer to commits and blobs by name.
Really appreciate you're introducing this command. I'm actually
surprised no else did before.
> + *
> + * Input directives (one per line, shell-style quoting):
> + *
> + * blob NAME LINE1 LINE2 ...
> + * Each LINE becomes a content line in the blob; lines are
> + * joined with '\n' and the blob ends with a final '\n'. With
> + * no LINEs, the blob is empty.
> + *
> + * commit NAME BRANCH SUBJECT [from=PARENT] [merge=PARENT]... [PATH=BLOB]...
I'm not sure how I feel about mixing named arguments (like `from=PARENT`) with
the `PATH=BLOB` arguments? Obviously this tool isn't made for anything
that's even close to production, but still feels strange. How about
putting a double dash (`--`) before the paths, or using the `PATH:BLOB`
syntax instead?
> + * Creates a commit on refs/heads/BRANCH using the listed
> + * file=blob mappings as the entire tree (no inheritance from
> + * parents). Up to one `from=` and any number of `merge=`
> + * parents may be given. `from=` defaults to the current branch
> + * tip; if BRANCH has no tip yet, the commit becomes a root.
At GitLab in our Gitaly suite we have a similar tool as what you're
introducing here, but there you have to specify the parent(s) for each
commit and if you want to assign a ref to a commit, you have to be
explicit about it. So I would replace `from=` and `merge=` with
`parent=` and allow that to be occur zero or more times (this would also
allow creating unrelated histories). And remove the mandatory argument
BRANCH, and instead allow the command to accept a `branch=` argument.
If we'd take an example from the follow-up commit:
# Setup:
# A (a) --- C (a, h) ----+--- M (a, g, h)
# \ /
# +-- B (a, g) ------+
#
# Topic touches `g` only; main touches `h` only. The auto-merge
# at M is clean.
blob a "shared content"
blob g guarded
blob h host
commit A main "A" a=a
commit B topic "B (introduces g)" from=A a=a g=g
commit C main "C (introduces h)" a=a h=h
commit M main "Merge topic" merge=B a=a g=g h=h
I would suggest to rewrite that to:
blob a "shared content"
blob g guarded
blob h host
commit A "A" a:a
commit B "B (introduces g)" parent=A branch=topic a:a g:g
commit C "C (introduces h)" parent=A a:a h:h
commit M "Merge topic" parent=A parent=B ref=main a:a g:g h:h
I realize this is less alike to git-fast-import(1), so I'd understand if
you'd reject this idea.
--
Cheers,
ToonThere was a problem hiding this comment.
Johannes Schindelin wrote on the Git mailing list (how to reply to this email):
Hi Toon,
On Tue, 12 May 2026, Toon Claes wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/t/helper/test-historian.c b/t/helper/test-historian.c
> > new file mode 100644
> > index 0000000000..2250d420c0
> > --- /dev/null
> > +++ b/t/helper/test-historian.c
> > @@ -0,0 +1,189 @@
> > +/*
> > + * Build a small history out of a tiny declarative input. Used by tests
> > + * that need specific merge topologies without long sequences of
> > + * plumbing commands or fragile shell helpers.
> > + *
> > + * The historian reads stdin line by line and emits an equivalent
> > + * stream to a `git fast-import` child process. It also allocates marks
> > + * for named objects so tests can refer to commits and blobs by name.
>
> Really appreciate you're introducing this command. I'm actually
> surprised no else did before.
Heh. I am not surprised, though. Given that it is _really_ hard to come up
with a decent Domain-Specific Language to describe commit history for
defining test fixtures (testament to which are your comments below), I was
debating whether to go for range-diff's/libgit2's approach and simply
adding a bare repository or a fast-import script as a fixture. But that
would make the test code even harder to follow! And I did not want to add
to the amount of hard-to-follow test code.
> > + *
> > + * Input directives (one per line, shell-style quoting):
> > + *
> > + * blob NAME LINE1 LINE2 ...
> > + * Each LINE becomes a content line in the blob; lines are
> > + * joined with '\n' and the blob ends with a final '\n'. With
> > + * no LINEs, the blob is empty.
> > + *
> > + * commit NAME BRANCH SUBJECT [from=PARENT] [merge=PARENT]... [PATH=BLOB]...
>
> I'm not sure how I feel about mixing named arguments (like `from=PARENT`) with
> the `PATH=BLOB` arguments? Obviously this tool isn't made for anything
> that's even close to production, but still feels strange. How about
> putting a double dash (`--`) before the paths, or using the `PATH:BLOB`
> syntax instead?
Okay, I can see that's a better design. To be honest, I did not really
optimize for unambiguity here, but for precision of defining a test
fixture. As such, I am mostly interested in keeping the definition as
small as possible without losing readability. Changing that `=` to a `:`
to disambiguate keeps the same length while clarifying the structure. I
like it!
> > + * Creates a commit on refs/heads/BRANCH using the listed
> > + * file=blob mappings as the entire tree (no inheritance from
> > + * parents). Up to one `from=` and any number of `merge=`
> > + * parents may be given. `from=` defaults to the current branch
> > + * tip; if BRANCH has no tip yet, the commit becomes a root.
>
> At GitLab in our Gitaly suite we have a similar tool as what you're
> introducing here, but there you have to specify the parent(s) for each
> commit and if you want to assign a ref to a commit, you have to be
> explicit about it. So I would replace `from=` and `merge=` with
> `parent=` and allow that to be occur zero or more times (this would also
> allow creating unrelated histories). And remove the mandatory argument
> BRANCH, and instead allow the command to accept a `branch=` argument.
>
> If we'd take an example from the follow-up commit:
>
> # Setup:
> # A (a) --- C (a, h) ----+--- M (a, g, h)
> # \ /
> # +-- B (a, g) ------+
> #
> # Topic touches `g` only; main touches `h` only. The auto-merge
> # at M is clean.
> blob a "shared content"
> blob g guarded
> blob h host
> commit A main "A" a=a
> commit B topic "B (introduces g)" from=A a=a g=g
> commit C main "C (introduces h)" a=a h=h
> commit M main "Merge topic" merge=B a=a g=g h=h
>
> I would suggest to rewrite that to:
>
> blob a "shared content"
> blob g guarded
> blob h host
> commit A "A" a:a
> commit B "B (introduces g)" parent=A branch=topic a:a g:g
> commit C "C (introduces h)" parent=A a:a h:h
> commit M "Merge topic" parent=A parent=B ref=main a:a g:g h:h
>
> I realize this is less alike to git-fast-import(1), so I'd understand if
> you'd reject this idea.
That makes sense to me!
I'm not interested in keeping the `historian` code as small as possible, I
am interested in reducing the cognitive load required to read and write
those test fixtures. Therefore, the `parent=` way is much preferable.
Thank you!
Johannes
|
User |
The merge-replay tests added in a follow-up commit need a way to set
up specific topologies with full control over blob contents, parent
order, and per-side trees. Sequencing plumbing commands or driving
plain `git fast-import` from shell quickly becomes unreadable for
the kinds of scenarios that exercise non-trivial merge resolution
(textual conflicts, semantic edits outside the conflict region,
intentional limitations such as new content on one side).
Add a small `test-tool historian` subcommand that reads a tight,
shell-quoted, one-line-per-object DSL and feeds an equivalent stream
to a `git fast-import` child process. Each blob and commit is given
a logical name; the helper allocates fast-import marks on first use
and emits a lightweight tag for every commit so tests can refer to
the resulting object via `refs/tags/<name>`.
The DSL has just two directives:
blob NAME LINE...
commit NAME BRANCH SUBJECT [from=NAME] [merge=NAME]... [PATH=BLOB]...
A blob's content is the listed lines joined with `\n` (and a final
`\n`); a commit's tree is exactly the listed PATH=BLOB pairs (the
helper emits a `deleteall` so nothing leaks in from the implicit
parent). Token splitting is delegated to `split_cmdline()` so quoted
arguments work as in shell. Marks for parent references and file
contents go through the same `strintmap`-backed name resolver, which
keeps the helper itself trivially small: blob writing, tree
construction, commit creation and merge-base computation are all
handled by `git fast-import`.
Note that the DSL reserves the names `from` and `merge` (with a
trailing `=`) for parent specification; a tree path called `from` or
`merge` cannot be expressed via this helper. That is acceptable here
because every input is a tightly controlled test fixture and the
filenames are chosen by the test author.
The helper trusts its caller: malformed input results in a
fast-import error rather than a friendly diagnostic.
Wire the new subcommand into the Makefile and meson build, register
it in `t/helper/test-tool.{c,h}`.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
Johannes Schindelin wrote on the Git mailing list (how to reply to this email): 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. |
Before adding support for replaying merge commits to `git replay` and
`git history`, spell out the desired behavior. Each test describes a
self-contained topology built with the `historian` helper and asserts a
single property of the replayed merge.
The feature surface covered:
* Clean merges (each side touches unrelated files): the replay must
reconstruct the merge and leave the user's resolution intact.
* Non-trivial merges with a manual conflict resolution or a semantic
edit outside the conflict region: the user's contribution must
survive the replay.
* Octopus merges in the rewrite path are rejected loudly rather than
corrupted or treated as 2-parent merges by mistake.
* Merges whose first parent sits outside the rewrite range keep that
parent as-is while only the in-range parent is rewritten.
* Symbol-level intent is not extrapolated: a rename the user did
inside the original merge is preserved on files that existed at the
time, but files that only enter the tree via the rebased parents are
not retroactively renamed.
* A conflict that the rebased parent newly introduces against the
unchanged parent (a region the original merge did not conflict on)
is reported as a content conflict, not silently embedded as plain
marker bytes in the replayed commit. The same property must hold
when the affected file is routed through a binary merge driver.
* A file whose plain content happens to contain conflict-marker byte
sequences is left alone, not mistaken for a real conflict.
* Under `merge.conflictStyle=diff3`, a conflict that the original
merge already resolved still collapses cleanly through the outer
replay merge even when the inner remerges of the original and the
rebased parents have different merge bases (which makes the
abbreviated-OID line in the diff3 marker differ between them).
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The imminent 2-parent merge replay support implements a merge commit's replay as an outer three-way merge between three trees the inner machinery already knows how to compute: the auto-merge of the original parents, the original merge's actual tree, and the auto-merge of the rebased parents. Either of the auto-merges can carry unresolved conflicts -- residual conflict markers from the inner xdl_merge calls, or a non-resolution from a binary or custom driver -- and the outer merge has no way of telling those conflict bytes apart from any other content unless the inner merge that produced them tells it. Let the xdiff machinery optionally populate an array of the ranges where conflicts were recorded. A `struct xdl_conflict_interval` is a half-open byte range identifying one complete conflict-marker block in the result buffer that xdl_merge produced; a `struct xdl_conflict_intervals` is the growable list of those, owned by the caller and released with the new xdl_conflict_intervals_release() helper. merge-ort's collect_merge_info_callback() and process_entry() both have shortcuts that take side2's blob verbatim when side1 matches the merge base. Both now consult in_replay_side2_intervals: if the strmap has an entry for the path, the shortcut is skipped (collect_merge_info_callback) or the result is marked conflicted with a CONFLICT (content) message (process_entry). For the non-xdiff drivers (binary, custom, union, ours/theirs) merge_3way() stores an empty interval list with `nr == 0` paired with a non-clean ll_merge status, so the presence-not-content check above also catches a fresh binary conflict for which no marker bytes were emitted. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
`git history` (introduced in v2.54) and the underlying `git replay`
infrastructure both refused to walk past any commit with more than
one parent, dying with "replaying merge commits is not supported
yet!". For real history-rewriting work this is a showstopper: the
natural fallback `git rebase --rebase-merges` is interactive and
stops to ask for re-resolution even when no re-resolution is
needed.
Lift that limitation for the 2-parent case. A merge commit M on
parents (P1, P2) records both an automatic merge of those parents
AND any manual layer the author put on top of that automatic merge
(textual conflict resolution and any semantic edit outside conflict
markers). Replaying M onto rewritten parents (P1', P2') must
preserve that manual layer, but the rewritten parents change the
automatic merge, so a simple cherry-pick is wrong: the manual layer
would land on top of stale auto-merge text.
What works instead is a three-way merge of three trees the existing
infrastructure already knows how to compute. Let R be the recursive
auto-merge of (P1, P2), O be M's actual tree and N be the recursive
auto-merge of (P1', P2'). `git diff R O` is morally `git show
--remerge-diff M`: it captures exactly what the author added on top
of the automatic merge. A non-recursive 3-way merge with R as the
merge base, O as side 1 and N as side 2 layers that manual
contribution onto the freshly auto-merged rewritten parents and
produces the replayed tree.
Implement `pick_merge_commit()` along those lines and dispatch to
it from `replay_revisions()` when the commit being replayed has
exactly two parents. Three details keep the non-trivial cases
sound:
1. R and N use identical `merge_options.branch1` / `branch2`
labels ("ours"/"theirs"). When the original parents conflicted
on a region of a file, both R and N produce textually identical
conflict markers; the outer non-recursive merge then sees N
equal to R in that region and the user's manual resolution from
O wins cleanly. Without this, the conflict-marker text would
differ between R and N (the inner merges would label the
conflicts differently), and the outer merge would itself be
unclean even when the user did supply a clean resolution.
2. An unclean inner merge (`result.clean == 0`) is not fatal: the
tree merge-ort produces in that case still has well-defined
contents (with conflict markers in the conflicted files) and is
a valid input to the outer non-recursive merge. Only a real
error (`< 0`) propagates as failure.
3. Each inner merge publishes the byte ranges of every
conflict-marker hunk it emits via the new
`out_replay_intervals` strmap on `merge_options`. The outer
merge receives both maps via `in_replay_orig_intervals` and
`in_replay_side2_intervals`, so xdl_merge can detect a hunk in
N that has no byte-equal counterpart in R (a conflict the
original parents did not have) and surface it as a real
conflict rather than absorbing the marker bytes as plain
content. The same channel makes merge-ort's trivial fast paths
refuse to take side2's blob verbatim for paths whose inner
merge was unresolved, which also covers paths handled by
binary or other non-textual drivers.
The replay propagates the textual diffs the user actually made in
M; it does not extrapolate symbol-level intent. If rewriting the
parents pulls in genuinely new content (for example, a brand-new
caller of a function that the merge renamed), that new content
stays as the rewritten parents have it. Symbol-aware refactoring
is out of scope, just as it is for plain rebase.
Octopus merges (more than two parents) and revert-of-merge are
not supported and are surfaced as explicit errors at the dispatch
point. The "split" sub-command of `git history` continues to
refuse when its target is a merge; split semantics do not apply
there. The pre-walk gate in `builtin/history.c` that previously
rejected any merge in the rewrite path now only rejects octopus
merges; rename it accordingly.
The walk in replay_revisions() pre-seeds the rewritten-commit map
with each negative-side ("BOTTOM") cmdline entry pointing at
`onto`. This matters for merge replay: a 2-parent merge whose
first parent is the boundary (e.g. the commit being reworded) must
replay onto the rewritten boundary, but pick_merge_commit's
mapped_commit fallback would return the original parent for
anything not in the map. Pre-seeding the boundary makes the lookup
return `onto`, which is the rewritten counterpart. Skip the
pre-seed in revert mode, which chains reverts through last_commit
and would short-circuit incorrectly.
A small refactor in `create_commit()` makes the merge case
possible: the helper now takes a `struct commit_list *parents`
rather than a single parent pointer and takes ownership of the
list. The single existing caller in `pick_regular_commit()` builds
and passes a one-element list; the new `pick_merge_commit()`
builds a two-element list, with parent order preserved.
Update the negative expectations in t3451, t3452 and t3650 that
were asserting the now-retired "not supported yet" message,
replacing them with positive coverage where it fits. The 2-parent
merge scenarios in t3454 (introduced as failing baselines in an
earlier commit) flip to test_expect_success, with the exception
of the `merge.conflictStyle=diff3` case that needs an additional
ancestor-label fix in a subsequent commit.
Helped-by: Elijah Newren <newren@gmail.com>
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…hanged For the common `git history reword` case the rewrite changes only commit messages, so every commit on the line being replayed has the same tree as before. When such a rewrite reaches a 2-parent merge whose rewritten parents AND merge bases all carry the same trees as the originals, the inner auto-merge of the rewritten parents (N) is tree-equal to the inner auto-merge of the original parents (R), and the outer 3-way merge with R as the merge base, the original merge tree as side 1 and N as side 2 yields the original tree as result. Detect this in `pick_merge_commit()` before doing any merge work and write the new merge commit directly with the original tree and the rewritten parents. This saves two recursive merges and one non-recursive merge per merge commit on the rewrite path, which dominates the cost of `git history reword` across histories with many merges. The merge-base trees must be checked too, in order. Tree-same parents over a tree-different base could still produce a different auto-merge (a conflict region that did not exist before, or vice versa), and the original resolution would be inappropriate to apply. To avoid recomputing the merge bases when the fast path does not apply, both pairs are computed up front and the slow path that follows reuses them. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The previous commits keep the inner remerges' branch labels constant
("ours" and "theirs") so that conflict-marker text is byte-identical
between R and N whenever both inner merges produce the same conflict
for a given region. Under the default merge.conflictStyle=merge that
is enough: the marker hunks compare byte-equal, and the outer
non-recursive merge lets the user's manual resolution from O pass
through cleanly.
It breaks under merge.conflictStyle=diff3. The diff3-style marker
embeds the merge base's abbreviated OID via opt->ancestor, which
merge_incore_recursive auto-derives from the single merge base when
the caller does not set it. For a topology where merge_base(P1, P2)
and merge_base(P1', P2') resolve to different commits, the
auto-derived label differs between R and N even though the inner
merges produced byte-identical content for the actual conflicting
region. The outer 3-way merge then sees an R-vs-N delta on the
ancestor-label line that overlaps with the R-vs-O delta covering the
user's resolution, and xdl_merge reports a conflict where there
should have been a clean fall-through.
Fix it the same way the side labels are pinned: set opt->ancestor to
a constant string on both inner merge_options before calling
merge_incore_recursive. The resulting marker bytes then match
regardless of which commit happens to be each pair's merge base, and
the outer merge takes the user's manual resolution from O.
Promote the diff3 test in t3454 from test_expect_failure to
test_expect_success.
Note: This obsoletes an assertion that was introduced specifically to
ensure that such a label is only provided by `git am`, for single merge
bases. This assertion is now no longer valid and is hence dropped.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Replace the blanket "does not (yet) work with histories that contain merges" caveat now that 2-parent merges are supported via the R/O/N algorithm. Spell out what works (the user's manual conflict resolution and any semantic edits inside the merge are preserved through the replay), what is intentionally out of scope (octopus merges; symbol-level extrapolation when rewriting parents pulls in genuinely new content), and what still requires interactive rebase (merges that would actually conflict on replay). The LIMITATIONS section also covers the newly-introduced-conflict case: when only one parent of a merge has been rewritten and the rewritten parent conflicts with the unchanged parent in a region the original parents did not conflict on, the replay stops and reports a conflict instead of silently absorbing the rewritten side's content. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
support-merge-commits-in-git-history
branch
from
2dec28b to
ebc7187
Compare
back to 3-way merge of the other parent's pre-/post-replay variants Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
There is an issue in commit 2ac4d0e:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
git history, the new history-rewriting builtin in v2.54, dies on any merge in the rewrite path withreplaying merge commits is not supported yet!. That makes it not very useful for the workflows I actually have, where almost every interesting branch contains at least one merge of a feature topic. The natural fallback,git rebase --rebase-merges, is interactive and stops to ask for re-resolution even when no re-resolution is needed.This series lifts that limitation for the common 2-parent case. The algorithm itself is not new: Elijah Newren wrote it down in his replay design notes and prototyped it in a 2022 work-in-progress sketch. What is new is wiring it into the
replay_revisions()API that backs bothgit replayandgit history, plus three specific tweaks that make the trickier cases work where the WIP sketch bailed out: identical conflict-marker labels for the inner remerges of the original and the rewritten parents (so their conflict-markered trees compare equal in the regions the user did not touch), toleratingresult.clean == 0from those inner merges (their well-defined conflict-markered trees are valid inputs to the outer 3-way merge), and self-fallback for both merge parents combined with mapping the rev-range boundary commits to theontocommit.Octopus merges and revert-of-merge are surfaced as explicit errors at the dispatch point. The
splitsub-command ofgit historycontinues to refuse when its target is a merge: split semantics simply do not apply there. The xdiff special mode for matching conflict-marker hunks across inner remerges, theXDL_MERGE_FAVOR_BASEvariant, and the modify/delete and binary-file specials that the design notes flag as future work all remain future work.While I was at it,
git history rewordhad a pre-existing silent-success bug: a positive return fromreplay_revisions()(which means "conflict, no updates queued") was treated as success. Obviously this should never occur, as arewordsimply 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.Cc: Elijah Newren newren@gmail.com
Cc: Patrick Steinhardt ps@pks.im
cc: "D. Ben Knoble" ben.knoble@gmail.com
cc: Phillip Wood phillip.wood123@gmail.com
cc: Toon Claes toon@iotcl.com