contrib/subtree: fix split with squashed subtrees

98ba49ccc2 (subtree: fix split processing with multiple subtrees
present, 2023-12-01) increases the performance of

    git subtree split --prefix=subA

by ignoring subtree merges which are outside of `subA/`. It also
introduces a regression. Subtree merges that should be retained
are incorrectly ignored if they:

1. are nested under `subA/`; and
2. are merged with `--squash`.

For example, a subtree merged like:

    git subtree merge --squash --prefix=subA/subB "$rev"
    #                 ^^^^^^^^          ^^^^

is erroneously ignored during a split of `subA`. This causes
missing tree files and different commit hashes starting in
git v2.44.0-rc0.

The method:

    should_ignore_subtree_split_commit REV

should test only a single commit REV, but the combination of

    git log -1 --grep=...

actually searches all *parent* commits until a `--grep` match is
discovered.

Rewrite this method to test only one REV at a time. Extract commit
information with a single `git` call as opposed to three. The
`test` conditions for rejecting a commit remain unchanged.

Unit tests now cover nested subtrees.

Signed-off-by: Colin Stagner <ask+git@howdoi.land>
Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 3fddba797cb92c95df0243e397b9dcc78cef7445..17106d1a721519978ef78b63e9b7763e78813674 100755 (executable)
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -785,20 +785,40 @@ ensure_valid_ref_format () {
                die "fatal: '$1' does not look like a ref"
 }
 
-# Usage: check if a commit from another subtree should be
+# Usage: should_ignore_subtree_split_commit REV
+#
+# Check if REV is a commit from another subtree and should be
 # ignored from processing for splits
 should_ignore_subtree_split_commit () {
        assert test $# = 1
-       local rev="$1"
-       if test -n "$(git log -1 --grep="git-subtree-dir:" $rev)"
+
+       git show \
+               --no-patch \
+               --no-show-signature \
+               --format='%(trailers:key=git-subtree-dir,key=git-subtree-mainline)' \
+               "$1" |
+       (
+       have_mainline=
+       subtree_dir=
+
+       while read -r trailer val
+       do
+               case "$trailer" in
+               git-subtree-dir:)
+                       subtree_dir="${val%/}" ;;
+               git-subtree-mainline:)
+                       have_mainline=y ;;
+               esac
+       done
+
+       if test -n "${subtree_dir}" &&
+               test -z "${have_mainline}" &&
+               test "${subtree_dir}" != "$arg_prefix"
        then
-               if test -z "$(git log -1 --grep="git-subtree-mainline:" $rev)" &&
-                       test -z "$(git log -1 --grep="git-subtree-dir: $arg_prefix$" $rev)"
-               then
-                       return 0
-               fi
+               return 0
        fi
        return 1
+       )
 }
 
 # Usage: process_split_commit REV PARENTS

diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 3edbb33af4697114972376371ecdb3a38b967699..316dc5269e2b6f47dd01f28e854dd9a68dbde493 100755 (executable)
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -9,6 +9,9 @@ This test verifies the basic operation of the add, merge, split, pull,
 and push subcommands of git subtree.
 '
 
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
 TEST_DIRECTORY=$(pwd)/../../../t
 . "$TEST_DIRECTORY"/test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
@@ -68,6 +71,33 @@ test_create_pre2_32_repo () {
        git -C "$1-clone" replace HEAD^2 $new_commit
 }
 
+# test_create_subtree_add REPO ORPHAN PREFIX FILENAME ...
+#
+# Create a simple subtree on a new branch named ORPHAN in REPO.
+# The subtree is then merged into the current branch of REPO,
+# under PREFIX. The generated subtree has has one commit
+# with subject and tag FILENAME with a single file "FILENAME.t"
+#
+# When this method returns:
+# - the current branch of REPO will have file PREFIX/FILENAME.t
+# - REPO will have a branch named ORPHAN with subtree history
+#
+# additional arguments are forwarded to "subtree add"
+test_create_subtree_add () {
+       (
+               cd "$1" &&
+               orphan="$2" &&
+               prefix="$3" &&
+               filename="$4" &&
+               shift 4 &&
+               last="$(git branch --show-current)" &&
+               git switch --orphan "$orphan" &&
+               test_commit "$filename" &&
+               git checkout "$last" &&
+               git subtree add --prefix="$prefix" "$@" "$orphan"
+       )
+}
+
 test_expect_success 'shows short help text for -h' '
        test_expect_code 129 git subtree -h >out 2>err &&
        test_must_be_empty err &&
@@ -426,6 +456,47 @@ test_expect_success 'split with multiple subtrees' '
                --squash --rejoin -d -m "Sub B Split 1" 2>&1 | grep -w "\[1\]")" = ""
 '
 
+# When subtree split-ing a directory that has other subtree
+# *merges* underneath it, the split must include those subtrees.
+# This test creates a nested subtree, `subA/subB`, and tests
+# that the tree is correct after a subtree split of `subA/`.
+# The test covers:
+# - An initial `subtree add`; and
+# - A follow-up `subtree merge`
+# both with and without `--squashed`.
+for is_squashed in '' 'y'
+do
+       test_expect_success "split keeps nested ${is_squashed:+--squash }subtrees that are part of the split" '
+               subtree_test_create_repo "$test_count" &&
+               (
+                       cd "$test_count" &&
+                       mkdir subA &&
+                       test_commit subA/file1 &&
+                       test_create_subtree_add \
+                               . mksubtree subA/subB file2 ${is_squashed:+--squash} &&
+                       test_path_is_file subA/file1.t &&
+                       test_path_is_file subA/subB/file2.t &&
+                       git subtree split --prefix=subA --branch=bsplit &&
+                       git checkout bsplit &&
+                       test_path_is_file file1.t &&
+                       test_path_is_file subB/file2.t &&
+                       git checkout mksubtree &&
+                       git branch -D bsplit &&
+                       test_commit file3 &&
+                       git checkout main &&
+                       git subtree merge \
+                               ${is_squashed:+--squash} \
+                               --prefix=subA/subB mksubtree &&
+                       test_path_is_file subA/subB/file3.t &&
+                       git subtree split --prefix=subA --branch=bsplit &&
+                       git checkout bsplit &&
+                       test_path_is_file file1.t &&
+                       test_path_is_file subB/file2.t &&
+                       test_path_is_file subB/file3.t
+               )
+       '
+done
+
 test_expect_success 'split sub dir/ with --rejoin from scratch' '
        subtree_test_create_repo "$test_count" &&
        test_create_commit "$test_count" main1 &&