From: Phillip Wood <phillip.wood123@gmail•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Elijah Newren <newren@gmail•com>
Subject: Re: [PATCH v2] replay: drop commits that become empty
Date: Tue, 16 Dec 2025 14:19:11 +0000 [thread overview]
Message-ID: <80477d23-eed3-4a99-be97-f692bc36095e@gmail.com> (raw)
In-Reply-To: <xmqqpl8f719x.fsf@gitster.g>
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.
next prev parent reply other threads:[~2025-12-16 14:19 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
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
2025-12-04 14:06 ` Phillip Wood
2025-12-15 10:07 ` [PATCH v2] " Phillip Wood
2025-12-15 23:50 ` Junio C Hamano
2025-12-16 14:19 ` Phillip Wood [this message]
2025-12-17 14:45 ` Phillip Wood
2025-12-17 23:49 ` Junio C Hamano
2025-12-16 0:21 ` Elijah Newren
2025-12-16 14:19 ` [PATCH v3] " Phillip Wood
2025-12-16 16:36 ` Elijah Newren
2025-12-17 14:47 ` Phillip Wood
2025-12-18 16:50 ` [PATCH v4] " Phillip Wood
2025-12-19 4:44 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=80477d23-eed3-4a99-be97-f692bc36095e@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox