git subtree bug produces divergent descendants

From: David Ware <davidw@realtimegenomics.com>
Date: Dec 6, 2015, 22:09 UTC
Message-ID: <CAET=KiVXh2UZwRSpM_+wX_QpfjBsyfdPPUVDSDoCRVe_0wbhCg@mail.gmail.com>

My group has run into a bug with "git-subtree split". Under some circumstances a split created from a descendant of another earlier split is not a descendant of that earlier split (thus blocking pushes). We originally noticed this on v1.9.1 but have also checked it on v2.6.3

When scanning the commits to produce the subtree it seems to skip creating a new commit if any of the parent commits have the same tree and instead uses that tree in its place. This is fine when the cause is a branch that did not cause any changes to the subtree. However it creates an issue when the cause is both branches ending up with the same tree through identical alterations (or more likely, one of the branches has just a subset of the alterations on the other, such as a branch just containing cherry-picks).

The attached patch (against v2.6.3) includes a test that reproduces the problem. The created 'master' branch has had the latest commits on the 'branch' branch merged into it, so it follows that a subtree on 'folder/' at 'master' (subtree_tip) should contain all the commits of a subtree on 'folder/' at 'branch' (subtree_branch). Hence it should be possible to push subtree_tip to subtree_branch.

The attached patch also fixes the issue for the cases we've encountered, however since we're not particularly familiar with git internals we may not have approached this optimally. We suspect it could be improved to also handle the cases where there are more than 2 parents.

Cheers, Dave Ware

From ce6e2bcb2116624082bf46663aa33c706fcab930 Mon Sep 17 00:00:00 2001
From: Dave Ware <davidw@netvalue.net.nz>
Date: Fri, 4 Dec 2015 16:30:03 +1300
Subject: [PATCH] Fix bug in git-subtree split.

A bug occurs in 'git-subtree split' where a merge is skipped even when both parents act on the subtree, provided the merge results in a tree identical to one of the parents. Fixed 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 adding a test case, this checks that a descendant can be pushed to
it's ancestor in this case.
---
 contrib/subtree/git-subtree.sh           | 12 +++++--
 contrib/subtree/t/t7901-subtree-split.sh | 62 ++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100755 contrib/subtree/t/t7901-subtree-split.sh

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 9f06571..b837531 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 --boundary $identical..$nonidentical)
+		if [ -n "$extras" ]; 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/t7901-subtree-split.sh b/contrib/subtree/t/t7901-subtree-split.sh
new file mode 100755
index 0000000..0a1ea56
--- /dev/null
+++ b/contrib/subtree/t/t7901-subtree-split.sh
@@ -0,0 +1,62 @@
+#!/bin/bash
+
+test_description='Test for bug in subtree commit filtering'
+
+
+TEST_DIRECTORY=$(pwd)/../../../t
+export TEST_DIRECTORY
+
+. ../../../t/test-lib.sh
+
+
+test_expect_success 'subtree descendent check' '
+  mkdir git_subtree_split_check &&
+  cd git_subtree_split_check &&
+  git init &&
+
+  mkdir folder &&
+
+  echo a > folder/a &&
+  git add . &&
+  git commit -m "first commit" &&
+
+  git branch branch &&
+
+  echo 0 > folder/0 &&
+  git add . &&
+  git commit -m "adding 0 to folder" &&
+
+  echo b > folder/b &&
+  git add . &&
+  git commit -m "adding b to folder" &&
+  git rev-list HEAD -1 > cherry.rev &&
+
+  git checkout branch &&
+  echo text > textBranch.txt &&
+  git add . &&
+  git commit -m "commit to fiddle with branch: branch" &&
+
+  git cherry-pick $(cat cherry.rev) &&
+  git checkout master &&
+  git merge -m "merge" branch &&
+
+  git branch noop_branch &&
+
+  echo d > folder/d &&
+  git add . &&
+  git commit -m "adding d to folder" &&
+
+  git checkout noop_branch &&
+  echo moreText > anotherText.txt &&
+  git add . &&
+  git commit -m "irrelevant" &&
+
+  git checkout master &&
+  git merge -m "second merge" noop_branch &&
+
+  git subtree split --prefix folder/ --branch subtree_tip master &&
+  git subtree split --prefix folder/ --branch subtree_branch branch &&
+  git push . subtree_tip:subtree_branch
+  '
+
+test_done

-- 
1.9.1