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>

Junio C Hamano committed May 16, 2022 at 16:53 UTC 0353c6881890db1302f0f1bdf85c6076eed61113
2 files changed +42 -1
builtin/fetch.c
+15 -1
@@ -2187,6 +2187,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
2187 else if (argc > 1)
2188 die(_("fetch --all does not make sense with refspecs"));
2189 (void) for_each_remote(get_one_remote_for_fetch, &list);
2190 +
2191 + /* do not do fetch_multiple() of one */
2192 + if (list.nr == 1)
2193 + remote = remote_get(list.items[0].string);
2194 } else if (argc == 0) {
2195 /* No arguments -- use default remote */
2196 remote = remote_get(NULL);
@@ -2261,7 +2265,17 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
2265 result = fetch_multiple(&list, max_children);
2266 }
2267
2264 - if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
2268 +
2269 + /*
2270 + * This is only needed after fetch_one(), which does not fetch
2271 + * submodules by itself.
2272 + *
2273 + * When we fetch from multiple remotes, fetch_multiple() has
2274 + * already updated submodules to grab commits necessary for
2275 + * the fetched history from each remote, so there is no need
2276 + * to fetch submodules from here.
2277 + */
2278 + if (!result && remote && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
2279 struct strvec options = STRVEC_INIT;
2280 int max_children = max_jobs;
2281
t/t5526-fetch-submodules.sh
+27
@@ -1125,4 +1125,31 @@ test_expect_success 'fetch --recurse-submodules updates name-conflicted, unpopul
1125 )
1126 '
1127
1128 +test_expect_success 'fetch --all with --recurse-submodules' '
1129 + test_when_finished "rm -fr src_clone" &&
1130 + git clone --recurse-submodules src src_clone &&
1131 + (
1132 + cd src_clone &&
1133 + git config submodule.recurse true &&
1134 + git config fetch.parallel 0 &&
1135 + git fetch --all 2>../fetch-log
1136 + ) &&
1137 + grep "^Fetching submodule sub$" fetch-log >fetch-subs &&
1138 + test_line_count = 1 fetch-subs
1139 +'
1140 +
1141 +test_expect_success 'fetch --all with --recurse-submodules with multiple' '
1142 + test_when_finished "rm -fr src_clone" &&
1143 + git clone --recurse-submodules src src_clone &&
1144 + (
1145 + cd src_clone &&
1146 + git remote add secondary ../src &&
1147 + git config submodule.recurse true &&
1148 + git config fetch.parallel 0 &&
1149 + git fetch --all 2>../fetch-log
1150 + ) &&
1151 + grep "Fetching submodule sub" fetch-log >fetch-subs &&
1152 + test_line_count = 2 fetch-subs
1153 +'
1154 +
1155 test_done