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

[Junio C Hamano <gitster@pobox.com>]

Glen Choo <chooglen@google.com> writes:

> This uses a "first one wins approach", which obviously doesn't have
> correctness guarantees. But in practice, I don't think this is likely to
> cause problems:
>
> - As far as I can tell, the only value we read from .gitmodules is
>   'submodule.<name>.fetchRecurseSubmodules', and this value gets
>   overridden by two other values: the CLI option, and the config
>   variable with the same name in .git/config.
>
>   During "git submodule init", we copy the config values from
>   .gitmodules to .git/config. Since we can only fetch init-ed submodules
>   anyway, it's quite unlikely that we will ever actually make use of the
>   .gitmodules config.

These are reasonable.

> - Even if we do use the .gitmodules config values, it's unlikely that
>   the values in .gitmodules will change often, so it _probably_ won't
>   matter which one we choose.

What bad things would we see if the value changes during the span of
history of the superproject we fetched?  How often we would see
broken behaviour is immaterial and breakage being rare is a no excuse
to import a new code with designed-in flaw.  Unless the "rare" is
"never", that is.

I would think using ANY values from .gitmodules without having the
end-user agree with the settings and copying the settings to the
.git/config is a BUG.  So if it mattered from which superproject
commit we took .gitmodules from, that would mean we already have
such a bug and it is not a new problem.

That would be a reasonable argument for this topic. Together with
the previous point, i.e. we do not copy values we see in the in-tree
.gitmodules file to .git/config anyway, it would make a good enough
assurance, I would think.

> - This only matters when the submodule is not in the index. If the
>   submodule _is_ in the index, we read .gitmodules from the filesystem
>   i.e. these patches shouldn't change the behavior for submodules in the
>   index.

How often we would see broken behaviour does not matter.  If it is
broken when the submodule is not in the index, we need to know.

But as you said, it does not sound likely that in-tree .gitmodules
matters.

It leads to a possible #leftoverbit clean-up.  Because we only fetch
submodules that are initialized, the API functions we are using in
this series has no reason to require us to feed _a_ commit in the
superproject to them so that they can find .gitmodules in them.

Fixing the API can probably be left outside the scope of the topic,
to be done soon after the dust from the topic settles, I think, to
avoid distracting us from the topic.

Thanks.