git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v6] contrib/subtree: fix "subtree split" skipped-merge bug

From
Dave Ware <davidw@realtimegenomics.com>
Date
Jan 14, 2016, 21:26 UTC
Message-ID
<1452806795-26621-1-git-send-email-davidw@realtimegenomics.com>
In-Reply-To
<87bn8o97mh.fsf@waller.obbligato.org>

'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>
---
 contrib/subtree/git-subtree.sh     | 12 ++++++--
 contrib/subtree/t/t7900-subtree.sh | 60 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index edf36f8..5c83727 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -479,8 +479,16 @@ copy_or_skip()
 			p="$p -p $parent"
 		fi
 	done
-	
-	if [ -n "$identical" ]; then
+
+	copycommit=
+	if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
+		extras=$(git rev-list --count $identical..$nonidentical)
+		if [ "$extras" -ne 0 ]; then
+			# we need to preserve history along the other branch
+			copycommit=1
+		fi
+	fi
+	if [ -n "$identical" ] && [ -z "$copycommit" ]; then
 		echo $identical
 	else
 		copy_commit $rev $tree "$p" || exit $?
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 751aee3..c5089c3 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -1014,4 +1014,64 @@ test_expect_success 'push split to subproj' '
 	)
 '
 
+#
+# This test covers 2 cases in subtree split copy_or_skip code
+# 1) Merges where one parent is a superset of the changes of the other
+#    parent regarding changes to the subtree, in this case the merge
+#    commit should be copied
+# 2) Merges where only one parent operate on the subtree, and the merge
+#    commit should be skipped
+#
+# (1) is checked by ensuring subtree_tip is a descendent of subtree_branch
+# (2) should have a check added (not_a_subtree_change shouldn't be present
+#     on the produced subtree)
+#
+# Other related cases which are not tested (or currently handled correctly)
+# - Case (1) where there are more than 2 parents, it will sometimes correctly copy
+#   the merge, and sometimes not
+# - Merge commit where both parents have same tree as the merge, currently
+#   will always be skipped, even if they reached that state via different
+#   set of commits.
+#
+
+next_test
+test_expect_success 'subtree descendant check' '
+	subtree_test_create_repo "$subtree_test_count" &&
+	test_create_commit "$subtree_test_count" folder_subtree/a &&
+	(
+		cd "$subtree_test_count" &&
+		git branch branch
+	) &&
+	test_create_commit "$subtree_test_count" folder_subtree/0 &&
+	test_create_commit "$subtree_test_count" folder_subtree/b &&
+	cherry=$(cd "$subtree_test_count"; git rev-parse HEAD) &&
+	(
+		cd "$subtree_test_count" &&
+		git checkout branch
+	) &&
+	test_create_commit "$subtree_test_count" commit_on_branch &&
+	(
+		cd "$subtree_test_count" &&
+		git cherry-pick $cherry &&
+		git checkout master &&
+		git merge -m "merge should be kept on subtree" branch &&
+		git branch no_subtree_work_branch
+	) &&
+	test_create_commit "$subtree_test_count" folder_subtree/d &&
+	(
+		cd "$subtree_test_count" &&
+		git checkout no_subtree_work_branch
+	) &&
+	test_create_commit "$subtree_test_count" not_a_subtree_change &&
+	(
+		cd "$subtree_test_count" &&
+		git checkout master
+		git merge -m "merge should be skipped on subtree" no_subtree_work_branch
+
+		git subtree split --prefix folder_subtree/ --branch subtree_tip master &&
+		git subtree split --prefix folder_subtree/ --branch subtree_branch branch
+		check_equal $(git rev-list --count subtree_tip..subtree_branch) 0
+	)
+'
+
 test_done
-- 
1.9.1
Previous: David A. GreeneNext: Dave Ware
Message 16 of 22 in “git subtree bug produces divergent descendants”
  1. David WareDec 6, 2015
  2. Eric SunshineDec 7, 2015
  3. contrib/subtree: fix "subtree split" skipped-merge bug.Dave Ware, Dec 7, 2015
  4. Eric SunshineDec 8, 2015
  5. contrib/subtree: fix "subtree split" skipped-merge bugDave Ware, Dec 8, 2015
  6. Junio C HamanoDec 8, 2015
  7. David WareDec 9, 2015
  8. contrib/subtree: fix "subtree split" skipped-merge bugDave Ware, Dec 9, 2015
  9. Eric SunshineDec 9, 2015
  10. contrib/subtree: fix "subtree split" skipped-merge bugDave Ware, Dec 9, 2015
  11. David A. GreeneJan 13, 2016
  12. David WareJan 13, 2016
  13. David A. GreeneJan 14, 2016
  14. David WareJan 14, 2016
  15. David A. GreeneJan 17, 2016
  16. contrib/subtree: fix "subtree split" skipped-merge bugDave Ware, Jan 14, 2016
  17. contrib/subtree: fix "subtree split" skipped-merge bugDave Ware, Jan 15, 2016
  18. Eric SunshineJan 15, 2016
  19. Junio C HamanoJan 15, 2016
  20. Eric SunshineJan 15, 2016
  21. David A. GreeneJan 17, 2016
  22. David WareDec 7, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.