[PATCH] fetch: do not run a redundant fetch from submodule

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

When 7dce19d3 (fetch/pull: Add the --recurse-submodules option,
2010-11-12) introduced the "--recurse-submodule" option, the
approach taken was to perform fetches in submodules only once, after
all the main fetching (it may usually be a fetch from a single
remote, but it could be fetching from a group of remotes using
fetch_multiple()) succeeded.  Later we added "--all" to fetch from
all defined remotes, which complicated things even more.

If your project has a submodule, and you try to run "git fetch
--recurse-submodule --all", you'd see a fetch for the top-level,
which invokes another fetch for the submodule, followed by another
fetch for the same submodule.  All but the last fetch for the
submodule come from a "git fetch --recurse-submodules" subprocess
that is spawned via the fetch_multiple() interface for the remotes,
and the last fetch comes from the code at the end.

Because recursive fetching from submodules is done in each fetch for
the top-level in fetch_multiple(), the last fetch in the submodule
is redundant.  It only matters when fetch_one() interacts with a
single remote at the top-level.

While we are at it, there is one optimization that exists in dealing
with a group of remote, but is missing when "--all" is used.  In the
former, when the group turns out to be a group of one, instead of
spawning "git fetch" as a subprocess via the fetch_multiple()
interface, we use the normal fetch_one() code path.  Do the same
when handing "--all", if it turns out that we have only one remote
defined.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * So, here is a mixed thing that fixes the issue in two different
   ways, which makes it unsuitable to be the final patch.  Either
   "turning --all into a single fetch with optimizaiton" that is in
   the first hunk, or "don't do the final sweep unless doing a
   single fetch" that is in the second hunk, is sufficient to make
   the symptom disappear.  Of course, using them both does not break
   anything, but it somehow feels unsatisfactory, and makes readers
   feel that we should be able to do better.

   But the better thing being what Glen alluded to as "non-trivial
   not prohibitivey hard", I'll stop here.

 builtin/fetch.c                    |  6 +++++-
 t/t5617-clone-submodules-remote.sh | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e3791f09ed..359321e731 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2187,6 +2187,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		else if (argc > 1)
 			die(_("fetch --all does not make sense with refspecs"));
 		(void) for_each_remote(get_one_remote_for_fetch, &list);
+
+		/* no point doing fetch_multiple() of one */
+		if (list.nr == 1)
+			remote = remote_get(list.items[0].string);
 	} else if (argc == 0) {
 		/* No arguments -- use default remote */
 		remote = remote_get(NULL);
@@ -2261,7 +2265,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;
 
diff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh
index ca8f80083a..8b6b482a39 100755
--- a/t/t5617-clone-submodules-remote.sh
+++ b/t/t5617-clone-submodules-remote.sh
@@ -72,6 +72,19 @@ test_expect_success 'clone with --single-branch' '
 	)
 '
 
+test_expect_success 'fetch --all with --recurse-submodules' '
+	test_when_finished "rm -fr super_clone" &&
+	git clone --recurse-submodules srv.bare super_clone &&
+	(
+		cd super_clone &&
+		git config submodule.recurse true &&
+		git config fetch.parallel 0 &&
+		git fetch --all 2>../fetch-log
+	) &&
+	grep "Fetching sub" fetch-log >fetch-subs &&
+	test_line_count = 1 fetch-subs
+'
+
 # do basic partial clone from "srv.bare"
 # confirm partial clone was registered in the local config for super and sub.
 test_expect_success 'clone with --filter' '
-- 
2.36.1-379-g841af8e974