Skip to content

Commit

Permalink
run-command API: don't fall back on online_cpus()
Browse files Browse the repository at this point in the history
When a "jobs = 0" is passed let's BUG() out rather than fall back on
online_cpus(). The default behavior was added when this API was
implemented in c553c72 (run-command: add an asynchronous parallel
child processor, 2015-12-15).

Most of our code in-tree that scales up to "online_cpus()" by default
calls that function by itself. Keeping this default behavior just for
the sake of two callers means that we'd need to maintain this one spot
where we're second-guessing the config passed down into pp_init().

The preceding commit has an overview of the API callers that passed
"jobs = 0". There were only two of them (actually three, but they
resolved to these two config parsing codepaths).

The "fetch.parallel" caller already had a test for the
"fetch.parallel=0" case added in 0353c68 (fetch: do not run a
redundant fetch from submodule, 2022-05-16), but there was no such
test for "submodule.fetchJobs". Let's add one here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
avar authored and gitster committed Oct 12, 2022
1 parent 6a48b42 commit 51243f9
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 5 deletions.
2 changes: 2 additions & 0 deletions builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
fetch_parallel_config = git_config_int(k, v);
if (fetch_parallel_config < 0)
die(_("fetch.parallel cannot be negative"));
if (!fetch_parallel_config)
fetch_parallel_config = online_cpus();
return 0;
}

Expand Down
7 changes: 3 additions & 4 deletions run-command.c
Original file line number Diff line number Diff line change
Expand Up @@ -1564,8 +1564,8 @@ static void pp_init(struct parallel_processes *pp,
task_finished_fn task_finished,
void *data, int ungroup)
{
if (n < 1)
n = online_cpus();
if (!n)
BUG("you must provide a non-zero number of processes!");

pp->max_processes = n;

Expand Down Expand Up @@ -1835,8 +1835,7 @@ void run_processes_parallel_tr2(size_t n, get_next_task_fn get_next_task,
task_finished_fn task_finished, void *pp_cb,
const char *tr2_category, const char *tr2_label)
{
trace2_region_enter_printf(tr2_category, tr2_label, NULL, "max:%d",
((n < 1) ? online_cpus() : n));
trace2_region_enter_printf(tr2_category, tr2_label, NULL, "max:%d", n);

run_processes_parallel(n, get_next_task, start_failure,
task_finished, pp_cb);
Expand Down
2 changes: 2 additions & 0 deletions submodule-config.c
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ int parse_submodule_fetchjobs(const char *var, const char *value)
int fetchjobs = git_config_int(var, value);
if (fetchjobs < 0)
die(_("negative values not allowed for submodule.fetchJobs"));
if (!fetchjobs)
fetchjobs = online_cpus();
return fetchjobs;
}

Expand Down
8 changes: 7 additions & 1 deletion t/t5526-fetch-submodules.sh
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,13 @@ test_expect_success 'fetching submodules respects parallel settings' '
GIT_TRACE=$(pwd)/trace.out git fetch &&
grep "8 tasks" trace.out &&
GIT_TRACE=$(pwd)/trace.out git fetch --jobs 9 &&
grep "9 tasks" trace.out
grep "9 tasks" trace.out &&
>trace.out &&
GIT_TRACE=$(pwd)/trace.out git -c submodule.fetchJobs=0 fetch &&
grep "preparing to run up to [0-9]* tasks" trace.out &&
! grep "up to 0 tasks" trace.out &&
>trace.out
)
'

Expand Down

0 comments on commit 51243f9

Please sign in to comment.