Message ID | pull.1587.v5.git.1701206267300.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v5] subtree: fix split processing with multiple subtrees present | expand |
On Tue, Nov 28, 2023 at 10:17 PM Zach FettersMoore via GitGitGadget <gitgitgadget@gmail.com> wrote: > 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'A and 'apollo-ios-codegen' It looks like there is a spurious A after 'apollo-ios' in the line above. > directories > -Create a commit containing these changes > -Do a split on apollo-ios-codegen > - git subtree split --prefix=apollo-ios-codegen --squash --rejoin I might be doing something stupid or wrong, but I get the following: $ git subtree split --prefix=apollo-ios-codegen --squash --rejoin fatal: could not rev-parse split hash cc70a7d49e84696f0df210710445784c504ed748 from commit 360f068ea0d57f250621ab7dbe205313f52a0e98 hint: hash might be a tag, try fetching it from the subtree repository: hint: git fetch <subtree-repository> cc70a7d49e84696f0df210710445784c504ed748 > -Do a split on apollo-ios > - git subtree split --prefix=apollo-ios --squash --rejoin Same issue: $ git subtree split --prefix=apollo-ios --squash --rejoin fatal: could not rev-parse split hash b852c0aa1fd5ab9e1323da92b606ad3f2211e111 from commit b48030c3eb6e2faf4bff981c5c63ca72aceecdfa hint: hash might be a tag, try fetching it from the subtree repository: hint: git fetch <subtree-repository> b852c0aa1fd5ab9e1323da92b606ad3f2211e111 I didn't try to get farther than this, as it seems that some instructions might be missing. [...] > 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. Nice! Except for the above issues in the commit message, the rest of the patch looks good to me, thanks!
>> 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'A and 'apollo-ios-codegen' > It looks like there is a spurious A after 'apollo-ios' in the line above. Thanks for catching that, definitely a typo on my part. >> directories >> -Create a commit containing these changes >> -Do a split on apollo-ios-codegen >> - git subtree split --prefix=apollo-ios-codegen --squash --rejoin > I might be doing something stupid or wrong, but I get the following: > > $ git subtree split --prefix=apollo-ios-codegen --squash --rejoin > fatal: could not rev-parse split hash > cc70a7d49e84696f0df210710445784c504ed748 from commit > 360f068ea0d57f250621ab7dbe205313f52a0e98 > hint: hash might be a tag, try fetching it from the subtree repository: > hint: git fetch <subtree-repository> cc70a7d49e84696f0df210710445784c504ed748 Updated this to include doing a fetch to ensure all remote repo info is available locally. >> -Do a split on apollo-ios >> - git subtree split --prefix=apollo-ios --squash --rejoin > Same issue: > > $ git subtree split --prefix=apollo-ios --squash --rejoin > fatal: could not rev-parse split hash > b852c0aa1fd5ab9e1323da92b606ad3f2211e111 from commit > b48030c3eb6e2faf4bff981c5c63ca72aceecdfa > hint: hash might be a tag, try fetching it from the subtree repository: > hint: git fetch <subtree-repository> b852c0aa1fd5ab9e1323da92b606ad3f2211e111 > > I didn't try to get farther than this, as it seems that some > instructions might be missing. Same as above, added extra instruction to do a fetch first. Also added a little extra info that the issue may present after the first split in the instructions depending on the current state of the repo being used. Also added a way to do the same steps with the changes applied to see that it resolves the issue. >> 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. > Nice! > > Except for the above issues in the commit message, the rest of the > patch looks good to me, thanks! Great! Thanks for the review and guidance!
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index e0c5d3b0de6..a0bf958ea66 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -778,6 +778,22 @@ ensure_valid_ref_format () { die "fatal: '$1' does not look like a ref" } +# 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)" + 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 + fi + return 1 +} + # Usage: process_split_commit REV PARENTS process_split_commit () { assert test $# = 2 @@ -963,7 +979,19 @@ cmd_split () { eval "$grl" | while read rev parents do - process_split_commit "$rev" "$parents" + if should_ignore_subtree_split_commit "$rev" + then + continue + fi + parsedparents='' + for parent in $parents + do + if ! should_ignore_subtree_split_commit "$parent" + then + parsedparents="$parsedparents$parent " + fi + done + process_split_commit "$rev" "$parsedparents" done || exit $? latest_new=$(cache_get latest_new) || exit $? diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 49a21dd7c9c..ca4df5be832 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -385,6 +385,46 @@ test_expect_success 'split sub dir/ with --rejoin' ' ) ' +# Tests that commits from other subtrees are not processed as +# part of a split. +# +# This test performs the following: +# - Creates Repo with subtrees 'subA' and 'subB' +# - Creates commits in the repo including changes to subtrees +# - Runs the following 'split' and commit' commands in order: +# - Perform 'split' on subtree A +# - Perform 'split' on subtree B +# - Create new commits with changes to subtree A and B +# - Perform split on subtree A +# - Check that the commits in subtree B are not processed +# as part of the subtree A split +test_expect_success 'split with multiple subtrees' ' + subtree_test_create_repo "$test_count" && + subtree_test_create_repo "$test_count/subA" && + subtree_test_create_repo "$test_count/subB" && + test_create_commit "$test_count" main1 && + test_create_commit "$test_count/subA" subA1 && + test_create_commit "$test_count/subA" subA2 && + test_create_commit "$test_count/subA" subA3 && + test_create_commit "$test_count/subB" subB1 && + git -C "$test_count" fetch ./subA HEAD && + git -C "$test_count" subtree add --prefix=subADir FETCH_HEAD && + git -C "$test_count" fetch ./subB HEAD && + git -C "$test_count" subtree add --prefix=subBDir FETCH_HEAD && + test_create_commit "$test_count" subADir/main-subA1 && + test_create_commit "$test_count" subBDir/main-subB1 && + git -C "$test_count" subtree split --prefix=subADir \ + --squash --rejoin -m "Sub A Split 1" && + git -C "$test_count" subtree split --prefix=subBDir \ + --squash --rejoin -m "Sub B Split 1" && + test_create_commit "$test_count" subADir/main-subA2 && + test_create_commit "$test_count" subBDir/main-subB2 && + git -C "$test_count" subtree split --prefix=subADir \ + --squash --rejoin -m "Sub A Split 2" && + test "$(git -C "$test_count" subtree split --prefix=subBDir \ + --squash --rejoin -d -m "Sub B Split 1" 2>&1 | grep -w "\[1\]")" = "" +' + test_expect_success 'split sub dir/ with --rejoin from scratch' ' subtree_test_create_repo "$test_count" && test_create_commit "$test_count" main1 &&