[v3,00/10] fetch --recurse-submodules: fetch unpopulated submodules

Message

Glen Choo Feb. 24, 2022, 10:08 a.m. UTC

Original cover letter: https://lore.kernel.org/git/[e-mail]

This series is based on gc/branch-recurse-submodules.

Thanks for the feedback, and sorry for the delay - I've made some
substantial changes in response to the feedback, as well as unearthing
some surprising bugs.

Patches 2 and 9 have some extra discussion in their --- descriptions.
I'd appreciate feedback on those (especially the 'future work' described
in patch 9).

= Patch organization

- Patches 1-3 are quality-of-life improvements to the test suite that
  make it easier to write the tests in patch 9.
- Patches 4-6 are preparation for "git fetch" to read .gitmodules from
  the superproject commit in patch 7.
- Patches 7-8 refactor out the logic of "finding which submodules to
  fetch" and "fetching the submodules", making it easier to tell "git
  fetch" to fetch unpopulated submodules.
- Patch 9 teaches "git fetch" to fetch changed, unpopulated submodules
  in addition to populated submodules.
- Patch 10 is an optional bugfix + cleanup of the "git fetch" code that
  removes the last caller of the deprecated "add_submodule_odb()".

= Changes 

== Since v2
- Numerous small fixes to the code and commit message (thanks to all who
  helped spot these :))
- In patch 2, use test_cmp + sed to assert on test output, effectively
  reverting the "use grep" approach of v1-2 (see patch 2's description).
- New patch 3: introduce a test helper that creates the expected
  superproject commit (instead of copy-pasting the code over and over).
  - I did not get rid of "git fetch" inside the test helper (as Jonathan
    suggested) though, because that requires a bigger change in the test
    setup, and I think the test helper makes the test straightforward
    enough.
- New patch 8: refactor some shared logic out into fetch_task_create().
  This reduces code duplication between the get_fetch_task_from_*
  functions.
- In patch 9, add additional tests for 'submodules with the same name'.
- In patch 9, handle a bug where a submodule that is unpopulated by "git
  rm" still has "core.worktree" set and cannot be fetched (see patch 9's
  description).
- Remove the "git fetch --update-shallow" patch (I'll try to send it
  separately).

== Since v1
- Numerous style fixes suggested by Jonathan (thanks!)
- In patch 3, don't prematurely read submodules from the superproject
  commit (see:
  <[e-mail]>).
- In patch 7, stop using "git checkout" and "! grep" in tests.
- In patch 7, stop doing the "find changed submodules" rev walk
  unconditionally. Instead, continue to check for .gitmodules, but also
  check for submodules in $GIT_DIR/modules.
  - I'm not entirely happy with the helper function name, see "---" for
    details.
- Move "git fetch --update-shallow" bugfix to patch 8.
  - Because the "find changed submodules" rev walk is no longer
    unconditional, this fix is no longer needed for tests to pass.
- Rename fetch_populated_submodules() to fetch_submodules().

Glen Choo (10):
  t5526: introduce test helper to assert on fetches
  t5526: stop asserting on stderr literally
  t5526: create superproject commits with test helper
  submodule: make static functions read submodules from commits
  submodule: inline submodule_commits() into caller
  submodule: store new submodule commits oid_array in a struct
  submodule: extract get_fetch_task()
  submodule: move logic into fetch_task_create()
  fetch: fetch unpopulated, changed submodules
  submodule: fix latent check_has_commit() bug

 Documentation/fetch-options.txt |  26 +-
 Documentation/git-fetch.txt     |  10 +-
 builtin/fetch.c                 |  14 +-
 submodule.c                     | 395 ++++++++++++++---------
 submodule.h                     |  21 +-
 t/t5526-fetch-submodules.sh     | 533 ++++++++++++++++++++++++--------
 6 files changed, 687 insertions(+), 312 deletions(-)

Range-diff against v2:
 1:  a159cdaabb !  1:  b6d34b0f5c t5526: introduce test helper to assert on fetches
    @@ Metadata
      ## Commit message ##
         t5526: introduce test helper to assert on fetches
     
    -    A future commit will change the stderr of "git fetch
    -    --recurse-submodules" and add new tests to t/t5526-fetch-submodules.sh.
    -    This poses two challenges:
    +    Tests in t/t5526-fetch-submodules.sh are unnecessarily noisy:
    +
    +    * The tests have extra logic in order to reproduce the expected stderr
    +      literally, but not all of these details (e.g. the head of the
    +      remote-tracking branch before the fetch) are relevant to the test.
     
    -    * The tests use test_cmp to assert on the stderr, which will fail on the
    -      future test because the stderr changes slightly, even though it still
    -      contains the information we expect.
         * The expect.err file is constructed by the add_upstream_commit() helper
           as input into test_cmp, but most tests fetch a different combination
           of repos from expect.err. This results in noisy tests that modify
    @@ Commit message
         helper to t/t5526-fetch-submodules.sh that asserts on the output of "git
         fetch --recurse-submodules" and handles the ordering of expect.err.
     
    -    As a result, the tests no longer construct expect.err manually. test_cmp
    -    is still invoked by verify_fetch_result(), but that will be replaced in
    -    a later commit.
    +    As a result, the tests no longer construct expect.err manually. Tests
    +    still consider the old head of the remote-tracking branch ("$head1"),
    +    but that will be fixed in a later commit.
     
         Signed-off-by: Glen Choo <[e-mail]>
     
 2:  48894c6c43 !  2:  0b85fa35c2 t5526: use grep to assert on fetches
    @@ Metadata
     Author: Glen Choo <[e-mail]>
     
      ## Commit message ##
    -    t5526: use grep to assert on fetches
    +    t5526: stop asserting on stderr literally
     
    -    In a previous commit, we replaced test_cmp invocations with
    -    verify_fetch_result(). Finish the process of removing test_cmp by using
    -    grep in verify_fetch_result() instead.
    +    In the previous commit message, we noted that not all of the "git fetch"
    +    stderr is relevant to the tests. Most of the test setup lines are
    +    dedicated to these details of the stderr:
     
    -    This makes the tests less sensitive to changes because, instead of
    -    checking the whole stderr, we only grep for the lines of the form
    +    1. which repos (super/sub/deep) are involved in the fetch
    +    2. the head of the remote-tracking branch before the fetch (i.e. $head1)
    +    3. the head of the remote-tracking branch after the fetch (i.e. $head2)
     
    -    * "<old-head>..<new-head>\s+branch\s+-> origin/branch"
    -    * "Fetching submodule <submodule-path>" (if fetching a submodule)
    +    1. and 3. are relevant because they tell us that the expected commit is
    +    fetched by the expected repo, but 2. is completely irrelevant.
     
    -    when we expect the repo to have fetched. If we expect the repo to not
    -    have fetched, grep to make sure the lines are absent. Also, simplify the
    -    assertions by using grep patterns that match only the relevant pieces of
    -    information, e.g. <old-head> is irrelevant because we only want to know
    -    if the fetch was performed, so we don't need to know where the branch
    -    was before the fetch.
    +    Stop asserting on $head1 by replacing it with a dummy value in the
    +    actual and expected output. Do this by introducing test
    +    helpers (check_*()) that make it easier to construct the expected
    +    output, and use sed to munge the actual output.
     
         Signed-off-by: Glen Choo <[e-mail]>
     
      ## t/t5526-fetch-submodules.sh ##
    +@@ t/t5526-fetch-submodules.sh: export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
    + 
    + pwd=$(pwd)
    + 
    ++check_sub() {
    ++	NEW_HEAD=$1 &&
    ++	cat <<-EOF >$pwd/expect.err.sub
    ++	Fetching submodule submodule
    ++	From $pwd/submodule
    ++	   OLD_HEAD..$NEW_HEAD  sub        -> origin/sub
    ++	EOF
    ++}
    ++
    ++check_deep() {
    ++	NEW_HEAD=$1 &&
    ++	cat <<-EOF >$pwd/expect.err.deep
    ++	Fetching submodule submodule/subdir/deepsubmodule
    ++	From $pwd/deepsubmodule
    ++	   OLD_HEAD..$NEW_HEAD  deep       -> origin/deep
    ++	EOF
    ++}
    ++
    ++check_super() {
    ++	NEW_HEAD=$1 &&
    ++	cat <<-EOF >$pwd/expect.err.super
    ++	From $pwd/.
    ++	   OLD_HEAD..$NEW_HEAD  super      -> origin/super
    ++	EOF
    ++}
    ++
    + # For each submodule in the test setup, this creates a commit and writes
    + # a file that contains the expected err if that new commit were fetched.
    + # These output files get concatenated in the right order by
     @@ t/t5526-fetch-submodules.sh: pwd=$(pwd)
      add_upstream_commit() {
      	(
    @@ t/t5526-fetch-submodules.sh: pwd=$(pwd)
     -		echo "Fetching submodule submodule" > ../expect.err.sub &&
     -		echo "From $pwd/submodule" >> ../expect.err.sub &&
     -		echo "   $head1..$head2  sub        -> origin/sub" >> ../expect.err.sub
    -+		git rev-parse --short HEAD >../subhead
    ++		new_head=$(git rev-parse --short HEAD) &&
    ++		check_sub $new_head
      	) &&
      	(
      		cd deepsubmodule &&
    @@ t/t5526-fetch-submodules.sh: pwd=$(pwd)
     -		echo "Fetching submodule submodule/subdir/deepsubmodule" > ../expect.err.deep
     -		echo "From $pwd/deepsubmodule" >> ../expect.err.deep &&
     -		echo "   $head1..$head2  deep       -> origin/deep" >> ../expect.err.deep
    -+		git rev-parse --short HEAD >../deephead
    ++		new_head=$(git rev-parse --short HEAD) &&
    ++		check_deep $new_head
      	)
      }
      
    - # Verifies that the expected repositories were fetched. This is done by
    --# concatenating the files expect.err.[super|sub|deep] in the correct
    --# order and comparing it to the actual stderr.
    -+# checking that the branches of [super|sub|deep] were updated to
    -+# [super|sub|deep]head if the corresponding file exists.
    - #
    --# If a repo should not be fetched in the test, its corresponding
    --# expect.err file should be rm-ed.
    -+# If the [super|sub|deep] head file does not exist, this verifies that
    -+# the corresponding repo was not fetched. Thus, if a repo should not be
    -+# fetched in the test, its corresponding head file should be
    -+# rm-ed.
    - verify_fetch_result() {
    - 	ACTUAL_ERR=$1 &&
    --	rm -f expect.err.combined &&
    --	if [ -f expect.err.super ]; then
    --		cat expect.err.super >>expect.err.combined
    -+	# Each grep pattern is guaranteed to match the correct repo
    -+	# because each repo uses a different name for their branch i.e.
    -+	# "super", "sub" and "deep".
    -+	if [ -f superhead ]; then
    -+		grep -E "\.\.$(cat superhead)\s+super\s+-> origin/super" $ACTUAL_ERR
    -+	else
    -+		! grep "super" $ACTUAL_ERR
    - 	fi &&
    --	if [ -f expect.err.sub ]; then
    --		cat expect.err.sub >>expect.err.combined
    -+	if [ -f subhead ]; then
    -+		grep "Fetching submodule submodule" $ACTUAL_ERR &&
    -+		grep -E "\.\.$(cat subhead)\s+sub\s+-> origin/sub" $ACTUAL_ERR
    -+	else
    -+		! grep "Fetching submodule submodule" $ACTUAL_ERR
    +@@ t/t5526-fetch-submodules.sh: verify_fetch_result() {
    + 	if [ -f expect.err.deep ]; then
    + 		cat expect.err.deep >>expect.err.combined
      	fi &&
    --	if [ -f expect.err.deep ]; then
    --		cat expect.err.deep >>expect.err.combined
    --	fi &&
     -	test_cmp expect.err.combined $ACTUAL_ERR
    -+	if [ -f deephead ]; then
    -+		grep "Fetching submodule submodule/subdir/deepsubmodule" $ACTUAL_ERR &&
    -+		grep -E "\.\.$(cat deephead)\s+deep\s+-> origin/deep" $ACTUAL_ERR
    -+	else
    -+		! grep "Fetching submodule submodule/subdir/deepsubmodule" $ACTUAL_ERR
    -+	fi
    ++	sed -E 's/[0-9a-f]+\.\./OLD_HEAD\.\./' $ACTUAL_ERR >actual.err.cmp &&
    ++	test_cmp expect.err.combined actual.err.cmp
      }
      
      test_expect_success setup '
    @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion doesn't happen when
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >>expect.err.super &&
    --	rm expect.err.deep &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm deephead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.deep &&
      	(
      		cd downstream &&
    - 		git fetch >../actual.out 2>../actual.err
     @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion stops when no new submodule commits are fetched"
      
      test_expect_success "Recursion doesn't happen when new superproject commits don't change any submodules" '
    @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion stops when no new su
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >> expect.err.super &&
    --	rm expect.err.sub &&
    --	rm expect.err.deep &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm subhead &&
    -+	rm deephead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.sub &&
    + 	rm expect.err.deep &&
      	(
    - 		cd downstream &&
    - 		git fetch >../actual.out 2>../actual.err
     @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion picks up config in submodule" '
      		)
      	) &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion picks up config in s
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >> expect.err.super &&
    -+	git rev-parse --short HEAD >superhead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
      	(
      		cd downstream &&
      		git fetch >../actual.out 2>../actual.err &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion picks up all submodu
     -		echo "Fetching submodule submodule" > ../expect.err.sub &&
     -		echo "From $pwd/submodule" >> ../expect.err.sub &&
     -		echo "   $head1..$head2  sub        -> origin/sub" >> ../expect.err.sub
    -+		git rev-parse --short HEAD >../subhead
    ++		new_head=$(git rev-parse --short HEAD) &&
    ++		check_sub $new_head
      	) &&
     -	head1=$(git rev-parse --short HEAD) &&
      	git add submodule &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "Recursion picks up all submodu
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >> expect.err.super &&
    -+	git rev-parse --short HEAD >superhead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
      	(
      		cd downstream &&
      		git fetch >../actual.out 2>../actual.err
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     -		echo Fetching submodule submodule > ../expect.err.sub &&
     -		echo "From $pwd/submodule" >> ../expect.err.sub &&
     -		echo "   $head1..$head2  sub        -> origin/sub" >> ../expect.err.sub
    -+		git rev-parse --short HEAD >../subhead
    ++		new_head=$(git rev-parse --short HEAD) &&
    ++		check_sub $new_head
      	) &&
      	(
      		cd downstream &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >>expect.err.super &&
    -+	git rev-parse --short HEAD >superhead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
      	(
      		cd downstream &&
      		git config fetch.recurseSubmodules false &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >> expect.err.super &&
    --	rm expect.err.sub &&
    --	rm expect.err.deep &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm subhead &&
    -+	rm deephead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.sub &&
    + 	rm expect.err.deep &&
      	(
    - 		cd downstream &&
    - 		git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err
     @@ t/t5526-fetch-submodules.sh: test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config
      	) &&
      	add_upstream_commit &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'fetch.recurseSubmodules=on-de
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >>expect.err.super &&
    --	rm expect.err.deep &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm deephead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.deep &&
      	(
      		cd downstream &&
    - 		git config fetch.recurseSubmodules on-demand &&
     @@ t/t5526-fetch-submodules.sh: test_expect_success "'submodule.<sub>.fetchRecurseSubmodules=on-demand' override
      	) &&
      	add_upstream_commit &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'submodule.<sub>.fetchRecurseS
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >>expect.err.super &&
    --	rm expect.err.deep &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm deephead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.deep &&
      	(
      		cd downstream &&
    - 		git config submodule.submodule.fetchRecurseSubmodules on-demand &&
     @@ t/t5526-fetch-submodules.sh: test_expect_success "don't fetch submodule when newly recorded commits are alrea
      		cd submodule &&
      		git checkout -q HEAD^^
    @@ t/t5526-fetch-submodules.sh: test_expect_success "don't fetch submodule when new
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." > expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >> expect.err.super &&
    --	rm expect.err.sub &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm subhead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.sub &&
      	# This file does not exist, but rm -f for readability
    --	rm -f expect.err.deep &&
    -+	rm -f deephead &&
    - 	(
    - 		cd downstream &&
    - 		git fetch >../actual.out 2>../actual.err
    + 	rm -f expect.err.deep &&
     @@ t/t5526-fetch-submodules.sh: test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git
      		git fetch --recurse-submodules
      	) &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'fetch.recurseSubmodules=on-de
     -	head2=$(git rev-parse --short HEAD) &&
     -	echo "From $pwd/." >expect.err.super &&
     -	echo "   $head1..$head2  super      -> origin/super" >>expect.err.super &&
    --	rm expect.err.deep &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	rm deephead &&
    ++	new_head=$(git rev-parse --short HEAD) &&
    ++	check_super $new_head &&
    + 	rm expect.err.deep &&
      	(
      		cd downstream &&
    - 		rm .gitmodules &&
 -:  ---------- >  3:  bb8ef6094a t5526: create superproject commits with test helper
 3:  6cf5e76d62 !  4:  e83a1713c4 submodule: make static functions read submodules from commits
    @@ Commit message
         Submodules will be read from commits when we fetch unpopulated
         submodules.
     
    -    The changed function signatures follow repo_submodule_init()'s argument
    -    order, i.e. "path" then "treeish_name". Where needed, reorder the
    -    arguments of functions that already take "path" and "treeish_name" to be
    -    consistent with this convention.
    -
         Signed-off-by: Glen Choo <[e-mail]>
     
      ## submodule.c ##
    @@ submodule.c: static int check_has_commit(const struct object_id *oid, void *data
      				 struct oid_array *commits)
      {
     -	struct has_commit_data has_commit = { r, 1, path };
    -+	struct has_commit_data has_commit = { r, 1, path, super_oid };
    ++	struct has_commit_data has_commit = {
    ++		.repo = r,
    ++		.result = 1,
    ++		.path = path,
    ++		.super_oid = super_oid
    ++	};
      
      	/*
      	 * Perform a cheap, but incorrect check for the existence of 'commits'.
 4:  07fd4ff0a9 =  5:  e27d402b9a submodule: inline submodule_commits() into caller
 5:  f049cb231b !  6:  1c7c8218b8 submodule: store new submodule commits oid_array in a struct
    @@ submodule.c: static void collect_changed_submodules(struct repository *r,
     +static void free_submodules_data(struct string_list *submodules)
      {
      	struct string_list_item *item;
    --	for_each_string_list_item(item, submodules)
    + 	for_each_string_list_item(item, submodules)
     -		oid_array_clear((struct oid_array *) item->util);
    -+	for_each_string_list_item(item, submodules) {
     +		changed_submodule_data_clear(item->util);
    -+	}
    ++
      	string_list_clear(submodules, 1);
      }
      
 6:  814073eecc !  7:  80cf317722 submodule: extract get_fetch_task()
    @@ Commit message
         Signed-off-by: Glen Choo <[e-mail]>
     
      ## submodule.c ##
    +@@ submodule.c: struct fetch_task {
    + 	struct repository *repo;
    + 	const struct submodule *sub;
    + 	unsigned free_sub : 1; /* Do we need to free the submodule? */
    ++	const char *default_argv;
    + 
    + 	struct oid_array *commits; /* Ensure these commits are fetched */
    + };
     @@ submodule.c: static struct repository *get_submodule_repo_for(struct repository *r,
      	return ret;
      }
    @@ submodule.c: static struct repository *get_submodule_repo_for(struct repository
     -static int get_next_submodule(struct child_process *cp,
     -			      struct strbuf *err, void *data, void **task_cb)
     +static struct fetch_task *
    -+get_fetch_task(struct submodule_parallel_fetch *spf,
    -+	       const char **default_argv, struct strbuf *err)
    ++get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
      {
     -	struct submodule_parallel_fetch *spf = data;
     -
    @@ submodule.c: static int get_next_submodule(struct child_process *cp,
      					task->sub->name))
      				continue;
     -			default_argv = "on-demand";
    -+			*default_argv = "on-demand";
    ++			task->default_argv = "on-demand";
      			break;
      		case RECURSE_SUBMODULES_ON:
     -			default_argv = "yes";
    -+			*default_argv = "yes";
    ++			task->default_argv = "yes";
      			break;
      		case RECURSE_SUBMODULES_OFF:
      			continue;
    @@ submodule.c: static int get_next_submodule(struct child_process *cp,
     +			      void *data, void **task_cb)
     +{
     +	struct submodule_parallel_fetch *spf = data;
    -+	const char *default_argv = NULL;
    -+	struct fetch_task *task = get_fetch_task(spf, &default_argv, err);
    ++	struct fetch_task *task = get_fetch_task(spf, err);
     +
     +	if (task) {
     +		struct strbuf submodule_prefix = STRBUF_INIT;
    @@ submodule.c: static int get_next_submodule(struct child_process *cp,
     +		cp->git_cmd = 1;
     +		strvec_init(&cp->args);
     +		strvec_pushv(&cp->args, spf->args.v);
    -+		strvec_push(&cp->args, default_argv);
    ++		strvec_push(&cp->args, task->default_argv);
     +		strvec_push(&cp->args, "--submodule-prefix");
     +
     +		strbuf_addf(&submodule_prefix, "%s%s/",
 -:  ---------- >  8:  bf9cfa7054 submodule: move logic into fetch_task_create()
 7:  10fd5bf921 !  9:  c7c2ff71b6 fetch: fetch unpopulated, changed submodules
    @@ Commit message
         commands like "git checkout --recurse-submodules" might fail.
     
         Teach "git fetch" to fetch cloned, changed submodules regardless of
    -    whether they are populated (this is in addition to the current behavior
    -    of fetching populated submodules).
    +    whether they are populated. This is in addition to the current behavior
    +    of fetching populated submodules (which is always attempted regardless
    +    of what was fetched in the superproject, or even if nothing was fetched
    +    in the superproject).
     
    -    Since a submodule may be encountered multiple times (via the list of
    -    populated submodules or via the list of changed submodules), maintain a
    -    list of seen submodules to avoid fetching a submodule more than once.
    +    A submodule may be encountered multiple times (via the list of
    +    populated submodules or via the list of changed submodules). When this
    +    happens, "git fetch" only reads the 'populated copy' and ignores the
    +    'changed copy'. Amend the verify_fetch_result() test helper so that we
    +    can assert on which 'copy' is being read.
     
         Signed-off-by: Glen Choo <[e-mail]>
     
    @@ submodule.c: void check_for_new_submodule_commits(struct object_id *oid)
      }
      
     +/*
    -+ * Returns 1 if the repo has absorbed submodule gitdirs, and 0
    -+ * otherwise. Like submodule_name_to_gitdir(), this checks
    ++ * Returns 1 if there is at least one submodule gitdir in
    ++ * $GIT_DIR/modules and 0 otherwise. This follows
    ++ * submodule_name_to_gitdir(), which looks for submodules in
     + * $GIT_DIR/modules, not $GIT_COMMON_DIR.
    ++ *
    ++ * A submodule can be moved to $GIT_DIR/modules manually by running "git
    ++ * submodule absorbgitdirs", or it may be initialized there by "git
    ++ * submodule update".
     + */
     +static int repo_has_absorbed_submodules(struct repository *r)
     +{
    @@ submodule.c: struct submodule_parallel_fetch {
      	.submodules_with_errors = STRBUF_INIT, \
      }
      
    -@@ submodule.c: static struct repository *get_submodule_repo_for(struct repository *r,
    +@@ submodule.c: struct fetch_task {
    + 	const struct submodule *sub;
    + 	unsigned free_sub : 1; /* Do we need to free the submodule? */
    + 	const char *default_argv;
    ++	struct strvec git_args;
    + 
    + 	struct oid_array *commits; /* Ensure these commits are fetched */
    + };
    +@@ submodule.c: static void fetch_task_release(struct fetch_task *p)
    + 	if (p->repo)
    + 		repo_clear(p->repo);
    + 	FREE_AND_NULL(p->repo);
    ++
    ++	strvec_clear(&p->git_args);
    + }
    + 
    + static struct repository *get_submodule_repo_for(struct repository *r,
    +@@ submodule.c: static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
    + 		task->free_sub = 1;
    + 	}
    + 
    ++	if (string_list_lookup(&spf->seen_submodule_names, task->sub->name))
    ++		goto cleanup;
    ++
    + 	switch (get_fetch_recurse_config(task->sub, spf))
    + 	{
    + 	default:
    +@@ submodule.c: static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
      }
      
      static struct fetch_task *
    --get_fetch_task(struct submodule_parallel_fetch *spf,
    --	       const char **default_argv, struct strbuf *err)
    +-get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
     +get_fetch_task_from_index(struct submodule_parallel_fetch *spf,
    -+			  const char **default_argv, struct strbuf *err)
    ++			  struct strbuf *err)
      {
     -	for (; spf->count < spf->r->index->cache_nr; spf->count++) {
     -		const struct cache_entry *ce = spf->r->index->cache[spf->count];
    @@ submodule.c: static struct repository *get_submodule_repo_for(struct repository
      		struct fetch_task *task;
      
      		if (!S_ISGITLINK(ce->ce_mode))
    -@@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
    - 		if (!task)
    - 			continue;
    - 
    -+		/*
    -+		 * We might have already considered this submodule
    -+		 * because we saw it when iterating the changed
    -+		 * submodule names.
    -+		 */
    -+		if (string_list_lookup(&spf->seen_submodule_names,
    -+				       task->sub->name))
    -+			continue;
    -+
    - 		switch (get_fetch_recurse_config(task->sub, spf))
    - 		{
    - 		default:
    -@@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
    +@@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
      				strbuf_addf(err, _("Fetching submodule %s%s\n"),
      					    spf->prefix, ce->name);
      
    @@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
      			return task;
      		} else {
      			struct strbuf empty_submodule_path = STRBUF_INIT;
    -@@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
    +@@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
      	return NULL;
      }
      
     +static struct fetch_task *
     +get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,
    -+			    const char **default_argv, struct strbuf *err)
    ++			    struct strbuf *err)
     +{
     +	for (; spf->changed_count < spf->changed_submodule_names.nr;
     +	     spf->changed_count++) {
    @@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
     +		struct changed_submodule_data *cs_data = item.util;
     +		struct fetch_task *task;
     +
    -+		/*
    -+		 * We might have already considered this submodule
    -+		 * because we saw it in the index.
    -+		 */
    -+		if (string_list_lookup(&spf->seen_submodule_names, item.string))
    ++		if (!is_tree_submodule_active(spf->r, cs_data->super_oid,cs_data->path))
     +			continue;
     +
    -+		task = fetch_task_create(spf->r, cs_data->path,
    ++		task = fetch_task_create(spf, cs_data->path,
     +					 cs_data->super_oid);
     +		if (!task)
     +			continue;
     +
    -+		switch (get_fetch_recurse_config(task->sub, spf)) {
    -+		default:
    -+		case RECURSE_SUBMODULES_DEFAULT:
    -+		case RECURSE_SUBMODULES_ON_DEMAND:
    -+			*default_argv = "on-demand";
    -+			break;
    -+		case RECURSE_SUBMODULES_ON:
    -+			*default_argv = "yes";
    -+			break;
    -+		case RECURSE_SUBMODULES_OFF:
    -+			continue;
    -+		}
    -+
    -+		task->repo = get_submodule_repo_for(spf->r, task->sub->path,
    -+						    cs_data->super_oid);
     +		if (!task->repo) {
    ++			strbuf_addf(err, _("Could not access submodule '%s' at commit %s\n"),
    ++				    cs_data->path,
    ++				    find_unique_abbrev(cs_data->super_oid, DEFAULT_ABBREV));
    ++
     +			fetch_task_release(task);
     +			free(task);
    -+
    -+			strbuf_addf(err, _("Could not access submodule '%s'\n"),
    -+				    cs_data->path);
     +			continue;
     +		}
    -+		if (!is_tree_submodule_active(spf->r, cs_data->super_oid,
    -+					      task->sub->path))
    -+			continue;
     +
     +		if (!spf->quiet)
     +			strbuf_addf(err,
    @@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
     +				    spf->prefix, task->sub->path,
     +				    find_unique_abbrev(cs_data->super_oid,
     +						       DEFAULT_ABBREV));
    ++
     +		spf->changed_count++;
    ++		/*
    ++		 * NEEDSWORK: A submodule unpopulated by "git rm" will
    ++		 * have core.worktree set, but the actual core.worktree
    ++		 * directory won't exist, causing the child process to
    ++		 * fail. Forcibly set --work-tree until we get smarter
    ++		 * handling for core.worktree in unpopulated submodules.
    ++		 */
    ++		strvec_push(&task->git_args, "--work-tree=.");
     +		return task;
     +	}
     +	return NULL;
    @@ submodule.c: get_fetch_task(struct submodule_parallel_fetch *spf,
      			      void *data, void **task_cb)
      {
      	struct submodule_parallel_fetch *spf = data;
    - 	const char *default_argv = NULL;
    --	struct fetch_task *task = get_fetch_task(spf, &default_argv, err);
    +-	struct fetch_task *task = get_fetch_task(spf, err);
     +	struct fetch_task *task =
    -+		get_fetch_task_from_index(spf, &default_argv, err);
    ++		get_fetch_task_from_index(spf, err);
     +	if (!task)
    -+		task = get_fetch_task_from_changed(spf, &default_argv, err);
    ++		task = get_fetch_task_from_changed(spf, err);
      
      	if (task) {
      		struct strbuf submodule_prefix = STRBUF_INIT;
    +@@ submodule.c: static int get_next_submodule(struct child_process *cp, struct strbuf *err,
    + 		prepare_submodule_repo_env_in_gitdir(&cp->env_array);
    + 		cp->git_cmd = 1;
    + 		strvec_init(&cp->args);
    ++		if (task->git_args.nr)
    ++			strvec_pushv(&cp->args, task->git_args.v);
    + 		strvec_pushv(&cp->args, spf->args.v);
    + 		strvec_push(&cp->args, task->default_argv);
    + 		strvec_push(&cp->args, "--submodule-prefix");
     @@ submodule.c: static int get_next_submodule(struct child_process *cp, struct strbuf *err,
      		*task_cb = task;
      
    @@ submodule.h: int should_update_submodules(void);
      
     
      ## t/t5526-fetch-submodules.sh ##
    +@@ t/t5526-fetch-submodules.sh: pwd=$(pwd)
    + 
    + check_sub() {
    + 	NEW_HEAD=$1 &&
    ++	SUPER_HEAD=$2 &&
    + 	cat <<-EOF >$pwd/expect.err.sub
    +-	Fetching submodule submodule
    ++	Fetching submodule submodule${SUPER_HEAD:+ at commit $SUPER_HEAD}
    + 	From $pwd/submodule
    + 	   OLD_HEAD..$NEW_HEAD  sub        -> origin/sub
    + 	EOF
    +@@ t/t5526-fetch-submodules.sh: check_sub() {
    + 
    + check_deep() {
    + 	NEW_HEAD=$1 &&
    ++	SUB_HEAD=$2 &&
    + 	cat <<-EOF >$pwd/expect.err.deep
    +-	Fetching submodule submodule/subdir/deepsubmodule
    ++	Fetching submodule submodule/subdir/deepsubmodule${SUB_HEAD:+ at commit $SUB_HEAD}
    + 	From $pwd/deepsubmodule
    + 	   OLD_HEAD..$NEW_HEAD  deep       -> origin/deep
    + 	EOF
     @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess
      	verify_fetch_result actual.err
      '
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     +	(
     +		cd downstream &&
     +		git checkout --recurse-submodules -b no-submodules &&
    -+		rm .gitmodules &&
    ++		git rm .gitmodules &&
     +		git rm submodule &&
    -+		git add .gitmodules &&
     +		git commit -m "no submodules" &&
     +		git checkout --recurse-submodules super
     +	)
     +'
     +
     +test_expect_success "'--recurse-submodules=on-demand' should fetch submodule commits if the submodule is changed but the index has no submodules" '
    -+	git -C downstream fetch --recurse-submodules &&
    -+	# Create new superproject commit with updated submodules
    -+	add_upstream_commit &&
    -+	(
    -+		cd submodule &&
    -+		(
    -+			cd subdir/deepsubmodule &&
    -+			git fetch &&
    -+			git checkout -q FETCH_HEAD
    -+		) &&
    -+		git add subdir/deepsubmodule &&
    -+		git commit -m "new deep submodule"
    -+	) &&
    -+	git add submodule &&
    -+	git commit -m "new submodule" &&
    -+
    ++	add_submodule_commits &&
    ++	add_superproject_commits &&
     +	# Fetch the new superproject commit
     +	(
     +		cd downstream &&
     +		git switch --recurse-submodules no-submodules &&
     +		git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err
     +	) &&
    -+	test_must_be_empty actual.out &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	git -C submodule rev-parse --short HEAD >subhead &&
    -+	git -C deepsubmodule rev-parse --short HEAD >deephead &&
    -+	verify_fetch_result actual.err &&
    ++	super_head=$(git rev-parse --short HEAD) &&
    ++	sub_head=$(git -C submodule rev-parse --short HEAD) &&
    ++	deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) &&
    ++
    ++	# assert that these are fetched from commits, not the index
    ++	check_sub $sub_head $super_head &&
    ++	check_deep $deep_head $sub_head &&
     +
    -+	# Assert that the fetch happened at the non-HEAD commits
    -+	grep "Fetching submodule submodule at commit $superhead" actual.err &&
    -+	grep "Fetching submodule submodule/subdir/deepsubmodule at commit $subhead" actual.err
    ++	test_must_be_empty actual.out &&
    ++	verify_fetch_result actual.err
     +'
     +
     +test_expect_success "'--recurse-submodules' should fetch submodule commits if the submodule is changed but the index has no submodules" '
    -+	# Fetch any leftover commits from other tests.
    -+	git -C downstream fetch --recurse-submodules &&
    -+	# Create new superproject commit with updated submodules
    -+	add_upstream_commit &&
    -+	(
    -+		cd submodule &&
    -+		(
    -+			cd subdir/deepsubmodule &&
    -+			git fetch &&
    -+			git checkout -q FETCH_HEAD
    -+		) &&
    -+		git add subdir/deepsubmodule &&
    -+		git commit -m "new deep submodule"
    -+	) &&
    -+	git add submodule &&
    -+	git commit -m "new submodule" &&
    -+
    ++	add_submodule_commits &&
    ++	add_superproject_commits &&
     +	# Fetch the new superproject commit
     +	(
     +		cd downstream &&
     +		git switch --recurse-submodules no-submodules &&
     +		git fetch --recurse-submodules >../actual.out 2>../actual.err
     +	) &&
    -+	test_must_be_empty actual.out &&
    -+	git rev-parse --short HEAD >superhead &&
    -+	git -C submodule rev-parse --short HEAD >subhead &&
    -+	git -C deepsubmodule rev-parse --short HEAD >deephead &&
    -+	verify_fetch_result actual.err &&
    ++	super_head=$(git rev-parse --short HEAD) &&
    ++	sub_head=$(git -C submodule rev-parse --short HEAD) &&
    ++	deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) &&
    ++
    ++	# assert that these are fetched from commits, not the index
    ++	check_sub $sub_head $super_head &&
    ++	check_deep $deep_head $sub_head &&
     +
    -+	# Assert that the fetch happened at the non-HEAD commits
    -+	grep "Fetching submodule submodule at commit $superhead" actual.err &&
    -+	grep "Fetching submodule submodule/subdir/deepsubmodule at commit $subhead" actual.err
    ++	test_must_be_empty actual.out &&
    ++	verify_fetch_result actual.err
     +'
     +
     +test_expect_success "'--recurse-submodules' should ignore changed, inactive submodules" '
    -+	# Fetch any leftover commits from other tests.
    -+	git -C downstream fetch --recurse-submodules &&
    -+	# Create new superproject commit with updated submodules
    -+	add_upstream_commit &&
    -+	(
    -+		cd submodule &&
    -+		(
    -+			cd subdir/deepsubmodule &&
    -+			git fetch &&
    -+			git checkout -q FETCH_HEAD
    -+		) &&
    -+		git add subdir/deepsubmodule &&
    -+		git commit -m "new deep submodule"
    -+	) &&
    -+	git add submodule &&
    -+	git commit -m "new submodule" &&
    ++	add_submodule_commits &&
    ++	add_superproject_commits &&
     +
     +	# Fetch the new superproject commit
     +	(
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     +		git -c submodule.submodule.active=false fetch --recurse-submodules >../actual.out 2>../actual.err
     +	) &&
     +	test_must_be_empty actual.out &&
    -+	git rev-parse --short HEAD >superhead &&
    ++	super_head=$(git rev-parse --short HEAD) &&
    ++	check_super $super_head &&
     +	# Neither should be fetched because the submodule is inactive
    -+	rm subhead &&
    -+	rm deephead &&
    ++	rm expect.err.sub &&
    ++	rm expect.err.deep &&
     +	verify_fetch_result actual.err
     +'
     +
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     +'
     +
     +test_expect_success "'--recurse-submodules' should fetch submodule commits in changed submodules and the index" '
    -+	# Fetch any leftover commits from other tests.
    -+	git -C downstream fetch --recurse-submodules &&
     +	# Create new commit in origin/super
    -+	add_upstream_commit &&
    -+	(
    -+		cd submodule &&
    -+		(
    -+			cd subdir/deepsubmodule &&
    -+			git fetch &&
    -+			git checkout -q FETCH_HEAD
    -+		) &&
    -+		git add subdir/deepsubmodule &&
    -+		git commit -m "new deep submodule"
    -+	) &&
    -+	git add submodule &&
    -+	git commit -m "new submodule" &&
    ++	add_submodule_commits &&
    ++	add_superproject_commits &&
     +
     +	# Create new commit in origin/super-sub2-only
     +	git checkout super-sub2-only &&
    @@ t/t5526-fetch-submodules.sh: test_expect_success "'--recurse-submodules=on-deman
     +		git fetch --recurse-submodules >../actual.out 2>../actual.err
     +	) &&
     +	test_must_be_empty actual.out &&
    -+
    -+	# Assert that the submodules in the super branch are fetched
    -+	git rev-parse --short HEAD >superhead &&
    -+	git -C submodule rev-parse --short HEAD >subhead &&
    -+	git -C deepsubmodule rev-parse --short HEAD >deephead &&
    -+	verify_fetch_result actual.err &&
    -+	# grep for the exact line to check that the submodule is read
    -+	# from the index, not from a commit
    -+	grep "^Fetching submodule submodule\$" actual.err &&
    -+
    -+	# Assert that super-sub2-only and submodule2 were fetched even
    -+	# though another branch is checked out
    ++	deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) &&
    ++	sub_head=$(git -C submodule rev-parse --short HEAD) &&
    ++	sub2_head=$(git -C submodule2 rev-parse --short HEAD) &&
    ++	super_head=$(git rev-parse --short HEAD) &&
     +	super_sub2_only_head=$(git rev-parse --short super-sub2-only) &&
    -+	grep -E "\.\.${super_sub2_only_head}\s+super-sub2-only\s+-> origin/super-sub2-only" actual.err &&
    -+	grep "Fetching submodule submodule2 at commit $super_sub2_only_head" actual.err &&
    -+	sub2head=$(git -C submodule2 rev-parse --short HEAD) &&
    -+	grep -E "\.\.${sub2head}\s+sub2\s+-> origin/sub2" actual.err
    ++
    ++	# Use test_cmp manually because verify_fetch_result does not
    ++	# consider submodule2. All the repos should be fetched, but only
    ++	# submodule2 should be read from a commit
    ++	cat <<-EOF > expect.err.combined &&
    ++	From $pwd/.
    ++	   OLD_HEAD..$super_head  super           -> origin/super
    ++	   OLD_HEAD..$super_sub2_only_head  super-sub2-only -> origin/super-sub2-only
    ++	Fetching submodule submodule
    ++	From $pwd/submodule
    ++	   OLD_HEAD..$sub_head  sub        -> origin/sub
    ++	Fetching submodule submodule/subdir/deepsubmodule
    ++	From $pwd/deepsubmodule
    ++	   OLD_HEAD..$deep_head  deep       -> origin/deep
    ++	Fetching submodule submodule2 at commit $super_sub2_only_head
    ++	From $pwd/submodule2
    ++	   OLD_HEAD..$sub2_head  sub2       -> origin/sub2
    ++	EOF
    ++	sed -E "s/[0-9a-f]+\.\./OLD_HEAD\.\./" actual.err >actual.err.cmp &&
    ++	test_cmp expect.err.combined actual.err.cmp
     +'
     +
      test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" '
    - 	add_upstream_commit &&
    + 	add_submodule_commits &&
      	echo a >> file &&
    +@@ t/t5526-fetch-submodules.sh: test_expect_success 'recursive fetch after deinit a submodule' '
    + 	test_cmp expect actual
    + '
    + 
    ++test_expect_success 'setup repo with upstreams that share a submodule name' '
    ++	mkdir same-name-1 &&
    ++	(
    ++		cd same-name-1 &&
    ++		git init &&
    ++		test_commit --no-tag a
    ++	) &&
    ++	git clone same-name-1 same-name-2 &&
    ++	# same-name-1 and same-name-2 both add a submodule with the
    ++	# name "submodule"
    ++	(
    ++		cd same-name-1 &&
    ++		mkdir submodule &&
    ++		git -C submodule init &&
    ++		test_commit -C submodule --no-tag a1 &&
    ++		git submodule add "$pwd/same-name-1/submodule" &&
    ++		git add submodule &&
    ++		git commit -m "super-a1"
    ++	) &&
    ++	(
    ++		cd same-name-2 &&
    ++		mkdir submodule &&
    ++		git -C submodule init &&
    ++		test_commit -C submodule --no-tag a2 &&
    ++		git submodule add "$pwd/same-name-2/submodule" &&
    ++		git add submodule &&
    ++		git commit -m "super-a2"
    ++	) &&
    ++	git clone same-name-1 -o same-name-1 same-name-downstream &&
    ++	(
    ++		cd same-name-downstream &&
    ++		git remote add same-name-2 ../same-name-2 &&
    ++		git fetch --all &&
    ++		# init downstream with same-name-1
    ++		git submodule update --init
    ++	)
    ++'
    ++
    ++test_expect_success 'fetch --recurse-submodules updates name-conflicted, populated submodule' '
    ++	test_when_finished "git -C same-name-downstream checkout master" &&
    ++	(
    ++		cd same-name-1 &&
    ++		test_commit -C submodule --no-tag b1 &&
    ++		git add submodule &&
    ++		git commit -m "super-b1"
    ++	) &&
    ++	(
    ++		cd same-name-2 &&
    ++		test_commit -C submodule --no-tag b2 &&
    ++		git add submodule &&
    ++		git commit -m "super-b2"
    ++	) &&
    ++	(
    ++		cd same-name-downstream &&
    ++		# even though the .gitmodules is correct, we cannot
    ++		# fetch from same-name-2
    ++		git checkout same-name-2/master &&
    ++		git fetch --recurse-submodules same-name-1 &&
    ++		test_must_fail git fetch --recurse-submodules same-name-2
    ++	) &&
    ++	super_head1=$(git -C same-name-1 rev-parse HEAD) &&
    ++	git -C same-name-downstream cat-file -e $super_head1 &&
    ++
    ++	super_head2=$(git -C same-name-2 rev-parse HEAD) &&
    ++	git -C same-name-downstream cat-file -e $super_head2 &&
    ++
    ++	sub_head1=$(git -C same-name-1/submodule rev-parse HEAD) &&
    ++	git -C same-name-downstream/submodule cat-file -e $sub_head1 &&
    ++
    ++	sub_head2=$(git -C same-name-2/submodule rev-parse HEAD) &&
    ++	test_must_fail git -C same-name-downstream/submodule cat-file -e $sub_head2
    ++'
    ++
    ++test_expect_success 'fetch --recurse-submodules updates name-conflicted, unpopulated submodule' '
    ++	(
    ++		cd same-name-1 &&
    ++		test_commit -C submodule --no-tag c1 &&
    ++		git add submodule &&
    ++		git commit -m "super-c1"
    ++	) &&
    ++	(
    ++		cd same-name-2 &&
    ++		test_commit -C submodule --no-tag c2 &&
    ++		git add submodule &&
    ++		git commit -m "super-c2"
    ++	) &&
    ++	(
    ++		cd same-name-downstream &&
    ++		git checkout master &&
    ++		git rm .gitmodules &&
    ++		git rm submodule &&
    ++		git commit -m "no submodules" &&
    ++		git fetch --recurse-submodules same-name-1
    ++	) &&
    ++	head1=$(git -C same-name-1/submodule rev-parse HEAD) &&
    ++	head2=$(git -C same-name-2/submodule rev-parse HEAD) &&
    ++	(
    ++		cd same-name-downstream/.git/modules/submodule &&
    ++		# The submodule has core.worktree pointing to the "git
    ++		# rm"-ed directory, overwrite the invalid value.
    ++		git --work-tree=. cat-file -e $head1 &&
    ++		test_must_fail git --work-tree=. cat-file -e $head2
    ++	)
    ++'
    ++
    + test_done
 8:  8aa68111b0 <  -:  ---------- submodule: read shallows when finding changed submodules
 9:  05a8b93154 ! 10:  e1ac74eee4 submodule: fix latent check_has_commit() bug
    @@ submodule.c: static int check_has_commit(const struct object_id *oid, void *data
      
      	type = oid_object_info(&subrepo, oid, NULL);
     @@ submodule.c: static int submodule_has_commits(struct repository *r,
    - {
    - 	struct has_commit_data has_commit = { r, 1, path, super_oid };
    + 		.super_oid = super_oid
    + 	};
      
     -	/*
     -	 * Perform a cheap, but incorrect check for the existence of 'commits'.

base-commit: 679e3693aba0c17af60c031f7eef68f2296b8dad