contrib/subtree: fix "subtree split" skipped-merge bug

'git subtree split' can incorrectly skip a merge even when both parents act on the subtree, provided the merge results in a tree identical to one of the parents. Fix by copying the merge if at least one parent is non-identical, and the non-identical parent is not an ancestor of the identical parent. Also, add a test case which checks that a descendant remains a descendent on the subtree in this case. Signed-off-by: Dave Ware <davidw@realtimegenomics.com> Reviewed-by: David A. Greene <greened@obbligato.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Dave Ware committed Jan 15, 2016 at 13:41 UTC 933cfeb90b5d03b4096db6d60494a6eedea25d03
2 files changed +70 -2
contrib/subtree/git-subtree.sh
+10 -2
@@ -479,8 +479,16 @@ copy_or_skip()
479 p="$p -p $parent"
480 fi
481 done
482 -
483 - if [ -n "$identical" ]; then
482 +
483 + copycommit=
484 + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
485 + extras=$(git rev-list --count $identical..$nonidentical)
486 + if [ "$extras" -ne 0 ]; then
487 + # we need to preserve history along the other branch
488 + copycommit=1
489 + fi
490 + fi
491 + if [ -n "$identical" ] && [ -z "$copycommit" ]; then
492 echo $identical
493 else
494 copy_commit $rev $tree "$p" || exit $?
contrib/subtree/t/t7900-subtree.sh
+60
@@ -1014,4 +1014,64 @@ test_expect_success 'push split to subproj' '
1014 )
1015 '
1016
1017 +#
1018 +# This test covers 2 cases in subtree split copy_or_skip code
1019 +# 1) Merges where one parent is a superset of the changes of the other
1020 +# parent regarding changes to the subtree, in this case the merge
1021 +# commit should be copied
1022 +# 2) Merges where only one parent operate on the subtree, and the merge
1023 +# commit should be skipped
1024 +#
1025 +# (1) is checked by ensuring subtree_tip is a descendent of subtree_branch
1026 +# (2) should have a check added (not_a_subtree_change shouldn't be present
1027 +# on the produced subtree)
1028 +#
1029 +# Other related cases which are not tested (or currently handled correctly)
1030 +# - Case (1) where there are more than 2 parents, it will sometimes correctly copy
1031 +# the merge, and sometimes not
1032 +# - Merge commit where both parents have same tree as the merge, currently
1033 +# will always be skipped, even if they reached that state via different
1034 +# set of commits.
1035 +#
1036 +
1037 +next_test
1038 +test_expect_success 'subtree descendant check' '
1039 + subtree_test_create_repo "$subtree_test_count" &&
1040 + test_create_commit "$subtree_test_count" folder_subtree/a &&
1041 + (
1042 + cd "$subtree_test_count" &&
1043 + git branch branch
1044 + ) &&
1045 + test_create_commit "$subtree_test_count" folder_subtree/0 &&
1046 + test_create_commit "$subtree_test_count" folder_subtree/b &&
1047 + cherry=$(cd "$subtree_test_count"; git rev-parse HEAD) &&
1048 + (
1049 + cd "$subtree_test_count" &&
1050 + git checkout branch
1051 + ) &&
1052 + test_create_commit "$subtree_test_count" commit_on_branch &&
1053 + (
1054 + cd "$subtree_test_count" &&
1055 + git cherry-pick $cherry &&
1056 + git checkout master &&
1057 + git merge -m "merge should be kept on subtree" branch &&
1058 + git branch no_subtree_work_branch
1059 + ) &&
1060 + test_create_commit "$subtree_test_count" folder_subtree/d &&
1061 + (
1062 + cd "$subtree_test_count" &&
1063 + git checkout no_subtree_work_branch
1064 + ) &&
1065 + test_create_commit "$subtree_test_count" not_a_subtree_change &&
1066 + (
1067 + cd "$subtree_test_count" &&
1068 + git checkout master &&
1069 + git merge -m "merge should be skipped on subtree" no_subtree_work_branch &&
1070 +
1071 + git subtree split --prefix folder_subtree/ --branch subtree_tip master &&
1072 + git subtree split --prefix folder_subtree/ --branch subtree_branch branch &&
1073 + check_equal $(git rev-list --count subtree_tip..subtree_branch) 0
1074 + )
1075 +'
1076 +
1077 test_done