* Re: [PATCH] replay: drop commits that become empty
2025-11-27 16:15 [PATCH] replay: drop commits that become empty Phillip Wood
@ 2025-11-28 7:29 ` Junio C Hamano
2025-12-04 14:08 ` Phillip Wood
2025-11-28 8:06 ` Elijah Newren
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2025-11-28 7:29 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Elijah Newren
Phillip Wood <phillip.wood123@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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.
If a commit that originally did two or more things is replayed on a
destination that already has only part of it, then the extent of the
change the replayed commit makes will shrink, and the commit message
no longer matches it, either. It is a lot harder to notice the
situation to prompt the user to rewrite the resulting commit
message, but in the degenerated case where the entire changes go
away, the rewrite of the resulting commit message is very simple,
which is to remove the commit altogether.
Makes sense.
> Commits
> that start off empty are not dropped.
Makes perfect sense, too.
> 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.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> Elijah - I'm not really clear why we were setting result->tree before
> calling merge_incore_nonrecursive(), was it just for convenience to
> avoid declaring a local variable or have I missed something?
>
> This patch is based on ps/history
As I take this more as a rfc/rfh than finalized version, it is OK to
depend on the topic that is known to be rerolled soonish.
> 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.
I think it would be a good default to drop what becomes empty.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] replay: drop commits that become empty
2025-11-28 7:29 ` Junio C Hamano
@ 2025-12-04 14:08 ` Phillip Wood
0 siblings, 0 replies; 16+ messages in thread
From: Phillip Wood @ 2025-12-04 14:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Elijah Newren
On 28/11/2025 07:29, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>> This patch is based on ps/history
>
> As I take this more as a rfc/rfh than finalized version, it is OK to
> depend on the topic that is known to be rerolled soonish.
Would you rather I rebased onto master when I re-roll?
Thanks
Phillip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] replay: drop commits that become empty
2025-11-27 16:15 [PATCH] replay: drop commits that become empty Phillip Wood
2025-11-28 7:29 ` Junio C Hamano
@ 2025-11-28 8:06 ` Elijah Newren
2025-12-04 14:06 ` Phillip Wood
2025-12-15 10:07 ` [PATCH v2] " Phillip Wood
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Elijah Newren @ 2025-11-28 8:06 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Phillip Wood
On Thu, Nov 27, 2025 at 8:16 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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.
Yeah, I've got a commit in my local branch that does the same thing.
It feels like there should be a paragraph break in here somewhere, but
maybe that's just me? Pretty minor either way.
> 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.
Yep.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> Elijah - I'm not really clear why we were setting result->tree before
> calling merge_incore_nonrecursive(), was it just for convenience to
> avoid declaring a local variable or have I missed something?
I don't know the reason. That traces back to a commit with
Christian's Co-authored-by, so it may have been either him or me that
introduced it. My original work on replay was on a branch that I long
ago rebased on top of the version Christian submitted, and the old
history is no longer reachable from my local reflog, so I don't have a
way to narrow down who of us did it. If it was him, he may be able to
answer. If it was me, I've long since forgotten. I think using a
temporary, as you've done, is better.
> 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.
I fully agree.
> Base-Commit: 4ac8283def34401e50908903b89fa22498bb23a2
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-drop-commits-that-become-empty%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/4ac8283de...8a2a12153
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-drop-commits-that-become-empty/v1
>
> Documentation/git-replay.adoc | 4 +++-
> replay.c | 10 +++++++---
> t/t3650-replay-basics.sh | 25 +++++++++++++++++++++++++
> 3 files changed, 35 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 <branch>` mode, they should have
> a single tip, so that it's clear where <branch> 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 58fdc20140b..7cd7206eee5 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);
> }
Makes sense; your version is similar but slightly cleaner than my
local implementation of the same thing. Plus you have a test, which I
hadn't added yet.
> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> index cf3aacf3551..d73ab16908a 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,29 @@ 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' '
This test is a bit more complicated than normal, and might benefit
from a comment or two.
> + git replay --ref-action=print --advance main topic1^! >result &&
> + ONTO=$(cut -f 3 -d " " result) &&
You're basically cherry-picking one commit from the middle of A..empty
(namely the tip of topic1) onto main, without updating any refs...
> + git replay --ref-action=print --onto $ONTO \
> + --branches --ancestry-path=empty ^A >result &&
...and here you replay the range A..empty onto what would have been
the new main, but since one of those commits were already
cherry-picked, you expect that one to be dropped.
Since "empty" has no descendant commits or branches, the flags
--branches --ancestry-path=empty ^A
feel like a more complicated way of saying
--contained A..empty
> + # Write the new value of refs/heads/empty to "new-empty" and
> + # generate a sed script that annotates the output of
> + # `git log --format="%H %s"` with the updated branches
> + SCRIPT="$(sed -e "
> + /empty/{
> + h
> + s|^.*empty \([^ ]*\) .*|\1|wnew-empty
> + g
> + }
> + s|^.*/\([^/ ]*\) \([^ ]*\).*|/^\2/s/\\\$/ (\1)/|
> + \$s|\$|;s/^[^ ]* //|" result)" &&
> + git log --format="%H %s" --stdin <new-empty >actual.raw &&
> + sed -e "$SCRIPT" actual.raw >actual &&
> + test_write_lines >expect \
> + "empty (empty)" "H (topic3)" G "C (topic1)" F M L B A &&
> + test_cmp expect actual
After digging around for a while (my sed-fu is far weaker than yours),
this feels like you are going out of your way to avoid changing any
branches, but then trying to figure out what the branch changes would
have been. Would it be simpler to remove the --ref-action=print
flags, check directly what changes were made, and use a
test_when_finished to reset the branches back to their starting point
at the end? That'd change this test to something like:
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 <original-branches &&
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 --contained A..empty &&
# Check that "F" was applied first, then "C", and that "F" wasn't
applied twice. Also, that topic1 now points to "C".
git log --format="%s%d" L..empty >actual &&
test_write_lines >expect \
"empty (empty)" "H (topic3)" G "C (topic1)" F "M (main)" &&
test_cmp expect actual
'
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] replay: drop commits that become empty
2025-11-28 8:06 ` Elijah Newren
@ 2025-12-04 14:06 ` Phillip Wood
0 siblings, 0 replies; 16+ messages in thread
From: Phillip Wood @ 2025-12-04 14:06 UTC (permalink / raw)
To: Elijah Newren, Phillip Wood; +Cc: git
On 28/11/2025 08:06, Elijah Newren wrote:
> On Thu, Nov 27, 2025 at 8:16 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> 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.
>
> Yeah, I've got a commit in my local branch that does the same thing.
>
> It feels like there should be a paragraph break in here somewhere, but
> maybe that's just me? Pretty minor either way.
Yes it could do with a paragraph break, I'll add one
>> 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.
>
> Yep.
>
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>> Elijah - I'm not really clear why we were setting result->tree before
>> calling merge_incore_nonrecursive(), was it just for convenience to
>> avoid declaring a local variable or have I missed something?
>
> I don't know the reason. That traces back to a commit with
> Christian's Co-authored-by, so it may have been either him or me that
> introduced it. My original work on replay was on a branch that I long
> ago rebased on top of the version Christian submitted, and the old
> history is no longer reachable from my local reflog, so I don't have a
> way to narrow down who of us did it. If it was him, he may be able to
> answer. If it was me, I've long since forgotten. I think using a
> temporary, as you've done, is better.
Thanks, I was worried I might have missed some subtlety and
inadvertently broken a corner case.
>> + # Write the new value of refs/heads/empty to "new-empty" and
>> + # generate a sed script that annotates the output of
>> + # `git log --format="%H %s"` with the updated branches
>> + SCRIPT="$(sed -e "
>> + /empty/{
>> + h
>> + s|^.*empty \([^ ]*\) .*|\1|wnew-empty
>> + g
>> + }
>> + s|^.*/\([^/ ]*\) \([^ ]*\).*|/^\2/s/\\\$/ (\1)/|
>> + \$s|\$|;s/^[^ ]* //|" result)" &&
>> + git log --format="%H %s" --stdin <new-empty >actual.raw &&
>> + sed -e "$SCRIPT" actual.raw >actual &&
>> + test_write_lines >expect \
>> + "empty (empty)" "H (topic3)" G "C (topic1)" F M L B A &&
>> + test_cmp expect actual
>
> After digging around for a while (my sed-fu is far weaker than yours),
> this feels like you are going out of your way to avoid changing any
> branches, but then trying to figure out what the branch changes would
> have been. Would it be simpler to remove the --ref-action=print
> flags, check directly what changes were made, and use a
> test_when_finished to reset the branches back to their starting point
> at the end? That'd change this test to something like:
I used --ref-action=print to match the existing tests, but it would be
much simpler to drop it. Your suggestion below looks good.
Thanks
Phillip
> 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 <original-branches &&
> 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 --contained A..empty &&
>
> # Check that "F" was applied first, then "C", and that "F" wasn't
> applied twice. Also, that topic1 now points to "C".
> git log --format="%s%d" L..empty >actual &&
> test_write_lines >expect \
> "empty (empty)" "H (topic3)" G "C (topic1)" F "M (main)" &&
> test_cmp expect actual
> '
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] replay: drop commits that become empty
2025-11-27 16:15 [PATCH] replay: drop commits that become empty Phillip Wood
2025-11-28 7:29 ` Junio C Hamano
2025-11-28 8:06 ` Elijah Newren
@ 2025-12-15 10:07 ` Phillip Wood
2025-12-15 23:50 ` Junio C Hamano
2025-12-16 0:21 ` Elijah Newren
2025-12-16 14:19 ` [PATCH v3] " Phillip Wood
2025-12-18 16:50 ` [PATCH v4] " Phillip Wood
4 siblings, 2 replies; 16+ messages in thread
From: Phillip Wood @ 2025-12-15 10:07 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
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 <newren@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
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.
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 <branch>` mode, they should have
a single tip, so that it's clear where <branch> 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 <original-branches &&
+ 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
'
Range-diff against v1:
1: 8a2a1215306 ! 1: 9a81644a0ec replay: drop commits that become empty
@@ Commit message
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.
-
+ --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 <newren@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
## Documentation/git-replay.adoc ##
@@ t/t3650-replay-basics.sh: test_expect_success 'using replay on bare repo to perf
'
+test_expect_success 'commits that become empty are dropped' '
-+ git replay --ref-action=print --advance main topic1^! >result &&
-+ ONTO=$(cut -f 3 -d " " result) &&
-+ git replay --ref-action=print --onto $ONTO \
-+ --branches --ancestry-path=empty ^A >result &&
-+ # Write the new value of refs/heads/empty to "new-empty" and
-+ # generate a sed script that annotates the output of
-+ # `git log --format="%H %s"` with the updated branches
-+ SCRIPT="$(sed -e "
-+ /empty/{
-+ h
-+ s|^.*empty \([^ ]*\) .*|\1|wnew-empty
-+ g
-+ }
-+ s|^.*/\([^/ ]*\) \([^ ]*\).*|/^\2/s/\\\$/ (\1)/|
-+ \$s|\$|;s/^[^ ]* //|" result)" &&
-+ git log --format="%H %s" --stdin <new-empty >actual.raw &&
-+ sed -e "$SCRIPT" actual.raw >actual &&
++ # Save original branches
++ git for-each-ref --format="update %(refname) %(objectname)" \
++ refs/heads/ >original-branches &&
++ test_when_finished "git update-ref --stdin <original-branches &&
++ 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 M L B A &&
++ "empty (empty)" "H (topic3)" G "C (topic1)" "F (main)" "M (tag: M)" &&
+ test_cmp expect actual
+'
+
--
2.52.0.362.g884e03848a9
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2] replay: drop commits that become empty
2025-12-15 10:07 ` [PATCH v2] " Phillip Wood
@ 2025-12-15 23:50 ` Junio C Hamano
2025-12-16 14:19 ` Phillip Wood
2025-12-17 14:45 ` Phillip Wood
2025-12-16 0:21 ` Elijah Newren
1 sibling, 2 replies; 16+ messages in thread
From: Junio C Hamano @ 2025-12-15 23:50 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Elijah Newren
Phillip Wood <phillip.wood123@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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".
OK. Maybe it is just me but "onto then" -> "onto," would flow the
sentence better?
> 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
If one thinks about it, it is the only natural behaviour to use the
last surviving commit to point the branch at. Thanks for spelling
it out so clearly.
BTW, "can been seen" -> "can be seen" (will amend locally).
> 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.
Again maybe it is just me, but I'd prefer to see a comma after "a
breaking change" to flow the sentence better.
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> ...
> 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 <branch>` mode, they should have
> a single tip, so that it's clear where <branch> 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.
OK.
> 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);
> }
OK, that is straight-forward. Instead of overriding the
result->tree upfront, we try the same using a temporary
replayed_base_tree, and that allows us to see if the resulting tree
computed by merge_incore matches. Only when it made a non-empty
change, we proceed to create a new commit.
> 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 &&
The use of "--only" here is a bit curious. As there is no change
between the index and the commit our "empty" branch points at,
wouldn't it be unnecessary? The option, together with --allow-empty,
would only matter if you did
git switch -c empty &&
modify blah &&
git add blah &&
git commit --allow-empty --only -m empty
because without --only, the changes to blah will be taken.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] replay: drop commits that become empty
2025-12-15 23:50 ` Junio C Hamano
@ 2025-12-16 14:19 ` Phillip Wood
2025-12-17 14:45 ` Phillip Wood
1 sibling, 0 replies; 16+ messages in thread
From: Phillip Wood @ 2025-12-16 14:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Elijah Newren
On 15/12/2025 23:50, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> 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".
>
> OK. Maybe it is just me but "onto then" -> "onto," would flow the
> sentence better?
I agree it reads better with a comma here and in the second paragraph,
I'll re-roll
Thanks
Phillip
>> 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
>
> If one thinks about it, it is the only natural behaviour to use the
> last surviving commit to point the branch at. Thanks for spelling
> it out so clearly.
>
> BTW, "can been seen" -> "can be seen" (will amend locally).
>
>> 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.
>
> Again maybe it is just me, but I'd prefer to see a comma after "a
> breaking change" to flow the sentence better.
>
>> Helped-by: Elijah Newren <newren@gmail.com>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>> ...
>> 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 <branch>` mode, they should have
>> a single tip, so that it's clear where <branch> 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.
>
> OK.
>
>> 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);
>> }
>
> OK, that is straight-forward. Instead of overriding the
> result->tree upfront, we try the same using a temporary
> replayed_base_tree, and that allows us to see if the resulting tree
> computed by merge_incore matches. Only when it made a non-empty
> change, we proceed to create a new commit.
>
>> 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 &&
>
> The use of "--only" here is a bit curious. As there is no change
> between the index and the commit our "empty" branch points at,
> wouldn't it be unnecessary? The option, together with --allow-empty,
> would only matter if you did
>
> git switch -c empty &&
> modify blah &&
> git add blah &&
> git commit --allow-empty --only -m empty
>
> because without --only, the changes to blah will be taken.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2] replay: drop commits that become empty
2025-12-15 23:50 ` Junio C Hamano
2025-12-16 14:19 ` Phillip Wood
@ 2025-12-17 14:45 ` Phillip Wood
2025-12-17 23:49 ` Junio C Hamano
1 sibling, 1 reply; 16+ messages in thread
From: Phillip Wood @ 2025-12-17 14:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Elijah Newren
On 15/12/2025 23:50, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>> 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 &&
>
> The use of "--only" here is a bit curious. As there is no change
> between the index and the commit our "empty" branch points at,
> wouldn't it be unnecessary? The option, together with --allow-empty,
> would only matter if you did
>
> git switch -c empty &&
> modify blah &&
> git add blah &&
> git commit --allow-empty --only -m empty
>
> because without --only, the changes to blah will be taken.
I've got into the habit of always adding "--only" when I want to create
an empty commit in case there are staged changes. I don't really like
"--allow-empty" as I've never wanted to create commit that might or
might not be empty - either I want to create an empty commit in which
case I don't want to commit any staged changes, or I want the commit to
fail if there are no staged changes). I can remove it if you want.
Thanks
Phillip
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] replay: drop commits that become empty
2025-12-17 14:45 ` Phillip Wood
@ 2025-12-17 23:49 ` Junio C Hamano
0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2025-12-17 23:49 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Elijah Newren
Phillip Wood <phillip.wood123@gmail.com> writes:
>> git commit --allow-empty --only -m empty
>>
>> because without --only, the changes to blah will be taken.
>
> I've got into the habit of always adding "--only" when I want to create
> an empty commit in case there are staged changes. I don't really like
> "--allow-empty" as I've never wanted to create commit that might or
> might not be empty - either I want to create an empty commit in which
> case I don't want to commit any staged changes, or I want the commit to
> fail if there are no staged changes). I can remove it if you want.
Being explicit when you are unsure is good, but in this script I
think we should be very sure that the index matches HEAD, so I would
consider that the only effect of the use of the "--only" here is to
puzzle readers.
A comment "# force an empty commit by including no paths" before the
command would work to help unpuzzle readers, though ;-)
Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] replay: drop commits that become empty
2025-12-15 10:07 ` [PATCH v2] " Phillip Wood
2025-12-15 23:50 ` Junio C Hamano
@ 2025-12-16 0:21 ` Elijah Newren
1 sibling, 0 replies; 16+ messages in thread
From: Elijah Newren @ 2025-12-16 0:21 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Phillip Wood
On Mon, Dec 15, 2025 at 2:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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 <newren@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> 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 <branch>` mode, they should have
> a single tip, so that it's clear where <branch> 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 <original-branches &&
> + 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!
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] replay: drop commits that become empty
2025-11-27 16:15 [PATCH] replay: drop commits that become empty Phillip Wood
` (2 preceding siblings ...)
2025-12-15 10:07 ` [PATCH v2] " Phillip Wood
@ 2025-12-16 14:19 ` Phillip Wood
2025-12-16 16:36 ` Elijah Newren
2025-12-18 16:50 ` [PATCH v4] " Phillip Wood
4 siblings, 1 reply; 16+ messages in thread
From: Phillip Wood @ 2025-12-16 14:19 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
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 be 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 <newren@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Changes since v2:
- added a couple of commas to the commit message as suggested by Junio
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.
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%2Fv3
View-Changes-At: https://github.com/phillipwood/git/compare/d37c42ea6...73ba74b8a
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-drop-commits-that-become-empty/v3
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 <branch>` mode, they should have
a single tip, so that it's clear where <branch> 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 <original-branches &&
+ 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
'
Range-diff against v2:
1: 9a81644a0ec ! 1: 73ba74b8a2e replay: drop commits that become empty
@@ Commit message
replay: drop commits that become empty
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
+ 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
+ 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 be 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
+ a breaking change, "git replay" is marked as experimental to allow
improvements like this that change the behavior.
Helped-by: Elijah Newren <newren@gmail.com>
--
2.52.0.362.g884e03848a9
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3] replay: drop commits that become empty
2025-12-16 14:19 ` [PATCH v3] " Phillip Wood
@ 2025-12-16 16:36 ` Elijah Newren
2025-12-17 14:47 ` Phillip Wood
0 siblings, 1 reply; 16+ messages in thread
From: Elijah Newren @ 2025-12-16 16:36 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Phillip Wood
On Tue, Dec 16, 2025 at 6:19 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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 be 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 <newren@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> Changes since v2:
>
> - added a couple of commas to the commit message as suggested by Junio
- also changes "can been seen" to "can be seen"
I'm also curious if you are keeping the "--only" in the testcase
intentionally, or overlooked that part of Junio's feedback.
Anyway, this round looks good to me.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] replay: drop commits that become empty
2025-11-27 16:15 [PATCH] replay: drop commits that become empty Phillip Wood
` (3 preceding siblings ...)
2025-12-16 14:19 ` [PATCH v3] " Phillip Wood
@ 2025-12-18 16:50 ` Phillip Wood
2025-12-19 4:44 ` Junio C Hamano
4 siblings, 1 reply; 16+ messages in thread
From: Phillip Wood @ 2025-12-18 16:50 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
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 be 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 <newren@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Changes since v3:
- dropped "--only" when creating an empty commit
Changes since v2:
- added a couple of commas to the commit message as suggested by Junio
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.
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%2Fv4
View-Changes-At: https://github.com/phillipwood/git/compare/d37c42ea6...375adc4e9
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-drop-commits-that-become-empty/v4
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 <branch>` mode, they should have
a single tip, so that it's clear where <branch> 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..b3fb8869600 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 -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 <original-branches &&
+ 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
'
Range-diff against v3:
1: 73ba74b8a2e ! 1: 375adc4e941 replay: drop commits that become empty
@@ t/t3650-replay-basics.sh: test_expect_success 'setup' '
test_commit G &&
test_commit H &&
+ git switch -c empty &&
-+ git commit --allow-empty --only -m empty &&
++ git commit --allow-empty -m empty &&
git switch -c topic4 main &&
test_commit I &&
test_commit J &&
--
2.52.0.362.g884e03848a9
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v4] replay: drop commits that become empty
2025-12-18 16:50 ` [PATCH v4] " Phillip Wood
@ 2025-12-19 4:44 ` Junio C Hamano
0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2025-12-19 4:44 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Elijah Newren, Patrick Steinhardt
Phillip Wood <phillip.wood123@gmail.com> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> ...
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> Changes since v3:
>
> - dropped "--only" when creating an empty commit
>
> Changes since v2:
>
> - added a couple of commas to the commit message as suggested by Junio
>
> 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.
>
> 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.
Thanks. Will replace.
But I am not sure what the next move for this topic would be, until
the base topic ps/history is sorted out. There was a discussion
between "it is experimental, the early adopters should be prepared
that the behaviour can and will change" and "the behaviour being
questioned is so fundamental in the workflow, it is impossible to
fix retrospecitively".
^ permalink raw reply [flat|nested] 16+ messages in thread