{"thread":{"id":"59274","subject":"[PATCH] fetch: choose a sensible default with --jobs=0 again","startedAt":"2023-02-20T21:33:32Z","lastAt":"2023-02-21T20:15:45Z","messageCount":2,"participants":["Matthias Aßhauer via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"472356","messageId":"pull.1483.git.1676928805555.gitgitgadget@gmail.com","threadId":"59274","inReplyTo":null,"subject":"[PATCH] fetch: choose a sensible default with --jobs=0 again","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-20T21:33:25Z","receivedAt":"2023-02-20T21:33:32Z","isPatch":true,"sender":{"key":"mha1993@live.de","avatar":"https://avatars.githubusercontent.com/u/6178234?v=4"},"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n\nprior to 51243f9 (run-command API: don't fall back on online_cpus(),\n2022-10-12) `git fetch --multiple --jobs=0` would choose some default amount\nof jobs, similar to `git -c fetch.parallel=0 fetch --multiple`. While our\ndocumentation only ever promised that `fetch.parallel` would fall back to a\n\"sensible default\", it makes sense to do the same for `--jobs`. So fall back\nto online_cpus() and not BUG() out.\n\nThis fixes https://github.com/git-for-windows/git/issues/4302\n\nReported-by: Drew Noakes <drnoakes@microsoft.com>\nSigned-off-by: Matthias Aßhauer <mha1993@live.de>\n---\n    fetch: choose a sensible default with --jobs=0 again\n    \n    Prior to 51243f9 (run-command API: don't fall back on online_cpus(),\n    2022-10-12) git fetch --multiple --jobs=0 would choose some default\n    amount of jobs, similar to git -c fetch.parallel=0 fetch --multiple.\n    While our documentation only ever promised that fetch.parallel would\n    fall back to a \"sensible default\", it makes sense to do the same for\n    --jobs. So fall back to online_cpus() and not BUG() out.\n    \n    This was originally reported at\n    https://lore.kernel.org/git/PSAP153MB03910458707331B64FA7460DCAA19@PSAP153MB0391.APCP153.PROD.OUTLOOK.COM/T/#u\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1483%2Frimrul%2Ffix-fetch-jobs-0-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1483/rimrul/fix-fetch-jobs-0-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1483\n\n builtin/fetch.c           | 3 +++\n t/t5514-fetch-multiple.sh | 5 +++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex a21ce89312d..a09606b4726 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -2196,6 +2196,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tif (dry_run)\n \t\twrite_fetch_head = 0;\n \n+\tif (!max_jobs)\n+\t\tmax_jobs = online_cpus();\n+\n \tif (!git_config_get_string_tmp(\"fetch.bundleuri\", &bundle_uri) &&\n \t    fetch_bundle_uri(the_repository, bundle_uri, NULL))\n \t\twarning(_(\"failed to fetch bundles from '%s'\"), bundle_uri);\ndiff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh\nindex 511ba3bd454..54f422ced32 100755\n--- a/t/t5514-fetch-multiple.sh\n+++ b/t/t5514-fetch-multiple.sh\n@@ -197,4 +197,9 @@ test_expect_success 'parallel' '\n \ttest_i18ngrep \"could not fetch .two.*128\" err\n '\n \n+test_expect_success 'git fetch --multiple --jobs=0 picks a default' '\n+\t(cd test &&\n+\t git fetch --multiple --jobs=0)\n+'\n+\n test_done\n\nbase-commit: d9d677b2d8cc5f70499db04e633ba7a400f64cbf\n-- \ngitgitgadget\n"},{"id":"472398","messageId":"xmqqcz62epw3.fsf@gitster.g","threadId":"59274","inReplyTo":"pull.1483.git.1676928805555.gitgitgadget@gmail.com","subject":"Re: [PATCH] fetch: choose a sensible default with --jobs=0 again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-21T20:15:40Z","receivedAt":"2023-02-21T20:15:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Matthias Aßhauer via GitGitGadget\"  <gitgitgadget@gmail.com>\nwrites:\n\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>\n>\n> prior to 51243f9 (run-command API: don't fall back on online_cpus(),\n> 2022-10-12) `git fetch --multiple --jobs=0` would choose some default amount\n> of jobs, similar to `git -c fetch.parallel=0 fetch --multiple`. While our\n> documentation only ever promised that `fetch.parallel` would fall back to a\n> \"sensible default\", it makes sense to do the same for `--jobs`. So fall back\n> to online_cpus() and not BUG() out.\n\nYup, the way I read 51243f9f (run-command API: don't fall back on\nonline_cpus(), 2022-10-12) is that it wanted to make it a best\npractice for the callers of the API to be making an explicit choice\nof scaling with online_cpus() or other metrics depending on their\nneeds.  The problematic commit does touch some fetch-related code\npaths and make them call online_cpus() themselves, so we probably\nare looking at an inadequate tests, and this is a fix that is in\nline of the spirit, completing what 51243f9f started to do.\n\nThanks, will queue.\n"}]}