From 0353c6881890db1302f0f1bdf85c6076eed61113 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 16 May 2022 16:53:40 -0700 Subject: [PATCH] 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 Signed-off-by: Junio C Hamano --- builtin/fetch.c | 16 +++++++++++++++- t/t5526-fetch-submodules.sh | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index e3791f09ed..8b15c40bb2 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); + + /* 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 43dada8544..a301b56db8 100755 --- 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