Skip to contentSkip to conversation

fetch: choose a sensible default with --jobs=0 again

PATCH In review Matthias Aßhauer via GitGitGadget 2 participants 2 messages Feb 20, 2023 – Feb 21, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fetch: choose a sensible default with --jobs=0 again

From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <[email protected]>

prior to 51243f9 (run-command API: don't fall back on online_cpus(), 2022-10-12) git fetch --multiple --jobs=0 would choose some default amount of jobs, similar to git -c fetch.parallel=0 fetch --multiple. While our documentation only ever promised that fetch.parallel would fall back to a "sensible default", it makes sense to do the same for --jobs. So fall back to online_cpus() and not BUG() out.

This fixes https://github.com/git-for-windows/git/issues/4302

---
fetch: choose a sensible default with --jobs=0 again

Prior to 51243f9 (run-command API: don't fall back on online_cpus(), 2022-10-12) git fetch --multiple --jobs=0 would choose some default amount of jobs, similar to git -c fetch.parallel=0 fetch --multiple. While our documentation only ever promised that fetch.parallel would fall back to a "sensible default", it makes sense to do the same for --jobs. So fall back to online_cpus() and not BUG() out.

This was originally reported at
https://lore.kernel.org/git/PSAP153MB03910458707331B64FA7460DCAA19@PSAP153MB0391.APCP153.PROD.OUTLOOK.COM/T/#u

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1483%2Frimrul%2Ffix-fetch-jobs-0-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1483/rimrul/fix-fetch-jobs-0-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1483

 builtin/fetch.c           | 3 +++
 t/t5514-fetch-multiple.sh | 5 +++++
 2 files changed, 8 insertions(+)
builtin/fetch.c +3 -0
index a21ce89312d..a09606b4726 100644
@@ -2196,6 +2196,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (dry_run)
 		write_fetch_head = 0;
 
+	if (!max_jobs)
+		max_jobs = online_cpus();
+
 	if (!git_config_get_string_tmp("fetch.bundleuri", &bundle_uri) &&
 	    fetch_bundle_uri(the_repository, bundle_uri, NULL))
 		warning(_("failed to fetch bundles from '%s'"), bundle_uri);
t/t5514-fetch-multiple.sh +5 -0
index 511ba3bd454..54f422ced32 100755
@@ -197,4 +197,9 @@ test_expect_success 'parallel' '
 	test_i18ngrep "could not fetch .two.*128" err
 '
 
+test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
+	(cd test &&
+	 git fetch --multiple --jobs=0)
+'
+
 test_done

base-commit: d9d677b2d8cc5f70499db04e633ba7a400f64cbf

Signature · 1 line
gitgitgadget
#1 lore raw

"Matthias Aßhauer via GitGitGadget" <[email protected]>

Quoted 8 lines · writes:

From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <[email protected]>

prior to 51243f9 (run-command API: don't fall back on online_cpus(), 2022-10-12) git fetch --multiple --jobs=0 would choose some default amount of jobs, similar to git -c fetch.parallel=0 fetch --multiple. While our documentation only ever promised that fetch.parallel would fall back to a "sensible default", it makes sense to do the same for --jobs. So fall back to online_cpus() and not BUG() out.

Yup, the way I read 51243f9f (run-command API: don't fall back on online_cpus(), 2022-10-12) is that it wanted to make it a best practice for the callers of the API to be making an explicit choice of scaling with online_cpus() or other metrics depending on their needs. The problematic commit does touch some fetch-related code paths and make them call online_cpus() themselves, so we probably are looking at an inadequate tests, and this is a fix that is in line of the spirit, completing what 51243f9f started to do.

Thanks, will queue.