Skip to content

Commit ba58ddd

Browse files
phil-blaingitster
authored andcommitted
t5572: add notes on a peculiar test
Test 5572.63 ("branch has no merge base with remote-tracking counterpart") was introduced in 4d36f88 (submodule: do not pass null OID to setup_revisions, 2018-05-24), as a regression test for the bug this commit was fixing (preventing a 'fatal: bad object' error when the current branch and the remote-tracking branch we are pulling have no merge-base). However, the commit message for 4d36f88 does not describe in which real-life situation this bug was encountered. The brief discussion on the mailing list [1] does not either. The regression test is not really representative of a real-life scenario: both the local repository and its upstream have only a single commit, and the "no merge-base" scenario is simulated by recreating this root commit in the local repository using 'git commit-tree' before calling 'git pull --rebase --recurse-submodules'. The rebase succeeds and results in the local branch being reset to the same root commit as the upstream branch. The fix in 4d36f88 modifies 'submodule.c::submodule_touches_in_range' so that if 'excl_oid' is null, which is the case when the 'git merge-base --fork-point' invocation in 'builtin/pull.c::get_rebase_fork_point' errors (no fork-point), then instead of 'incl_oid --not excl_oid' being passed to setup_revisions, only 'incl_oid' is passed, and 'submodule_touches_in_range' examines 'incl_oid' and all its ancestors to verify that they do not touch the submodule. In test 5572.63, the recreated lone root commit in the local repository is thus the only commit being examined by 'submodule_touches_in_range', and this commit *adds* the submodule. However, 'submodule_touches_in_range' *succeeds* because 'combine-diff.c::diff_tree_combined' (see the backtrace below) returns early since this commit is the root commit and has no parents. #0 diff_tree_combined at combine-diff.c:1494 #1 0x0000000100150cbe in diff_tree_combined_merge at combine-diff.c:1649 #2 0x00000001002c7147 in collect_changed_submodules at submodule.c:869 #3 0x00000001002c7d6f in submodule_touches_in_range at submodule.c:1268 #4 0x00000001000ad58b in cmd_pull at builtin/pull.c:1040 In light of all this, add a note in t5572 documenting this peculiar test. [1] https://lore.kernel.org/git/[email protected]/t/#u Signed-off-by: Philippe Blain <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4f66d79 commit ba58ddd

File tree

1 file changed

+15
-0
lines changed

1 file changed

+15
-0
lines changed

t/t5572-pull-submodule.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,21 @@ test_expect_success 'pull rebase recursing fails with conflicts' '
136136
test_i18ngrep "locally recorded submodule modifications" err
137137
'
138138

139+
# NOTE:
140+
#
141+
# This test is particular because there is only a single commit in the upstream superproject
142+
# 'parent' (which adds the submodule 'a-submodule'). The clone of the superproject
143+
# ('child') hard-resets its branch to a new root commit with the same tree as the one
144+
# from the upstream superproject, so that its branch has no merge-base with its
145+
# remote-tracking counterpart, and then calls 'git pull --recurse-submodules --rebase'.
146+
# The result is that the local branch is reset to the remote-tracking branch (as it was
147+
# originally before the hard-reset).
148+
149+
# The only commit in the range generated by 'submodule.c::submodule_touches_in_range' and
150+
# passed to 'submodule.c::collect_changed_submodules' is the new (regenerated) initial commit,
151+
# which adds the submodule.
152+
# However, 'submodule_touches_in_range' does not error (even though this commit adds the submodule)
153+
# because 'combine-diff.c::diff_tree_combined' returns early, as the initial commit has no parents.
139154
test_expect_success 'branch has no merge base with remote-tracking counterpart' '
140155
rm -rf parent child &&
141156

0 commit comments

Comments
 (0)