From: Elijah Newren Date: Tue, 16 Dec 2025 00:21:16 GMT Subject: Re: [PATCH v2] replay: drop commits that become empty Message-ID: In-Reply-To: <9a81644a0ec670261a85c155fa32e5a1f4576ef4.1765793254.git.phillip.wood@dunelm.org.uk> On Mon, Dec 15, 2025 at 2:07 AM Phillip Wood wrote: > > From: Phillip Wood > > If the changes in a commit being replayed are already in the branch > that the commits are being replayed onto then "git replay" creates an > empty commit. This is confusing because the commit message no longer > matches the contents of the commit. Drop the commit instead. Commits > that start off empty are not dropped. This matches the behavior of > "git rebase --reapply-cherry-pick --empty=drop" and "git cherry-pick > --empty-drop". > > If a branch points to a commit that is dropped it will be updated to > point to the last commit that was not dropped. This can been seen > in the new test where "topic1" is updated to point to the rebased > "C" as "F" is dropped because it is already upstream. While this is > a breaking change "git replay" is marked as experimental to allow > improvements like this that change the behavior. > > Helped-by: Elijah Newren > Signed-off-by: Phillip Wood > --- > Changes since v1: > > - modified test to update refs as suggested by Elijah. I've kept > --ancestry-path --branches rather than switching to --contained as > I think it is useful to have test coverage for those options and it > means we can check that empty commits are dropped with out replying > on --contained working. Fair enough. > This patch is based on ps/history > > I think dropping commits that become empty is the sensible default, > if it turns out that some users are relying on the current behavior > we can add an option to retain the empty commits. > > Base-Commit: d37c42ea661434c347d2047f01b338341099fa60 > Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-drop-commits-that-become-empty%2Fv2 > View-Changes-At: https://github.com/phillipwood/git/compare/d37c42ea6...9a81644a0 > Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-drop-commits-that-become-empty/v2 > > Documentation/git-replay.adoc | 4 +++- > replay.c | 10 +++++++--- > t/t3650-replay-basics.sh | 21 +++++++++++++++++++++ > 3 files changed, 31 insertions(+), 4 deletions(-) > > diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc > index dcb26e8a8e8..96a3a557bf3 100644 > --- a/Documentation/git-replay.adoc > +++ b/Documentation/git-replay.adoc > @@ -59,7 +59,9 @@ The default mode can be configured via the `replay.refAction` configuration vari > be passed, but in `--advance ` mode, they should have > a single tip, so that it's clear where should point > to. See "Specifying Ranges" in linkgit:git-rev-parse[1] and the > - "Commit Limiting" options below. > + "Commit Limiting" options below. Any commits in the range whose > + changes are already present in the branch the commits are being > + replayed onto will be dropped. > > include::rev-list-options.adoc[] > > diff --git a/replay.c b/replay.c > index 13983dbc566..2864c213993 100644 > --- a/replay.c > +++ b/replay.c > @@ -88,12 +88,12 @@ struct commit *replay_pick_regular_commit(struct repository *repo, > struct merge_result *result) > { > struct commit *base, *replayed_base; > - struct tree *pickme_tree, *base_tree; > + struct tree *pickme_tree, *base_tree, *replayed_base_tree; > > base = pickme->parents->item; > replayed_base = mapped_commit(replayed_commits, base, onto); > > - result->tree = repo_get_commit_tree(repo, replayed_base); > + replayed_base_tree = repo_get_commit_tree(repo, replayed_base); > pickme_tree = repo_get_commit_tree(repo, pickme); > base_tree = repo_get_commit_tree(repo, base); > > @@ -103,13 +103,17 @@ struct commit *replay_pick_regular_commit(struct repository *repo, > > merge_incore_nonrecursive(merge_opt, > base_tree, > - result->tree, > + replayed_base_tree, > pickme_tree, > result); > > free((char*)merge_opt->ancestor); > merge_opt->ancestor = NULL; > if (!result->clean) > return NULL; > + /* Drop commits that become empty */ > + if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) && > + !oideq(&pickme_tree->object.oid, &base_tree->object.oid)) > + return replayed_base; > return replay_create_commit(repo, result->tree, pickme, replayed_base); > } > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh > index cf3aacf3551..9d4b0dd1a77 100755 > --- a/t/t3650-replay-basics.sh > +++ b/t/t3650-replay-basics.sh > @@ -25,6 +25,8 @@ test_expect_success 'setup' ' > git switch -c topic3 && > test_commit G && > test_commit H && > + git switch -c empty && > + git commit --allow-empty --only -m empty && > git switch -c topic4 main && > test_commit I && > test_commit J && > @@ -106,6 +108,25 @@ test_expect_success 'using replay on bare repo to perform basic cherry-pick' ' > test_cmp expect result-bare > ' > > +test_expect_success 'commits that become empty are dropped' ' > + # Save original branches > + git for-each-ref --format="update %(refname) %(objectname)" \ > + refs/heads/ >original-branches && > + test_when_finished "git update-ref --stdin + rm original-branches" && > + # Cherry-pick tip of topic1 ("F"), from the middle of A..empty, to main > + git replay --advance main topic1^! && > + > + # Replay all of A..empty onto main (which includes topic1 & thus F > + # in the middle) > + git replay --onto main --branches --ancestry-path=empty ^A \ > + >result && > + git log --format="%s%d" L..empty >actual && > + test_write_lines >expect \ > + "empty (empty)" "H (topic3)" G "C (topic1)" "F (main)" "M (tag: M)" && > + test_cmp expect actual > +' > + > test_expect_success 'replay on bare repo fails with both --advance and --onto' ' > test_must_fail git -C bare replay --advance main --onto main topic1..topic2 >result-bare > ' I like the minor edits Junio suggested, but otherwise this version looks good to me. Thanks!