git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	Stefan Beller <stefanbeller@gmail.com>,
	Brandon Williams <bwilliamseng@gmail.com>,
	Brice Goglin <bgoglin@free.fr>,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Philippe Blain <levraiphilippeblain@gmail.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: <85c5766556e53f780b9a3ac677a0c758c51baae2.1605314085.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.789.git.1605314085.gitgitgadget@gmail.com>

From: Philippe Blain <levraiphilippeblain@gmail.com>

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 [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 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.

[1] https://lore.kernel.org/git/20180524204729.19896-1-jonathantanmy@google.com/t/#u

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 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


  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 \
    --in-reply-to=85c5766556e53f780b9a3ac677a0c758c51baae2.1605314085.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=bgoglin@free.fr \
    --cc=bwilliamseng@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=stefanbeller@gmail.com \
    --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).