Re: [PATCH] replay: drop commits that become empty

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

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.