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

>
> 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".
>
> 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.
>
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> Changes since v1:
>
>  - modified test to update refs as suggested by Elijah. I've kept
>    --ancestry-path --branches rather than switching to --contained as
>    I think it is useful to have test coverage for those options and it
>    means we can check that empty commits are dropped with out replying
>    on --contained working.

> This patch is based on ps/history
>
> 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.
>
> Base-Commit: d37c42ea661434c347d2047f01b338341099fa60
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Freplay-drop-commits-that-become-empty%2Fv2
> View-Changes-At: https://github.com/phillipwood/git/compare/d37c42ea6...9a81644a0
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/replay-drop-commits-that-become-empty/v2
>
>  Documentation/git-replay.adoc |  4 +++-
>  replay.c                      | 10 +++++++---
>  t/t3650-replay-basics.sh      | 21 +++++++++++++++++++++
>  3 files changed, 31 insertions(+), 4 deletions(-)
>
> 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.
>
>  include::rev-list-options.adoc[]
>
> 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);
>  }
> 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 &&
>         git switch -c topic4 main &&
>         test_commit I &&
>         test_commit J &&
> @@ -106,6 +108,25 @@ test_expect_success 'using replay on bare repo to perform basic cherry-pick' '
>         test_cmp expect result-bare
>  '
>
> +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 --branches --ancestry-path=empty ^A \
> +               >result &&
> +       git log --format="%s%d" L..empty >actual &&
> +       test_write_lines >expect \
> +               "empty (empty)" "H (topic3)" G "C (topic1)" "F (main)" "M (tag: M)" &&
> +       test_cmp expect actual
> +'
> +
>  test_expect_success 'replay on bare repo fails with both --advance and --onto' '
>         test_must_fail git -C bare replay --advance main --onto main topic1..topic2 >result-bare
>  '