Skip to content

Commit 98ba49c

Browse files
BobaFettersgitster
authored andcommitted
subtree: fix split processing with multiple subtrees present
When there are multiple subtrees present in a repository and they are all using 'git subtree split', the 'split' command can take a significant (and constantly growing) amount of time to run even when using the '--rejoin' flag. This is due to the fact that when processing commits to determine the last known split to start from when looking for changes, if there has been a split/merge done from another subtree there will be 2 split commits, one mainline and one subtree, for the second subtree that are part of the processing. The non-mainline subtree split commit will cause the processing to always need to search the entire history of the given subtree as part of its processing even though those commits are totally irrelevant to the current subtree split being run. To see this in practice you can use the open source GitHub repo 'apollo-ios-dev' and do the following in order: -Make a changes to a file in 'apollo-ios' and 'apollo-ios-codegen' directories -Create a commit containing these changes -Do a split on apollo-ios-codegen - Do a fetch on the subtree repo - git fetch [email protected]:apollographql/apollo-ios-codegen.git - git subtree split --prefix=apollo-ios-codegen --squash --rejoin - Depending on the current state of the 'apollo-ios-dev' repo you may see the issue at this point if the last split was on apollo-ios -Do a split on apollo-ios - Do a fetch on the subtree repo - git fetch [email protected]:apollographql/apollo-ios.git - git subtree split --prefix=apollo-ios --squash --rejoin -Make changes to a file in apollo-ios-codegen -Create a commit containing the change(s) -Do a split on apollo-ios-codegen - git subtree split --prefix=apollo-ios-codegen --squash --rejoin -To see that the patch fixes the issue you can use the custom subtree script in the repo so following the same steps as above, except instead of using 'git subtree ...' for the commands use 'git-subtree.sh ...' for the commands You will see that the final split is looking for the last split on apollo-ios-codegen to use as it's starting point to process commits. Since there is a split commit from apollo-ios in between the 2 splits run on apollo-ios-codegen, the processing ends up traversing the entire history of apollo-ios which increases the time it takes to do a split based on how long of a history apollo-ios has, while none of these commits are relevant to the split being done on apollo-ios-codegen. So this commit makes a change to the processing of commits for the split command in order to ignore non-mainline commits from other subtrees such as apollo-ios in the above breakdown by adding a new function 'should_ignore_subtree_commit' which is called during 'process_split_commit'. This allows the split/rejoin processing to still function as expected but removes all of the unnecessary processing that takes place currently which greatly inflates the processing time. In the above example, previously the final split would take ~10-12 minutes, while after this fix it takes seconds. Added a test to validate that the proposed fix solves the issue. The test accomplishes this by checking the output of the split command to ensure the output from the progress of 'process_split_commit' function that represents the 'extracount' of commits processed remains at 0, meaning none of the commits from the second subtree were processed. This was tested against the original functionality to show the test failed, and then with this fix to show the test passes. This illustrated that when using multiple subtrees, A and B, when doing a split on subtree B, the processing does not traverse the entire history of subtree A which is unnecessary and would cause the 'extracount' of processed commits to climb based on the number of commits in the history of subtree A. Signed-off-by: Zach FettersMoore <[email protected]> Reviewed-by: Christian Couder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e02ecfc commit 98ba49c

File tree

2 files changed

+69
-1
lines changed

2 files changed

+69
-1
lines changed

contrib/subtree/git-subtree.sh

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,22 @@ ensure_valid_ref_format () {
787787
die "fatal: '$1' does not look like a ref"
788788
}
789789

790+
# Usage: check if a commit from another subtree should be
791+
# ignored from processing for splits
792+
should_ignore_subtree_split_commit () {
793+
assert test $# = 1
794+
local rev="$1"
795+
if test -n "$(git log -1 --grep="git-subtree-dir:" $rev)"
796+
then
797+
if test -z "$(git log -1 --grep="git-subtree-mainline:" $rev)" &&
798+
test -z "$(git log -1 --grep="git-subtree-dir: $arg_prefix$" $rev)"
799+
then
800+
return 0
801+
fi
802+
fi
803+
return 1
804+
}
805+
790806
# Usage: process_split_commit REV PARENTS
791807
process_split_commit () {
792808
assert test $# = 2
@@ -972,7 +988,19 @@ cmd_split () {
972988
eval "$grl" |
973989
while read rev parents
974990
do
975-
process_split_commit "$rev" "$parents"
991+
if should_ignore_subtree_split_commit "$rev"
992+
then
993+
continue
994+
fi
995+
parsedparents=''
996+
for parent in $parents
997+
do
998+
if ! should_ignore_subtree_split_commit "$parent"
999+
then
1000+
parsedparents="$parsedparents$parent "
1001+
fi
1002+
done
1003+
process_split_commit "$rev" "$parsedparents"
9761004
done || exit $?
9771005

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

contrib/subtree/t/t7900-subtree.sh

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,46 @@ test_expect_success 'split sub dir/ with --rejoin' '
385385
)
386386
'
387387

388+
# Tests that commits from other subtrees are not processed as
389+
# part of a split.
390+
#
391+
# This test performs the following:
392+
# - Creates Repo with subtrees 'subA' and 'subB'
393+
# - Creates commits in the repo including changes to subtrees
394+
# - Runs the following 'split' and commit' commands in order:
395+
# - Perform 'split' on subtree A
396+
# - Perform 'split' on subtree B
397+
# - Create new commits with changes to subtree A and B
398+
# - Perform split on subtree A
399+
# - Check that the commits in subtree B are not processed
400+
# as part of the subtree A split
401+
test_expect_success 'split with multiple subtrees' '
402+
subtree_test_create_repo "$test_count" &&
403+
subtree_test_create_repo "$test_count/subA" &&
404+
subtree_test_create_repo "$test_count/subB" &&
405+
test_create_commit "$test_count" main1 &&
406+
test_create_commit "$test_count/subA" subA1 &&
407+
test_create_commit "$test_count/subA" subA2 &&
408+
test_create_commit "$test_count/subA" subA3 &&
409+
test_create_commit "$test_count/subB" subB1 &&
410+
git -C "$test_count" fetch ./subA HEAD &&
411+
git -C "$test_count" subtree add --prefix=subADir FETCH_HEAD &&
412+
git -C "$test_count" fetch ./subB HEAD &&
413+
git -C "$test_count" subtree add --prefix=subBDir FETCH_HEAD &&
414+
test_create_commit "$test_count" subADir/main-subA1 &&
415+
test_create_commit "$test_count" subBDir/main-subB1 &&
416+
git -C "$test_count" subtree split --prefix=subADir \
417+
--squash --rejoin -m "Sub A Split 1" &&
418+
git -C "$test_count" subtree split --prefix=subBDir \
419+
--squash --rejoin -m "Sub B Split 1" &&
420+
test_create_commit "$test_count" subADir/main-subA2 &&
421+
test_create_commit "$test_count" subBDir/main-subB2 &&
422+
git -C "$test_count" subtree split --prefix=subADir \
423+
--squash --rejoin -m "Sub A Split 2" &&
424+
test "$(git -C "$test_count" subtree split --prefix=subBDir \
425+
--squash --rejoin -d -m "Sub B Split 1" 2>&1 | grep -w "\[1\]")" = ""
426+
'
427+
388428
test_expect_success 'split sub dir/ with --rejoin from scratch' '
389429
subtree_test_create_repo "$test_count" &&
390430
test_create_commit "$test_count" main1 &&

0 commit comments

Comments
 (0)