Subject: Re: [PATCH v2 8/8] cherry-pick: add `--empty` for more robust redundant commit handling

From: Junio C Hamano <[email-masked]>
Date: Tue, 27 Feb 2024 09:33:59 -0800

[email-masked] writes:

>>>> I do not quite see a good reason to do the opposite, dropping
>>>> commits that started out as empty but keeping the ones that have
>>>> become empty.  Such a behaviour has additional downside that after
>>>> such a cherry-pick, when you cherry-pick the resulting history onto
>>>> yet another base, your precious "were not empty but have become so
>>>> during the initial cherry-pick" commits will appear as commits that
>>>> were empty from the start.  So I do not see much reason to allow the
>>>> decoupling, even with the new "empty=keep" thing that does not imply
>>>> "allow-empty".
>>>
>>> Junio -- can you clarify this part?
>>>
>>>> So I do not see much reason to allow the decoupling, even with the new
>>>> "empty=keep" thing that does not imply "allow-empty"
>>>
>>> I'm not 100% sure if you are saying that you want `--empty=keep` to
>>> *also* imply `--allow-empty`, or that you simply want
>>> `--keep-redundant-commits` to continue implying `--allow-empty`
>>> *despite* the new `--empty=keep` no implying the same.
>
> FWIW I read it as the latter, but I can't claim to know what was in
> Junio's mind when he wrote it.

Given that "drop what was empty originally while keeping what became
empty" would "lose" what it wanted to keep (i.e. the one that has
become empty") when used to cherry-pick the result of doing such a
cherry-pick, I do not think allowing such combination makes as much
sense as the opposite "keep what was empty originally while dropping
what became empty", which does not have such a property.

And it does not matter if that is expressed via the combination of
existing --allow-empty and --keep-redundant-commits options, or the
newly proposed --empty=keep option.  If we start allowing the "drop
what was originally empty and keep what has become empty"
combination if we make empty=keep not to imply --allow-empty, I do
not think it is a good idea.

That was what was on my mind when I wrote it.  It may be that I was
not following the discussion correctly, and making "empty=keep" not
to imply "allow-empty" does *not* allow a request to "drop what was
originally empty, keep what has become empty".  If that is the case,
then I have no objection to making "empty=keep" not to imply
"allow-empty".

Thanks.