replay: support replaying 2-parent merges
Subsystems (3)
cover replay: support replaying 2-parent merges
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.
Johannes Schindelin (5):
replay: support replaying 2-parent merges
replay: short-circuit merge replay when parent and base trees are
unchanged
history.adoc: describe merge-replay support and its limits
test-tool: add a "historian" subcommand for building merge fixtures
t3454: cover merge-replay scenarios with the historian helper
Documentation/git-history.adoc | 27 ++- Makefile | 1 + builtin/history.c | 16 +- replay.c | 258 +++++++++++++++++++++++++-- t/helper/meson.build | 1 + t/helper/test-historian.c | 189 ++++++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/meson.build | 1 + t/t3451-history-reword.sh | 21 ++- t/t3452-history-split.sh | 6 +- t/t3454-history-merges.sh | 308 +++++++++++++++++++++++++++++++++ t/t3650-replay-basics.sh | 46 ++++- 13 files changed, 840 insertions(+), 36 deletions(-) create mode 100644 t/helper/test-historian.c create mode 100755 t/t3454-history-merges.sh
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2106%2Fdscho%2Fsupport-merge-commits-in-git-history-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2106/dscho/support-merge-commits-in-git-history-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2106
Signature · 1 line
gitgitgadget
PATCH 1/5 replay: support replaying 2-parent merges
From: Johannes Schindelin <[email protected]>
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.
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.
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
--- 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(-)
builtin/history.c +11 -5
@@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt, const char *value, int unsreturn 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 ;
replay.c +199 -10
257 lines not on this page · raw message
t/t3451-history-reword.sh +15 -6
@@ -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
t/t3452-history-split.sh +2 -4
@@ -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 ) '
t/t3650-replay-basics.sh +42 -4
52 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Hi Johannes
Quoted 12 lines · Johannes Schindelin via GitGitGadget
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.
Quoted 9 lines
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 Ois morallygit 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 /
|/
Othen 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
Quoted 511 lines
Implement
pick_merge_commit()along those lines and dispatch to it fromreplay_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 identicalmerge_options.branch1andbranch2labels ("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 historycontinues to refuse when the targeted commit is itself a merge: split semantics do not apply to merges. The pre-walk gate inbuiltin/history.cthat 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 astruct commit_list *parentsrather than a single parent pointer and takes ownership of the list. The single existing caller inpick_regular_commit()builds and passes a one-element list; the newpick_merge_commit()builds a two-element list, with the order of thefromandmergeparents 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/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45Helped-by Elijah Newren <[email protected]>Assisted-by Claude Opus 4.7Signed-off-by Johannes Schindelin <[email protected]>--- 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(-)builtin/history.c +11
-5@@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt, const char *value, int unsreturn 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 ;replay.c +199
-10@@ -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 ;t/t3451-history-reword.sh +15
-6@@ -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 EOFt/t3452-history-split.sh +2
-4@@ -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 ) 't/t3650-replay-basics.sh +42
-4@@ -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' '
Quoted 36 lines · Phillip Wood
Hi Johannes
Quoted 12 lines · Johannes Schindelin via GitGitGadget
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.
Quoted 9 lines
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 Ois morallygit 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
Quoted 7 lines
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
Quoted 522 lines
Thanks
Phillip
Quoted 517 lines
Implement
pick_merge_commit()along those lines and dispatch to it fromreplay_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 identicalmerge_options.branch1andbranch2labels ("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 historycontinues to refuse when the targeted commit is itself a merge: split semantics do not apply to merges. The pre-walk gate inbuiltin/history.cthat 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 astruct commit_list *parentsrather than a single parent pointer and takes ownership of the list. The single existing caller inpick_regular_commit()builds and passes a one-element list; the newpick_merge_commit()builds a two-element list, with the order of thefromandmergeparents 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/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45Helped-by Elijah Newren <[email protected]>Assisted-by Claude Opus 4.7Signed-off-by Johannes Schindelin <[email protected]>---
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(-)builtin/history.c +11
-5@@ -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(structrepository *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;replay.c +6
-0@@ -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,
+ structcommit_list*parents,
enumreplay_modemode)
{
structobject_idret;
struct object *obj = NULL;
- structcommit_list*parents = NULL;
char *author = NULL;
char*sign_commit= NULL; /* FIXME: cli users might want to sign again */ structcommit_extra_header*extra = NULL;replay.c +1
-1@@ -96,7 +103,6 @@ static struct commit *create_commit(structrepository *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-
Quoted 1 line
object.oid) &&
!oideq(&pickme_tree->object.oid,
&base_tree->object.oid)) returnreplayed_base;
- returncreate_commit(repo, result->tree, pickme, replayed_base, mode);
+commit_list_insert(replayed_base, &parents);
+ returncreate_commit(repo, result->tree, pickme, parents, mode);replay.c +4
-0+} + +/* +* 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 Ois 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.txton thereplaybranch of newren/git).
+ *
+ * If the outer 3-way merge is unclean, propagate the conflict status to
+ * the caller viaresult->clean = 0and 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.
+ */replay.c +28
-0+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 bymerge_incore_recursive*/ + if (remerge_res.clean < 0) {
+result->clean= remerge_res.clean;
+ goto out;
+ }replay.c +52
-0+ + /* 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;}
voidreplay_result_release(struct replay_result *result)replay.c +49
-6@@ -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;t/t3451-history-reword.sh +0
-0@@ -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_failgit -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_messageHEAD~ <<-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_messageHEAD <<-EOF&&
Reworded merge commit
EOFt/t3451-history-reword.sh +0
-0@@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' ' * Reworded merge commit |\| * theirs
- * | ours
+ * | ours reworded
|/
* base
EOFt/t3452-history-split.sh +0
-0@@ -36,7 +36,7 @@ expect_tree_entries () {
test_cmpexpect actual
}t/t3452-history-split.sh +1
-1-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 &&
(t/t3452-history-split.sh +0
-0@@ -49,9 +49,7 @@ test_expect_success 'refuses to work with mergecommits' '
git switch - &&
git merge theirs &&
test_must_failgit history split HEAD 2>err &&
-test_grep"cannot split up merge commit" err &&
-test_must_failgit history split HEAD~ 2>err &&
-test_grep"replaying merge commits is not supported yet" err +test_grep"cannot split up merge commit" err
)
't/t3650-replay-basics.sh +0
-0@@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ...ordering would be ill-defined' '
test_cmpexpect actual
'
-test_expect_success 'replaying merge commits is not supported yet' ' - echo "fatal: replaying merge commits is not supported yet!"Quoted 1 line
expect &&
-
test_must_failgit replay --advance=main main..topic-with-merge 2>actual &&
-test_cmpexpect actual
+test_expect_success 'using replay to rebase a 2-parent merge' ' + # main..topic-with-merge contains a 2-parent merge (P) introduced + # viatest_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-mergeQuoted 1 line
result &&
+
test_line_count= 1 result &&t/t3650-replay-basics.sh +34
-0+ + 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' '
Hi Phillip,
Quoted 61 lines · Phillip Wood
Quoted 48 lines · Phillip Wood
Quoted 12 lines · Johannes Schindelin via GitGitGadget
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.
Quoted 9 lines
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 Ois morallygit 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 /
|/
Othen 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.
Quoted 11 lines
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.
Quoted 4 lines
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
Quoted 532 lines
Thanks
Phillip
Quoted 524 lines
Thanks
Phillip
Quoted 519 lines
Implement
pick_merge_commit()along those lines and dispatch to it fromreplay_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 identicalmerge_options.branch1andbranch2labels ("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 historycontinues to refuse when the targeted commit is itself a merge: split semantics do not apply to merges. The pre-walk gate inbuiltin/history.cthat 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 astruct commit_list *parentsrather than a single parent pointer and takes ownership of the list. The single existing caller inpick_regular_commit()builds and passes a one-element list; the newpick_merge_commit()builds a two-element list, with the order of thefromandmergeparents 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/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45Helped-by Elijah Newren <[email protected]>Assisted-by Claude Opus 4.7Signed-off-by Johannes Schindelin <[email protected]>---
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(-)builtin/history.c +11
-5@@ -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;replay.c +6
-0@@ -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,
+ structcommit_list*parents,
enumreplay_modemode)
{
structobject_idret;
struct object *obj = NULL;
- structcommit_list*parents = NULL;
char *author = NULL;
char*sign_commit= NULL; /* FIXME: cli users might want to sign again */
structcommit_extra_header*extra = NULL;replay.c +1
-1@@ -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-
Quoted 1 line
object.oid) &&
!oideq(&pickme_tree->object.oid,
&base_tree->object.oid)) returnreplayed_base;
- returncreate_commit(repo, result->tree, pickme, replayed_base, mode);
+commit_list_insert(replayed_base, &parents);
+ returncreate_commit(repo, result->tree, pickme, parents, mode);replay.c +4
-0+} + +/* +* 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 Ois 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.txton thereplaybranch of newren/git).
+ *
+ * If the outer 3-way merge is unclean, propagate the conflict status to
+ * the caller viaresult->clean = 0and 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.
+ */replay.c +46
-0+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 bymerge_incore_recursive*/ + if (remerge_res.clean < 0) {
+result->clean= remerge_res.clean;
+ goto out;
+ }replay.c +52
-0+ + /* 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;}
voidreplay_result_release(struct replay_result *result)replay.c +49
-6@@ -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;t/t3451-history-reword.sh +0
-0@@ -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_failgit -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_messageHEAD~ <<-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_messageHEAD <<-EOF&&
Reworded merge commit
EOFt/t3451-history-reword.sh +0
-0@@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' ' * Reworded merge commit |\| * theirs
- * | ours
+ * | ours reworded
|/
* base
EOFt/t3452-history-split.sh +0
-0@@ -36,7 +36,7 @@ expect_tree_entries () {
test_cmpexpect actual
}t/t3452-history-split.sh +1
-1-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 &&
(t/t3452-history-split.sh +0
-0@@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge commits''
git switch - &&
git merge theirs &&
test_must_failgit history split HEAD 2>err &&
-test_grep"cannot split up merge commit" err &&
-test_must_failgit history split HEAD~ 2>err &&
-test_grep"replaying merge commits is not supported yet" err +test_grep"cannot split up merge commit" err
)
't/t3650-replay-basics.sh +0
-0@@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ...ordering would be ill-defined' '
test_cmpexpect actual
'
-test_expect_success 'replaying merge commits is not supported yet' ' - echo "fatal: replaying merge commits is not supported yet!"Quoted 1 line
expect &&
-
test_must_failgit replay --advance=main main..topic-with-merge 2>actual &&
-test_cmpexpect actual
+test_expect_success 'using replay to rebase a 2-parent merge' ' + # main..topic-with-merge contains a 2-parent merge (P) introduced + # viatest_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-mergeQuoted 1 line
result &&
+
test_line_count= 1 result &&t/t3650-replay-basics.sh +34
-0+ + 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' '
Hi Phillip,
Quoted 38 lines · Phillip Wood
Quoted 12 lines · Johannes Schindelin via GitGitGadget
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.
Quoted 9 lines
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 Ois morallygit 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 / |/ Othen running
Quoted 8 lines
git replay --onto E --ancestry-path B..Owill 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!
Quoted 4 lines
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
Quoted 8 lines
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
Quoted 534 lines
Thanks
Phillip
Quoted 527 lines
Implement
pick_merge_commit()along those lines and dispatch to it fromreplay_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 identicalmerge_options.branch1andbranch2labels ("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 historycontinues to refuse when the targeted commit is itself a merge: split semantics do not apply to merges. The pre-walk gate inbuiltin/history.cthat 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 astruct commit_list *parentsrather than a single parent pointer and takes ownership of the list. The single existing caller inpick_regular_commit()builds and passes a one-element list; the newpick_merge_commit()builds a two-element list, with the order of thefromandmergeparents 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/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45Helped-by Elijah Newren <[email protected]>Assisted-by Claude Opus 4.7Signed-off-by Johannes Schindelin <[email protected]>--- 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(-)builtin/history.c +0
-0@@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt,const char *value, int uns
return 0;
}builtin/history.c +3
-3-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)) {builtin/history.c +0
-1- ret = error(_("replaying merge commits is not supportedyet!"));
builtin/history.c +4
-0+ 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 werenot rewritten"));
builtin/history.c +4
-0+ 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;replay.c +10
-2@@ -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;replay.c +137
-1@@ -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)replay.c +38
-5@@ -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 notsupported"));
replay.c +4
-0+ goto out; + } +if (mode ==REPLAY_MODE_REVERT ) { + ret = error(_("reverting merge commits is notsupported"));
replay.c +7
-0+ 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,replay.c +1
-0+ mode ==
REPLAY_MODE_REVERT?last_commit: onto,replay.c +1
-0+ &merge_opt, &result,mode);
replay.c +1
-0+ }if (!last_commit)break ; diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.shindex
de7b357685..d103f866a2100755t/t3451-history-reword.sh +0
-2@@ -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 &&
t/t3451-history-reword.sh +0
-1- test_grep "replaying merge commits is not supported yet" err&&
t/t3451-history-reword.sh +14
-0+ # 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
t/t3451-history-reword.sh +1
-1- * | ours + * | ours reworded|/
* base
EOFt/t3452-history-split.sh +2
-4@@ -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 ) 't/t3650-replay-basics.sh +0
-0@@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ...ordering would be ill-defined' '
test_cmpexpect actual
't/t3650-replay-basics.sh +0
-3-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-merge2>actual &&
t/t3650-replay-basics.sh +5
-1- 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-mergeQuoted 1 line
result &&
t/t3650-replay-basics.sh +35
-0+ 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\
t/t3650-replay-basics.sh +2
-0+ 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' '
replay.c static struct commit *pick_regular_commit(struct repository *repo, · line 470
Quoted 8 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]> [snip]
replay.c +4
-0+out: +free (ancestor_name); +free_commit_list (parent_bases); +free_commit_list (replayed_bases);
free_commit_list is deprecated in favor of commit_list_free since 52882024 (Merge branch 'ps/commit-list-functions-renamed', 2026-02-13).
Quoted 5 lines
replay.c +3
-0+merge_finalize (&remerge_opt, &remerge_res); +merge_finalize (&new_merge_opt, &new_merge_res); +return picked; }
Hi Dscho,
Quoted 2 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
—8<—
Quoted 13 lines
---
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(-)replay.c +0
-0
replay.c line 2
Quoted 2 lines
replay.c +0
-0@@ -1,6 +1,7 @@#define USE_THE_REPOSITORY_VARIABLE
—8<—
Quoted 4 lines
replay.c +4
-0+/* +* 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
replay.c static struct commit *pick_regular_commit(struct repository *repo, · line 354
Quoted 10 lines
replay.c +10
-0+* 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).
Maybe we would want to add these design notes as a technical document when this series is out of RFC ? This way we refer to our own documentation.
—8<—
Quoted 29 lines
replay.c +25
-0@@ -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,
replay.c int replay_revisions(struct rev_info *revs, · line 587
Quoted 10 lines
replay.c +10
-0+ &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; + } + } +
I understand the intent here, we seed the replayed commit map with onto for the negative revisions, so that a merge with its first parent at the boundary gets its first parent rewritten to onto, but keeps its second parent. But I think a further tweak is necessary.
I tried the following:
- amend the first parent of a merge M (manually, in detached HEAD), changing only the commit message
- try to replay the merge. Since none of the trees have changed, and the merge-base hasn’t changed, I expect replay to succeed and keep manually resolutions from M. I ran this:
git replay --onto HEAD M^!
and because with this syntax M^2 is a negative revision, it gets mapped to onto, whereas from what I understand it should _not_ be mapped and should be left untouched by replay: the new merge should have M^2 as second parent.
Since the second parent gets mapped to onto, the merge-base of the replayed merge is changed, so the fast past is skipped, the new merge conflicts and the replay fails.
Also, I think this pre-seeding should be mentionned in the commit message.
Thank you for working on this,and Elijah for prototyping it. It’s already is a massive time saver for me.
Philippe.
PATCH 2/5 replay: short-circuit merge replay when parent and base trees are unchanged
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- replay.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 9 deletions(-)
replay.c +58 -9
@@ -396,6 +396,64 @@ static struct commit *pick_merge_commit(struct repository *repo, replayed_par1 =mapped_commit (replayed_commits, parent1, parent1); replayed_par2 =mapped_commit (replayed_commits, parent2, parent2); +/* +* Compute both pairs of merge bases up front. The fast path below +* needs them for the tree-equality check, and the slow path that +* follows reuses them to avoid recomputing. +*/ +if (repo_get_merge_bases (repo, parent1, parent2, &parent_bases) <0 || +repo_get_merge_bases (repo, replayed_par1, replayed_par2, + &replayed_bases) <0 ) { + result->clean =-1 ; + goto out; + } + +/* +* Fast path: when both rewritten parents carry the same trees as +* the originals AND every merge base does too (in order), the +* auto-merges R and N would be tree-equal (their inputs match +* content-wise), so the outer 3-way merge trivially yields the +* original merge's tree. Skip the inner merges and write the new +* merge commit directly. +* +* This is the common case for `git history reword`, which only +* changes commit messages and so leaves every tree on the line +* being replayed unchanged. The merge-base trees must be checked +* too: 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. +*/ +if (oideq (&repo_get_commit_tree (repo, parent1)->object.oid, + &repo_get_commit_tree (repo, replayed_par1)->object.oid) && +oideq (&repo_get_commit_tree (repo, parent2)->object.oid, + &repo_get_commit_tree (repo, replayed_par2)->object.oid)) { +struct commit_list *bo, *bn; +int bases_match =1 ; + +for (bo = parent_bases, bn = replayed_bases; + bo && bn; + bo = bo->next, bn = bn->next) { +if (!oideq (&repo_get_commit_tree (repo, bo->item)->object.oid, + &repo_get_commit_tree (repo, bn->item)->object.oid)) { + bases_match =0 ; +break ; + } + } +if (bo || bn) + bases_match =0 ; + +if (bases_match) { + pickme_tree =repo_get_commit_tree (repo, pickme); + parents =NULL ; +commit_list_insert (replayed_par2, &parents); +commit_list_insert (replayed_par1, &parents); + picked =create_commit (repo, pickme_tree, pickme, + parents,REPLAY_MODE_PICK ); + goto out; + } + } +/* * R: auto-remerge of the original parents. *
@@ -408,10 +466,6 @@ static struct commit *pick_merge_commit(struct repository *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 */
@@ -425,11 +479,6 @@ static struct commit *pick_merge_commit(struct repository *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 */
Signature · 1 line
gitgitgadget
PATCH 3/5 history.adoc: describe merge-replay support and its limits
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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).
--- Documentation/git-history.adoc | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)
Documentation/git-history.adoc +20 -7
@@ -40,13 +40,26 @@ at once. LIMITATIONS ----------- -This command does not (yet) work with histories that contain merges. You -should use linkgit:git-rebase[1] with the `--rebase-merges` flag instead. - -Furthermore, the command does not support operations that can result in merge -conflicts. This limitation is by design as history rewrites are not intended to -be stateful operations. The limitation can be lifted once (if) Git learns about -first-class conflicts. +This command supports two-parent merge commits in the rewrite path: +the auto-remerged tree of the original parents, the merge commit +itself, and the auto-merged tree of the rewritten parents are +combined so that the user's manual conflict resolution (textual or +semantic) is preserved through the replay. Octopus merges (more than +two parents) are not supported and are rejected with an error. + +The replay propagates the textual diffs the user actually made in +the merge commit. It does _not_ extrapolate symbol-level intent: if +rewriting the parents pulls in genuinely new content (for example, a +new caller of a function that the merge renamed), that new content +is _not_ rewritten by the replay and may need a follow-up edit. +Symbol-aware refactoring is out of scope here, just as it is for +plain rebase. + +The command does not support operations that can result in merge +conflicts on the replayed merge itself. This limitation is by design +as history rewrites are not intended to be stateful operations. Use +linkgit:git-rebase[1] with the `--rebase-merges` flag when the +rewrite is expected to require interactive conflict resolution. COMMANDS --------
Signature · 1 line
gitgitgadget
PATCH 4/5 test-tool: add a "historian" subcommand for building merge fixtures
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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}.
--- 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
Makefile +1 -0
@@ -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
t/helper/meson.build +1 -0
@@ -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',
t/helper/test-historian.c new file +189 -0
189 lines not on this page · raw message
t/helper/test-tool.c +1 -0
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = { { "hashmap", cmd__hashmap }, { "hash-speed", cmd__hash_speed }, { "hexdump", cmd__hexdump }, + { "historian", cmd__historian }, { "json-writer", cmd__json_writer }, { "lazy-init-name-hash", cmd__lazy_init_name_hash }, { "match-trees", cmd__match_trees },
t/helper/test-tool.h +1 -0
@@ -32,6 +32,7 @@ int cmd__getcwd(int argc, const char **argv);int cmd__hashmap (int argc,const char **argv);int cmd__hash_speed (int argc,const char **argv);int cmd__hexdump (int argc,const char **argv); +int cmd__historian (int argc,const char **argv);int cmd__json_writer (int argc,const char **argv);int cmd__lazy_init_name_hash (int argc,const char **argv);int cmd__match_trees (int argc,const char **argv);
Signature · 1 line
gitgitgadget
Re: [PATCH/RFC 4/5] test-tool: add a "historian" subcommand for building merge fixtures
Quoted 85 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
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-importfrom 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 historiansubcommand that reads a tight, shell-quoted, one-line-per-object DSL and feeds an equivalent stream to agit fast-importchild 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 viarefs/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 adeleteallso nothing leaks in from the implicit parent). Token splitting is delegated tosplit_cmdline()so quoted arguments work as in shell. Marks for parent references and file contents go through the samestrintmap-backed name resolver, which keeps the helper itself trivially small: blob writing, tree construction, commit creation and merge-base computation are all handled bygit fast-import.Note that the DSL reserves the names
fromandmerge(with a trailing=) for parent specification; a tree path calledfromormergecannot 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
Makefileand meson build, register it int/helper/test-tool.{c,h}.Assisted-by Claude Opus 4.7Signed-off-by Johannes Schindelin <[email protected]>--- 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.cMakefile +1
-0@@ -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.ot/helper/meson.build +1
-0@@ -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',t/helper/test-historian.c new file +0
-0
t/helper/test-historian.c line 8
Quoted 8 lines
t/helper/test-historian.c new file +8
-0@@ -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.
t/helper/test-historian.c line 17
Quoted 9 lines
t/helper/test-historian.c new file +9
-0+* +* 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?
t/helper/test-historian.c line 22
Quoted 5 lines
t/helper/test-historian.c new file +5
-0+* 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.
Signature · 2 lines
Cheers, Toon
Re: [PATCH/RFC 4/5] test-tool: add a "historian" subcommand for building merge fixtures
Hi Toon,
Quoted 11 lines · Toon Claes
Quoted 9 lines · Johannes Schindelin via GitGitGadget
t/helper/test-historian.c new file +3
-0@@ -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
Quoted 8 lines
t/helper/test-historian.c line 8
Quoted 5 lines
t/helper/test-historian.c new file +5
-0+* 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.
Quoted 10 lines
t/helper/test-historian.c line 17
Quoted 9 lines
t/helper/test-historian.c new file +9
-0+* +* 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]...
Quoted 5 lines
I'm not sure how I feel about mixing named arguments (like
from=PARENT) with thePATH=BLOBarguments? 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 thePATH:BLOBsyntax 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!
Quoted 41 lines
t/helper/test-historian.c line 22
Quoted 5 lines
t/helper/test-historian.c new file +5
-0+* 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=andmerge=withparent=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 abranch=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 touchesgonly; main toucheshonly. 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=hI 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
Quoted 2 lines
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
PATCH 5/5 t3454: cover merge-replay scenarios with the historian helper
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Add a dedicated test script for git history reword (and git replay via the same code path) across 2-parent merges, using the test-tool historian fixture builder so each scenario reads as a small declarative recipe rather than a sequence of plumbing commands.
The script exercises the cases that motivated the merge-replay work:
* a clean merge where each side touches unrelated files;
* a non-trivial merge where the same line was changed on both sides and the user resolved by hand (textual manual resolution must be preserved through the replay);
* a non-trivial merge where the user also touched a line outside any conflict region (a "semantic" edit must also be preserved through the replay);
* an octopus merge in the rewrite path, which is rejected;
* a function rename across the merge with a brand-new caller introduced by the rewritten parents. The pre-existing caller that the user manually renamed in the original merge must keep its rename, and the brand-new caller must _not_ be rewritten (calvin/hobbes naming chosen for legibility). This second part is the documented limitation: the replay propagates the textual diffs the user actually made, it does not extrapolate symbol-level intent. Symbol-aware refactoring is out of scope, just as it is for plain rebase.
The fixture builder lets each scenario sit in roughly a dozen lines of historian directives plus the assertions, which keeps the test file readable when more scenarios are added later.
--- t/meson.build | 1 + t/t3454-history-merges.sh | 308 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 309 insertions(+) create mode 100755 t/t3454-history-merges.sh
t/meson.build +1 -0
@@ -397,6 +397,7 @@ integration_tests = [ 't3450-history.sh', 't3451-history-reword.sh', 't3452-history-split.sh', + 't3454-history-merges.sh', 't3500-cherry.sh', 't3501-revert-cherry-pick.sh', 't3502-cherry-pick-merge.sh',
t/t3454-history-merges.sh new file +308 -0
308 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Hi Dscho,
Quoted 31 lines · Johannes Schindelin via GitGitGadget
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 thereplay_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_BASEvariant, and the modify/delete and binary-file specials that the design notes flag as future work all remain future work.
Quoted 6 lines
While I was at it, git history reword had 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 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,
Signature · 1 line
D. Ben Knoble
Hi Ben,
Quoted 13 lines · D. Ben Knoble
Quoted 10 lines · Johannes Schindelin via GitGitGadget
[...]
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.
Quoted 2 lines
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;
Quoted 1 line
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
Quoted 37 lines · Johannes Schindelin
Hi Ben,
Quoted 15 lines · D. Ben Knoble
Quoted 6 lines
On Wed, May 6, 2026 at 6:44 PM Johannes Schindelin via GitGitGadget <[email protected]> wrote:
[...]
Quoted 7 lines
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.Quoted 2 lines
Fixing this bug sounded interesting; I had a hard time spotting it while skimming the first 2 patches.
It's this part:
(quoted diff) +3
-0@@ -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;
Review static int cmd_history_split(int argc, · line 732
Quoted 9 lines
(quoted diff) +3
-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.
Quoted 6 lines
Quoted 1 line
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 rewordisn't supposed to be able to result in merge conflicts, nor isgit history splitbecause they leave the respective commits tree-same (in thesplitcase, the second commit).
I seem to recall Patrick making a similar argument, but don’t let me put words in anyone’s mouth.
Quoted 7 lines
I could see the point were anybody to suggest using
BUG()instead oferror()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.
Quoted 8 lines
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.
Hi Ben,
Quoted 41 lines · Ben Knoble
Quoted 41 lines · Johannes Schindelin
Quoted 16 lines · D. Ben Knoble
Quoted 6 lines
On Wed, May 6, 2026 at 6:44 PM Johannes Schindelin via GitGitGadget <[email protected]> wrote:
[...]
Quoted 7 lines
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.Quoted 2 lines
Fixing this bug sounded interesting; I had a hard time spotting it while skimming the first 2 patches.
It's this part:
(quoted diff) +6
-0@@ -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; }
Quoted 5 lines
Review static int cmd_history_split(int argc, · line 732
Quoted 1 line
(quoted diff) +0
-0ret = 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
Quoted 27 lines
Quoted 6 lines
Quoted 1 line
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 rewordisn't supposed to be able to result in merge conflicts, nor isgit history splitbecause they leave the respective commits tree-same (in thesplitcase, the second commit).I seem to recall Patrick making a similar argument, but don’t let me put words in anyone’s mouth.
Quoted 7 lines
I could see the point were anybody to suggest using
BUG()instead oferror()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.
Quoted 8 lines
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,
JohannesMakes sense, thanks.