Glen Choo [off-list ref] writes:

quoted

quoted

+	# 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
+'

Could verify_fetch_result be modified to consider the new submodule
instead?

Since submodule2 is on the end of the file, I could modify
verify_fetch_result() to concatenate extra text on the end. But if it
weren't in the middle, we'd need to insert arbitrary text in the middle
of the file.

I can't think of a good way to do this without compromising test
readability, so I'll just do concatenation for now.

Looking at it, I think you can do it by adding a section that verifies
the "Fetching submodule submodule2" part if the file is present (so, no
change in behavior in the rest of the tests since they don't write this
file) and also modifying check_super to allow specification of the sub2
part (or making a new function for this).

quoted

What's the error message printed to the user here? (Just from reading
the code, I would have expected this to succeed, with the submodule
fetch being from same-name-1's submodule since we're fetching submodules
by name, but apparently that is not the case.)

Yeah, I think this might trip up some readers. The message is:

  From ../same-name-2
    b7ebb59..944b5ac  master     -> same-name-2/master
  Fetching submodule submodule
  fatal: git upload-pack: not our ref 7ff6874077503acb9d0a52e280aaed9748276319
  fatal: remote error: upload-pack: not our ref 7ff6874077503acb9d0a52e280aaed9748276319
  Errors during submodule fetch:
          submodule

Which, I believe, comes from how we fetch commits by oid:

  static int get_next_submodule(struct child_process *cp, struct strbuf *err,
              void *data, void **task_cb)
  [...]
    oid_array_for_each_unique(task->commits,
          append_oid_to_argv, &cp->args);

When the following is true:

- the submodule is found in the index
- we are fetching submodules unconditionally (--recurse-submodules=yes")
- no superproject commit "changes" the submodule

task->commits is empty, and we just fetch the from the submodule's
remote by name. But as long as any superproject commit "changes" the
submodule, we try to fetch by oid, which, as this test demonstrates, may
fail.

Ah, so we try to fetch an OID from a submodule given by a fetched
commit, which is different from the submodule the client already has
locally. This might be a sign that we need to store more information
about the submodule so that we can print a clearer message. I haven't
looked into this deeply, but this might be possible by putting more
information in the util of changed_submodule_names, and when we have
already seen that submodule, to add more information to the util instead
of skipping it.