Skip to content

Commit 1f70684

Browse files
Colin Stagnergitster
authored andcommitted
contrib/subtree: process out-of-prefix subtrees
`should_ignore_subtree_split_commit` detects subtrees which are outside of the current path --prefix and ignores them. This can speed up splits of repositories that have many subtrees. Since its inception [1], every iteration of this logic [2], [3] incorrectly excludes commits. This alters the split history. The split history and its commit hashes are API contract, so this is not permissible. While a commit from a different subtree may look like it doesn't contribute anything to a split, sometimes it does. Merge commits are a particular hot spot. For these, the pruning logic in `copy_or_skip` performs: 1. a check for "treesame" parents 2. two different common ancestry checks These checks operate on the **split history**, not the input history. The split history omits commits that do not affect the --prefix. This can significantly alter the ancestry of a merge. In order to determine if `copy_or_skip` will skip a merge, it is likely necessary to compute all the split history... which is what `should_ignore_subtree_split_commit` tries to avoid. To make this logic API-preserving, we could gate it behind a new CLI argument. The present implementation is actually a speed penalty in many cases, however, so this is not done here. Remove the `should_ignore_subtree_split_commit` logic. This fixes the regression reported in [4]. [1]: 98ba49c (subtree: fix split processing with multiple subtrees present, 2023-12-01) [2]: 83f9dad (contrib/subtree: fix split with squashed subtrees, 2025-09-09) [3]: 28a7e27 (contrib/subtree: detect rewritten subtree commits, 2026-01-09) [4]: <20251230170719.845029-1-george@mail.dietrich.pub> Reported-by: George <george@mail.dietrich.pub> Reported-by: Christian Heusel <christian@heusel.eu> Signed-off-by: Colin Stagner <ask+git@howdoi.land> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 715b406 commit 1f70684

2 files changed

Lines changed: 65 additions & 53 deletions

File tree

contrib/subtree/git-subtree.sh

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -785,42 +785,6 @@ ensure_valid_ref_format () {
785785
die "fatal: '$1' does not look like a ref"
786786
}
787787

788-
# Usage: should_ignore_subtree_split_commit REV
789-
#
790-
# Check if REV is a commit from another subtree and should be
791-
# ignored from processing for splits
792-
should_ignore_subtree_split_commit () {
793-
assert test $# = 1
794-
795-
git show \
796-
--no-patch \
797-
--no-show-signature \
798-
--format='%(trailers:key=git-subtree-dir,key=git-subtree-mainline)' \
799-
"$1" |
800-
(
801-
have_mainline=
802-
subtree_dir=
803-
804-
while read -r trailer val
805-
do
806-
case "$trailer" in
807-
git-subtree-dir:)
808-
subtree_dir="${val%/}" ;;
809-
git-subtree-mainline:)
810-
have_mainline=y ;;
811-
esac
812-
done
813-
814-
if test -n "${subtree_dir}" &&
815-
test -z "${have_mainline}" &&
816-
test "${subtree_dir}" != "$arg_prefix"
817-
then
818-
return 0
819-
fi
820-
return 1
821-
)
822-
}
823-
824788
# Usage: process_split_commit REV PARENTS
825789
process_split_commit () {
826790
assert test $# = 2
@@ -1006,19 +970,7 @@ cmd_split () {
1006970
eval "$grl" |
1007971
while read rev parents
1008972
do
1009-
if should_ignore_subtree_split_commit "$rev"
1010-
then
1011-
continue
1012-
fi
1013-
parsedparents=''
1014-
for parent in $parents
1015-
do
1016-
if ! should_ignore_subtree_split_commit "$parent"
1017-
then
1018-
parsedparents="$parsedparents$parent "
1019-
fi
1020-
done
1021-
process_split_commit "$rev" "$parsedparents"
973+
process_split_commit "$rev" "$parents"
1022974
done || exit $?
1023975

1024976
latest_new=$(cache_get latest_new) || exit $?

contrib/subtree/t/t7900-subtree.sh

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -428,8 +428,7 @@ test_expect_success 'split sub dir/ with --rejoin' '
428428
# - Perform 'split' on subtree B
429429
# - Create new commits with changes to subtree A and B
430430
# - Perform split on subtree A
431-
# - Check that the commits in subtree B are not processed
432-
# as part of the subtree A split
431+
# - Check for expected history
433432
test_expect_success 'split with multiple subtrees' '
434433
subtree_test_create_repo "$test_count" &&
435434
subtree_test_create_repo "$test_count/subA" &&
@@ -458,8 +457,8 @@ test_expect_success 'split with multiple subtrees' '
458457
--squash --rejoin -m "Sub A Split 2" -b a2 &&
459458
test "$(git -C "$test_count" rev-list --count main..a2)" -eq 2 &&
460459
test "$(git -C "$test_count" rev-list --count a1..a2)" -eq 1 &&
461-
test "$(git -C "$test_count" subtree split --prefix=subBDir \
462-
--squash --rejoin -d -m "Sub B Split 1" -b b2 2>&1 | grep -w "\[1\]")" = "" &&
460+
git -C "$test_count" subtree split --prefix=subBDir \
461+
--squash --rejoin -d -m "Sub B Split 1" -b b2 &&
463462
test "$(git -C "$test_count" rev-list --count main..b2)" -eq 2 &&
464463
test "$(git -C "$test_count" rev-list --count b1..b2)" -eq 1
465464
'
@@ -507,6 +506,67 @@ do
507506
'
508507
done
509508

509+
# Usually,
510+
#
511+
# git subtree merge -P subA --squash f00...
512+
#
513+
# makes two commits, in this order:
514+
#
515+
# 1. Squashed 'subA/' content from commit f00...
516+
# 2. Merge commit (1) as 'subA'
517+
#
518+
# Commit 1 updates the subtree but does *not* rewrite paths.
519+
# Commit 2 rewrites all trees to start with `subA/`
520+
#
521+
# Commit 1 either has no parents or depends only on other
522+
# "Squashed 'subA/' content" commits.
523+
#
524+
# For merge without --squash, subtree produces just one commit:
525+
# a merge commit with git-subtree trailers.
526+
#
527+
# In either case, if the user rebases these commits, they will
528+
# still have the git-subtree-* trailers… but will NOT have
529+
# the layout described above.
530+
#
531+
# Test that subsequent `git subtree split` are not confused by this.
532+
test_expect_success 'split with rebased subtree commit' '
533+
subtree_test_create_repo "$test_count" &&
534+
(
535+
cd "$test_count" &&
536+
test_commit file0 &&
537+
test_create_subtree_add \
538+
. mksubtree subA file1 --squash &&
539+
test_path_is_file subA/file1.t &&
540+
mkdir subB &&
541+
test_commit subB/bfile &&
542+
git commit --amend -F - <<'EOF' &&
543+
Squashed '\''subB/'\'' content from commit '\''badf00da911bbe895347b4b236f5461d55dc9877'\''
544+
545+
Simulate a cherry-picked or rebased subtree commit.
546+
547+
git-subtree-dir: subB
548+
git-subtree-split: badf00da911bbe895347b4b236f5461d55dc9877
549+
EOF
550+
test_commit subA/file2 &&
551+
test_commit subB/bfile2 &&
552+
git commit --amend -F - <<'EOF' &&
553+
Split '\''subB/'\'' into commit '\''badf00da911bbe895347b4b236f5461d55dc9877'\''
554+
555+
Simulate a cherry-picked or rebased subtree commit.
556+
557+
git-subtree-dir: subB
558+
git-subtree-mainline: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
559+
git-subtree-split: badf00da911bbe895347b4b236f5461d55dc9877
560+
EOF
561+
git subtree split --prefix=subA --branch=bsplit &&
562+
git checkout bsplit &&
563+
test_path_is_file file1.t &&
564+
test_path_is_file file2.t &&
565+
test "$(last_commit_subject)" = "subA/file2" &&
566+
test "$(git rev-list --count bsplit)" -eq 2
567+
)
568+
'
569+
510570
test_expect_success 'split sub dir/ with --rejoin from scratch' '
511571
subtree_test_create_repo "$test_count" &&
512572
test_create_commit "$test_count" main1 &&

0 commit comments

Comments
 (0)