Hi,
I'm observing an issue with 'git subtree split' after adding a subtree
(with 'git subtree add') in a topic branch, and then merging the topic
branch back into the main branch (where other/unrelated changes have
accrued in the meantime). The patch below adds a failing t7900-subtree
test case which reproduces the issue.
Here is the repo history just prior to running 'git subtree split'
$ git log --oneline --graph --decorate
* 1355f01 (HEAD -> master) merge should be skipped on subtree
|\
| * adc8ecf (otherbranch) Add 'sub dir/' from commit '5280958b2f997c3ce7bff7192cceb19f55b45cd9'
| |\
| | * 5280958 sub1
* | a966436 other_unrelated
|/
* 9b6e8f6 main1
The only commit that touches the subtree is the one created by
'git subtree add' (adc8ecf), hence I would expect 'git subtree split'
to NOT synthesize any new history, but simply return the last commit
in the subtree history (5280958)
However, 'git subtree split' ends up synthesizing a history that
replicates the master history since the start of the topic branch
(which consists of entirely unrelated commits) from the superproject
(aka. mainline) into the synthesized subtree history (57cbde8 and its
left-hand parent in this graph):
$ git log --oneline --graph --all
* 1355f01 (HEAD -> master) merge should be skipped on subtree
|\
| * adc8ecf (otherbranch) Add 'sub dir/' from commit '5280958b2f997c3ce7bff7192cceb19f55b45cd9'
| |\
| | | * 57cbde8 (subproj-br) merge should be skipped on subtree
| |_|/|
|/| |/
| | * 5280958 sub1
* | a966436 other_unrelated
|/
* 9b6e8f6 main1
I've been trying to understand how the subtree cache (mis)behaves in
this case. The cache is initially seeded from find_existing_splits(),
which finds these lines the adc8ecf commit message:
git-subtree-mainline: 9b6e8f677b700a00e9f1715e2624bf5ed756dc85
git-subtree-split: 5280958b2f997c3ce7bff7192cceb19f55b45cd9
and adds these corresponding entries to the cache:
9b6e8f6 -> 5280958
5280958 -> 5280958
In other words, the cache starts out claiming that 5280958 is the
equivalent subtree commit for the 9b6e8f6 mainline commit.
However, in my naive understanding this does not make sense, as
9b6e8f6 _precedes_ the subtree addition, and has no content in
the relevant subdir.
When process_split_commit() proceeds to tackle each subsequent commit
in order, it adds the following cache entries:
a966436 -> a966436 (notree flag set)
adc8ecf -> 5280958 (correct, AFAICS)
1355f01 -> 57cbde8 (synthesizing a merge of a966436 and 5280958)
AFAICS, the problem originates from the combination of:
- the first mainline commit 9b6e8f6 mapping to subtree commit 5280958
- the subsequent mainline commit a966436 mapping to _itself_
I suspect that if both had pointed to the same cache state (either
both pointing to 5280958, or both having set the notree flag, the
latter makes more sense, IMHO), then we would skip synthesizing the
incorrect history. However I suspect I've overlooked something as I
have yet to find a simple tweak that does not break other test cases.
Stay safe,
...Johan
---
contrib/subtree/t/t7900-subtree.sh | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 57ff4b25c1..ef1fcaa3c0 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -971,6 +971,34 @@ test_expect_success 'push split to subproj' '
)
'
+# If a subtree is added to a topic branch, which is then merged back to master
+# (where other, unrelated commits have been committed in meantime, a later
+# subtree split should not generate any new history (as nothing has touched
+# the sub dir since it was added)
+
+next_test
+test_expect_success 'split a subtree across mainline history with merge' '
+ subtree_test_create_repo "$subtree_test_count" &&
+ subtree_test_create_repo "$subtree_test_count/sub proj" &&
+ test_create_commit "$subtree_test_count" main1 &&
+ test_create_commit "$subtree_test_count/sub proj" sub1 &&
+ sub_head=$(cd "$subtree_test_count/sub proj"; git rev-parse HEAD) &&
+ (
+ cd "$subtree_test_count" &&
+ git checkout -b otherbranch &&
+ git fetch ./"sub proj" master &&
+ git subtree add --prefix="sub dir" FETCH_HEAD &&
+ git checkout master
+ ) &&
+ test_create_commit "$subtree_test_count" other_unrelated &&
+ (
+ cd "$subtree_test_count" &&
+ git merge -m "merge should be skipped on subtree" otherbranch &&
+ git subtree split --prefix="sub dir" --branch subproj-br -d &&
+ check_equal "$(git rev-parse subproj-br)" "$sub_head"
+ )
+'
+
#
# 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
--
2.25.4
