Re: [PATCH v5 00/10] fetch --recurse-submodules: fetch unpopulated submodules

[Jonathan Tan <jonathantanmy@google.com>]

Glen Choo <chooglen@google.com> writes:
> - <20220304235328.649768-1-jonathantanmy@google.com> I've described the
>   differences between the no-submodule-in-index test and the
>   other-submodule-in-index test (their comments now refer to one
>   another, so the contrast is more obvious), but didn't reorder them
>   because I thought that made the test setup less intuitive to read.

Thanks - the comments make sense.

> - <20220304234622.647776-1-jonathantanmy@google.com> I added
>   expect.err.sub2 to verify_test_result() but didn't change
>   write_expected_super() to account for sub2. It turned out to be tricky
>   to predict the output when 'super' fetches >1 branch because each
>   fetched branch can affect the formatting. e.g.
> 
>     	   OLD_HEAD..super  super           -> origin/super
> 
>   can become
> 
>     	   OLD_HEAD..super  super                   -> origin/super
>     	   OLD_HEAD..super  some-other-branch       -> origin/some-other-branch
> 
>   (I could work around this by replacing the whitespace with sed, but it
>   seemed like too much overhead for a one-off test).

Overwriting just the super part works for me, thanks.

The only thing remaining from me is my comment about fetching OIDs from
one submodule into another (of the same name but different URL) [1], but
I looked into it myself and we can probably postpone handling this to
another patch set.

In such a patch set, we would probably need to store the URLs that are
reported by upstream .gitmodules somewhere. (I forgot that we don't use
them in this patch.) And then, either implement an autosync function
(like "git submodule sync", perhaps gated by a "--sync-submodules"
argument so that users can include it when fetching new commits and
exclude it when fetching historical commits) and/or use those URLs in a
diagnostic message to be printed when the fetch fails.

As it is, the existing fetch-into-submodules-at-HEAD also suffers from
the same flaw, so I'm OK postponing this to another patch set.

So,
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>

[1] https://lore.kernel.org/git/20220304234622.647776-1-jonathantanmy@google.com/