From: "Philippe Blain via GitGitGadget" <firstname.lastname@example.org> To: email@example.com Cc: Jonathan Tan <firstname.lastname@example.org>, Stefan Beller <email@example.com>, Brandon Williams <firstname.lastname@example.org>, Brice Goglin <email@example.com>, Philippe Blain <firstname.lastname@example.org>, Philippe Blain <email@example.com> Subject: [PATCH 2/4] t5572: add notes on a peculiar test Date: Sat, 14 Nov 2020 00:34:43 +0000 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> From: Philippe Blain <firstname.lastname@example.org> Test 5572.63 ("branch has no merge base with remote-tracking counterpart") was introduced in 4d36f88be7 (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 4d36f88be7 does not describe in which real-life situation this bug was encountered. The brief discussion on the mailing list  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 4d36f88be7 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.  https://email@example.com/t/#u Signed-off-by: Philippe Blain <firstname.lastname@example.org> --- t/t5572-pull-submodule.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh index 1d75e3b12b..7f658dba6d 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -136,6 +136,21 @@ test_expect_success 'pull rebase recursing fails with conflicts' ' test_i18ngrep "locally recorded submodule modifications" err ' +# NOTE: +# +# This test is particular because there is only a single commit in the upstream superproject +# 'parent' (which adds the submodule 'a-submodule'). The clone of the superproject +# ('child') hard-resets its branch to a new root commit with the same tree as the one +# from the upstream superproject, so that its branch has no merge-base with its +# remote-tracking counterpart, and then calls 'git pull --recurse-submodules --rebase'. +# The result is that the local branch is reset to the remote-tracking branch (as it was +# originally before the hard-reset). + +# The only commit in the range generated by 'submodule.c::submodule_touches_in_range' and +# passed to 'submodule.c::collect_changed_submodules' is the new (regenerated) initial commit, +# which adds the submodule. +# However, 'submodule_touches_in_range' does not error (even though this commit adds the submodule) +# because 'combine-diff.c::diff_tree_combined' returns early, as the initial commit has no parents. test_expect_success 'branch has no merge base with remote-tracking counterpart' ' rm -rf parent child && -- gitgitgadget
next prev parent reply other threads:[~2020-11-14 0:35 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-14 0:34 [PATCH 0/4] Fix 'pull --rebase --recurse-submodules' when local and upstream branches have no fork-point Philippe Blain via GitGitGadget 2020-11-14 0:34 ` [PATCH 1/4] pull --rebase: compute rebase arguments in separate function Philippe Blain via GitGitGadget 2020-11-14 0:34 ` Philippe Blain via GitGitGadget [this message] 2020-11-14 0:34 ` [PATCH 3/4] t5572: describe '--rebase' tests a little more Philippe Blain via GitGitGadget 2020-11-14 0:34 ` [PATCH 4/4] pull: check for local submodule modifications with the right range Philippe Blain via GitGitGadget
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 2/4] t5572: add notes on a peculiar test' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).