Subject: Re: Bugreport - submodules are fetched twice in some cases

From: Junio C Hamano <[email-masked]>
Date: Mon, 16 May 2022 11:25:03 -0700

Glen Choo <[email-masked]> writes:

> I don't think we are in the ideal scenario because we only snapshot when
> we fetch without "--all":
>
>   cmd_fetch() > *fetch_one()* > do_fetch() > backfill_tags() >
>     fetch_and_consume_refs() > store_updated_refs() >
>     check_for_new_submodule_commits()
>
> In the ideal scenario, it would be something like:
>
>   cmd_fetch() >
>     TODO_snapshot_old_refs(), fetch_[one|multiple](),
>     TODO_register_new_refs()
>
> It looks non-trivial enough that I don't think I'll try to fix this
> soon, but it does not look prohibitively hard.

It matches my gut feeling.

>> Provided if we have the "make sure everything needed in the
>> submodule is fetched by inspecting the range of commits we fetch for
>> a superproject" working correctly for a single remote, an
>> alternative approach is to run "git fetch --recurse-submodules" for
>> each remote separately, without the "parent" process doing anything
>> in the submodule (i.e. you earlier counted R+1 fetches, but instead,
>> we make R fetches in the submodule.  It is less than ideal but it
>> may be easier to implement).
>>
>> Thoughts?
>
> The +1 fetch is redundant, so it's probably good to get rid of it
> anyway.

Sounds sensible.  It should be a single-liner, i.e.

 builtin/fetch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git i/builtin/fetch.c w/builtin/fetch.c
index eeee5ac8f1..be61c390c1 100644
--- i/builtin/fetch.c
+++ w/builtin/fetch.c
@@ -2262,7 +2262,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		result = fetch_multiple(&list, max_children);
 	}
 
-	if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
+	if (!result && remote && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
 		struct strvec options = STRVEC_INIT;
 		int max_children = max_jobs;