Re: [PATCH 7/8] fetch: fetch unpopulated, changed submodules

[Glen Choo <chooglen@google.com>]

Glen Choo <chooglen@google.com> writes:

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

Reviewers (and myself) have rightfully asked why "git fetch" should
continue to bother looking for submodules in the index if it already
fetches all of the changed submodules. The reasons for this are twofold:

1. The primary reason is that --recurse-submodules, aka
--recurse-submodules=yes does an unconditional fetch in each of the
submodules regardless of whether they have been changed by a
superproject commit. This is the behavior of e.g. from
 t/t5526-fetch-submodules.sh:101:

  test_expect_success "fetch --recurse-submodules recurses into submodules" '
    # Creates commits in the submodules but NOT the superproject
    add_upstream_commit &&
    (
      cd downstream &&
      git fetch --recurse-submodules >../actual.out 2>../actual.err
    ) &&
    test_must_be_empty actual.out &&
    # Assert that the new submodule commits have been fetched and that
    # no superproject commit was fetched
    verify_fetch_result actual.err
  '

Thus, we continue to check the index to implement this unconditional
fetching behavior.

2. In the --recurse-submodule=on-demand case, it can be correct to
ignore the index because "on-demand" only requires us to fetch changed
submodules. But in the event that a submodule is both changed and
populated, we may prefer to read the index instead of the superproject
commit, because the contents of the index are more obvious and more
actionable to the user.

For example, we print the path of the submodule when attempting to fetch
a submodule for debugging purposes:

- For a populated submodule, we print "Fetching submodule <path>"
- For an unpopulated submodule, we print "Fetching submodule <path> at
  commit foo"

Presumably, the user would prefer to see the "populated submodule"
message because it's easier to work with, e.g. "git -C <path>
<fix-the-problem>" instead of "git checkout --recurse-submodules
<commit-with-submodule> && git <fix-the-problem>".

The latter is not a sufficient reason to read the index and then the
changed submodule list (because we could try to read the changed
submodule configuration from index), but since we need to support
--recurse-submodules=yes, this implementation is convenient for
achieving both goals.