[PATCH] contrib/subtree: fix split with squashed subtrees

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 if REV is a subtree commit, but the combination of

    git log -1 --grep=...

actually searches all *parent* commits until a `--grep` match is
discovered. Limit these checks to the current REV only.

Tests now cover nested subtrees.

Signed-off-by: Colin Stagner <ask+git@xxxxxxxxxxx>
---

Notes:
    The unit test changes in t7900-subtree.sh demonstrate the bug.
    
    See also:
    
    * <pull.1587.v5.git.1701206267300.gitgitgadget@xxxxxxxxx>
    * <c9e8f54f-2594-4092-ae41-f1da73e97f6e@xxxxxxxxxxx>

 contrib/subtree/git-subtree.sh     |  6 +--
 contrib/subtree/t/t7900-subtree.sh | 70 ++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 3fddba797c..139049351d 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -788,17 +788,17 @@ ensure_valid_ref_format () {
 # Usage: check if a commit from another subtree 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)"
+	if test -n "$(git log -1 --grep="git-subtree-dir:" "$rev^!")"
 	then
-		if test -z "$(git log -1 --grep="git-subtree-mainline:" $rev)" &&
-			test -z "$(git log -1 --grep="git-subtree-dir: $arg_prefix$" $rev)"
+		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
 	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 3edbb33af4..1ddc213621 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -68,6 +68,34 @@ 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 checkout --orphan "$orphan" &&
+		git rm -rf . &&
+		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 +454,48 @@ 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 &&
+			git branch -m main &&
+			test_create_subtree_add \
+				. mksubtree subA/subB file2 ${is_squashed:+--squash} &&
+			test -e subA/file1.t &&
+			test -e subA/subB/file2.t &&
+			git subtree split --prefix=subA --branch=bsplit &&
+			git checkout bsplit &&
+			test -e file1.t &&
+			test -e 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 -e subA/subB/file3.t &&
+			git subtree split --prefix=subA --branch=bsplit &&
+			git checkout bsplit &&
+			test -e file1.t &&
+			test -e subB/file2.t &&
+			test -e 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 &&

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
-- 
2.43.0





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux