From: Phillip Wood <phillip.wood123@gmail•com>
To: Elijah Newren <newren@gmail•com>,
Phillip Wood <phillip.wood@dunelm•org.uk>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] replay: drop commits that become empty
Date: Thu, 4 Dec 2025 14:06:51 +0000 [thread overview]
Message-ID: <10f9afd8-6ac2-4e17-979f-2222bd0a2fda@gmail.com> (raw)
In-Reply-To: <CABPp-BEZFPmLnEtnD0WaNbkZ5uE7q5T6uKJQRUvtq+L=C1o9wg@mail.gmail.com>
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
> '
>
next prev parent reply other threads:[~2025-12-04 14:07 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 [this message]
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-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=10f9afd8-6ac2-4e17-979f-2222bd0a2fda@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--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