Re: What's cooking in git.git (Dec 2015, #01; Tue, 1)

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote:
>> On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote:
> [snip]
>> > "--keep-empty" has always been about keeping an originally empty
>> > commit, not a commit that becomes empty because of rebasing
>> > (i.e. what has already been applied to the updated base).  The
>> > documentation, if it leads to any other interpretation, needs to be
>> > fixed.
>> > 
>> > Besides, if "--keep-empty" were to mean "keep redundant ones that
>> > are already in the updated base", the patch must do a lot more,
>> > e.g. stop filtering with git-cherry patch equivalence.
>> > 
>> > I'm inclined to eject this topic.
>> 
>> That was my thinking too (and I notice it didn't get any review from
>> anybody else).
> [snip]
>
> Well, I kind of agree. The cherry-pick command has both
> --allow-empty and --keep-redundant flags, where the second one is
> the kind of behavior I want to achieve in my case. As an
> alternative to the proposed change to `--keep-empty` I could
> instead introduce a new flag `--keep-redundant-commits` to `git
> rebase` which would then pass the flag through to the
> cherry-pick.
>
> Any opinions on this?

Honestly, I am not interested if that is only about passing that
option and doing nothing else.

Imagine that you have two changes from the branch being rebased
already in the updated base, one was accepted verbatim, while the
other one was accepted with a slight tweak.  Perhaps there were some
conflicts in the context, or something.

When you run your rebase, the former will not even be considered
because command will notice, via patch equivalence, that you do not
need it, even before it attempts to replay it.  But not the latter.
The command will attempt to replay it, and only in this step it may
notice, by seeing that the result becomes a no-op change, that the
change has already been included in the updated base.

I cannot quite see how adding "--keep-redundant-commits" to the
command would help anybody by allowing the latter in the history but
not the former.  That is primarily because you can imagine a future
in which the patch equivalence determination becomes smarter.  With
or without "--keep-redundant-commits", both of these two changes
would not appear in the resulting history when that happens.

The "--keep-redundant" option to "cherry-pick" makes sense, as the
user will be the one who is deciding which commit to replay on top
of a new base.  If the user fed a commit that is already in the new
base, and the command is run with the option, that is a deliberate
request to leave a no-op cruft.

We cannot say the same thing for "rebase", as it tries to avoid
redundant cruft, and it cannot do as good a job as humans in doing
so.  The "--keep-redundant-commits" option will become a way to make
a distinction between what got dropped by the command automatically
and what didn't get dropped because the command did not look deeply
enough.  That distinction is not at all useful, as that can change
as the tool improves.

A "--keep-redundant-commits" to "rebase" that also disables the
patch equivalence filtering (not just keeping empty crufts in the
resulting history) might make sense, but I do not see myself (or
anybody sane) using it.  "In what situation would such a feature be
useful?" is a question I cannot answer offhand.