* [PATCH] fetch: choose a sensible default with --jobs=0 again
@ 2023-02-20 21:33 Matthias Aßhauer via GitGitGadget
2023-02-21 20:15 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Matthias Aßhauer via GitGitGadget @ 2023-02-20 21:33 UTC (permalink / raw)
To: git
Cc: Drew Noakes, Ævar Arnfjörð Bjarmason,
Matthias Aßhauer, Matthias Aßhauer
From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
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
Reported-by: Drew Noakes <drnoakes@microsoft.com>
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
---
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(+)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a21ce89312d..a09606b4726 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -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);
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index 511ba3bd454..54f422ced32 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] fetch: choose a sensible default with --jobs=0 again
2023-02-20 21:33 [PATCH] fetch: choose a sensible default with --jobs=0 again Matthias Aßhauer via GitGitGadget
@ 2023-02-21 20:15 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2023-02-21 20:15 UTC (permalink / raw)
To: Matthias Aßhauer via GitGitGadget
Cc: git, Drew Noakes, Ævar Arnfjörð Bjarmason,
Matthias Aßhauer
"Matthias Aßhauer via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>
>
> 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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-02-21 20:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-20 21:33 [PATCH] fetch: choose a sensible default with --jobs=0 again Matthias Aßhauer via GitGitGadget
2023-02-21 20:15 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).