git/t/t3427-rebase-subtree.sh
Elijah Newren e98c4269c8 rebase (interactive-backend): fix handling of commits that become empty
As established in the previous commit and commit b00bf1c9a8
(git-rebase: make --allow-empty-message the default, 2018-06-27), the
behavior for rebase with different backends in various edge or corner
cases is often more happenstance than design.  This commit addresses
another such corner case: commits which "become empty".

A careful reader may note that there are two types of commits which would
become empty due to a rebase:

  * [clean cherry-pick] Commits which are clean cherry-picks of upstream
    commits, as determined by `git log --cherry-mark ...`.  Re-applying
    these commits would result in an empty set of changes and a
    duplicative commit message; i.e. these are commits that have
    "already been applied" upstream.

  * [become empty] Commits which are not empty to start, are not clean
    cherry-picks of upstream commits, but which still become empty after
    being rebased.  This happens e.g. when a commit has changes which
    are a strict subset of the changes in an upstream commit, or when
    the changes of a commit can be found spread across or among several
    upstream commits.

Clearly, in both cases the changes in the commit in question are found
upstream already, but the commit message may not be in the latter case.

When cherry-mark can determine a commit is already upstream, then
because of how cherry-mark works this means the upstream commit message
was about the *exact* same set of changes.  Thus, the commit messages
can be assumed to be fully interchangeable (and are in fact likely to be
completely identical).  As such, the clean cherry-pick case represents a
case when there is no information to be gained by keeping the extra
commit around.  All rebase types have always dropped these commits, and
no one to my knowledge has ever requested that we do otherwise.

For many of the become empty cases (and likely even most), we will also
be able to drop the commit without loss of information -- but this isn't
quite always the case.  Since these commits represent cases that were
not clean cherry-picks, there is no upstream commit message explaining
the same set of changes.  Projects with good commit message hygiene will
likely have the explanation from our commit message contained within or
spread among the relevant upstream commits, but not all projects run
that way.  As such, the commit message of the commit being rebased may
have reasoning that suggests additional changes that should be made to
adapt to the new base, or it may have information that someone wants to
add as a note to another commit, or perhaps someone even wants to create
an empty commit with the commit message as-is.

Junio commented on the "become-empty" types of commits as follows[1]:

    WRT a change that ends up being empty (as opposed to a change that
    is empty from the beginning), I'd think that the current behaviour
    is desireable one.  "am" based rebase is solely to transplant an
    existing history and want to stop much less than "interactive" one
    whose purpose is to polish a series before making it publishable,
    and asking for confirmation ("this has become empty--do you want to
    drop it?") is more appropriate from the workflow point of view.

[1] https://lore.kernel.org/git/xmqqfu1fswdh.fsf@gitster-ct.c.googlers.com/

I would simply add that his arguments for "am"-based rebases actually
apply to all non-explicitly-interactive rebases.  Also, since we are
stating that different cases should have different defaults, it may be
worth providing a flag to allow users to select which behavior they want
for these commits.

Introduce a new command line flag for selecting the desired behavior:
    --empty={drop,keep,ask}
with the definitions:
    drop: drop commits which become empty
    keep: keep commits which become empty
    ask:  provide the user a chance to interact and pick what to do with
          commits which become empty on a case-by-case basis

In line with Junio's suggestion, if the --empty flag is not specified,
pick defaults as follows:
    explicitly interactive: ask
    otherwise: drop

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-16 15:40:42 -08:00

111 lines
4.0 KiB
Bash
Executable File

#!/bin/sh
test_description='git rebase tests for -Xsubtree
This test runs git rebase and tests the subtree strategy.
'
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh
commit_message() {
git log --pretty=format:%s -1 "$1"
}
# There are a few bugs in the rebase with regards to the subtree strategy, and
# this test script tries to document them. First, the following commit history
# is generated (the onelines are shown, time flows from left to right):
#
# master1 - master2 - master3
# \
# README ---------------------- Add subproject master - master4 - files_subtree/master5
#
# Where the merge moves the files master[123].t into the subdirectory
# files_subtree/ and master4 as well as files_subtree/master5 add files to that
# directory directly.
#
# Then, in subsequent test cases, `git filter-branch` is used to distill just
# the commits that touch files_subtree/. To give it a final pre-rebase touch,
# an empty commit is added on top. The pre-rebase commit history looks like
# this:
#
# Add subproject master - master4 - files_subtree/master5 - Empty commit
#
# where the root commit adds three files: master1.t, master2.t and master3.t.
#
# This commit history is then rebased onto `master3` with the
# `-Xsubtree=files_subtree` option in three different ways:
#
# 1. using `--preserve-merges`
# 2. using `--preserve-merges` and --keep-empty
# 3. without specifying a rebase backend
test_expect_success 'setup' '
test_commit README &&
git init files &&
test_commit -C files master1 &&
test_commit -C files master2 &&
test_commit -C files master3 &&
: perform subtree merge into files_subtree/ &&
git fetch files refs/heads/master:refs/heads/files-master &&
git merge -s ours --no-commit --allow-unrelated-histories \
files-master &&
git read-tree --prefix=files_subtree -u files-master &&
git commit -m "Add subproject master" &&
: add two extra commits to rebase &&
test_commit -C files_subtree master4 &&
test_commit files_subtree/master5 &&
git checkout -b to-rebase &&
git fast-export --no-data HEAD -- files_subtree/ |
sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" |
git fast-import --force --quiet &&
git reset --hard &&
git commit -m "Empty commit" --allow-empty
'
# FAILURE: Does not preserve master4.
test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' '
reset_rebase &&
git checkout -b rebase-preserve-merges to-rebase &&
git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
verbose test "$(commit_message HEAD~)" = "master4" &&
verbose test "$(commit_message HEAD)" = "files_subtree/master5"
'
# FAILURE: Does not preserve master4.
test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
reset_rebase &&
git checkout -b rebase-keep-empty to-rebase &&
git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
verbose test "$(commit_message HEAD~2)" = "master4" &&
verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
verbose test "$(commit_message HEAD)" = "Empty commit"
'
test_expect_success 'Rebase -Xsubtree --empty=ask --onto commit' '
reset_rebase &&
git checkout -b rebase-onto to-rebase &&
test_must_fail git rebase -Xsubtree=files_subtree --empty=ask --onto files-master master &&
: first pick results in no changes &&
git rebase --skip &&
verbose test "$(commit_message HEAD~2)" = "master4" &&
verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
verbose test "$(commit_message HEAD)" = "Empty commit"
'
test_expect_success 'Rebase -Xsubtree --empty=ask --rebase-merges --onto commit' '
reset_rebase &&
git checkout -b rebase-merges-onto to-rebase &&
test_must_fail git rebase -Xsubtree=files_subtree --empty=ask --rebase-merges --onto files-master --root &&
: first pick results in no changes &&
git rebase --skip &&
verbose test "$(commit_message HEAD~2)" = "master4" &&
verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
verbose test "$(commit_message HEAD)" = "Empty commit"
'
test_done