fetch: do not run a redundant fetch from submodule

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.

Reviewed-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e3791f09ed51d0d96b945b747d3dd74ad2d63165..8b15c40bb27e6d62a029f547c20d28ff89c9d2a1 100644 (file)
--- 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);
+
+               /* do not do 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,17 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
                result = fetch_multiple(&list, max_children);
        }
 
-       if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
+
+       /*
+        * This is only needed after fetch_one(), which does not fetch
+        * submodules by itself.
+        *
+        * When we fetch from multiple remotes, fetch_multiple() has
+        * already updated submodules to grab commits necessary for
+        * the fetched history from each remote, so there is no need
+        * to fetch submodules from here.
+        */
+       if (!result && remote && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
                struct strvec options = STRVEC_INIT;
                int max_children = max_jobs;
 

diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 43dada854443dcf430439dffd6d38d5d5be4f3d0..a301b56db894253829a0934647396ad854ec11b7 100755 (executable)
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -1125,4 +1125,31 @@ test_expect_success 'fetch --recurse-submodules updates name-conflicted, unpopul
        )
 '
 
+test_expect_success 'fetch --all with --recurse-submodules' '
+       test_when_finished "rm -fr src_clone" &&
+       git clone --recurse-submodules src src_clone &&
+       (
+               cd src_clone &&
+               git config submodule.recurse true &&
+               git config fetch.parallel 0 &&
+               git fetch --all 2>../fetch-log
+       ) &&
+       grep "^Fetching submodule sub$" fetch-log >fetch-subs &&
+       test_line_count = 1 fetch-subs
+'
+
+test_expect_success 'fetch --all with --recurse-submodules with multiple' '
+       test_when_finished "rm -fr src_clone" &&
+       git clone --recurse-submodules src src_clone &&
+       (
+               cd src_clone &&
+               git remote add secondary ../src &&
+               git config submodule.recurse true &&
+               git config fetch.parallel 0 &&
+               git fetch --all 2>../fetch-log
+       ) &&
+       grep "Fetching submodule sub" fetch-log >fetch-subs &&
+       test_line_count = 2 fetch-subs
+'
+
 test_done