diff --git a/run-command.c b/run-command.c index a8501e38ce..7ab2dd28f3 100644 --- a/run-command.c +++ b/run-command.c @@ -1471,6 +1471,7 @@ enum child_state { GIT_CP_WAIT_CLEANUP, }; +int run_processes_parallel_ungroup; struct parallel_processes { void *data; @@ -1494,6 +1495,7 @@ struct parallel_processes { struct pollfd *pfd; unsigned shutdown : 1; + unsigned ungroup : 1; int output_owner; struct strbuf buffered_output; /* of finished children */ @@ -1537,7 +1539,7 @@ static void pp_init(struct parallel_processes *pp, get_next_task_fn get_next_task, start_failure_fn start_failure, task_finished_fn task_finished, - void *data) + void *data, int ungroup) { int i; @@ -1559,15 +1561,21 @@ static void pp_init(struct parallel_processes *pp, pp->nr_processes = 0; pp->output_owner = 0; pp->shutdown = 0; + pp->ungroup = ungroup; CALLOC_ARRAY(pp->children, n); - CALLOC_ARRAY(pp->pfd, n); + if (pp->ungroup) + pp->pfd = NULL; + else + CALLOC_ARRAY(pp->pfd, n); strbuf_init(&pp->buffered_output, 0); for (i = 0; i < n; i++) { strbuf_init(&pp->children[i].err, 0); child_process_init(&pp->children[i].process); - pp->pfd[i].events = POLLIN | POLLHUP; - pp->pfd[i].fd = -1; + if (pp->pfd) { + pp->pfd[i].events = POLLIN | POLLHUP; + pp->pfd[i].fd = -1; + } } pp_for_signal = pp; @@ -1615,24 +1623,31 @@ static int pp_start_one(struct parallel_processes *pp) BUG("bookkeeping is hard"); code = pp->get_next_task(&pp->children[i].process, - &pp->children[i].err, + pp->ungroup ? NULL : &pp->children[i].err, pp->data, &pp->children[i].data); if (!code) { - strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); - strbuf_reset(&pp->children[i].err); + if (!pp->ungroup) { + strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); + strbuf_reset(&pp->children[i].err); + } return 1; } - pp->children[i].process.err = -1; - pp->children[i].process.stdout_to_stderr = 1; + if (!pp->ungroup) { + pp->children[i].process.err = -1; + pp->children[i].process.stdout_to_stderr = 1; + } pp->children[i].process.no_stdin = 1; if (start_command(&pp->children[i].process)) { - code = pp->start_failure(&pp->children[i].err, + code = pp->start_failure(pp->ungroup ? NULL : + &pp->children[i].err, pp->data, pp->children[i].data); - strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); - strbuf_reset(&pp->children[i].err); + if (!pp->ungroup) { + strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); + strbuf_reset(&pp->children[i].err); + } if (code) pp->shutdown = 1; return code; @@ -1640,7 +1655,8 @@ static int pp_start_one(struct parallel_processes *pp) pp->nr_processes++; pp->children[i].state = GIT_CP_WORKING; - pp->pfd[i].fd = pp->children[i].process.err; + if (pp->pfd) + pp->pfd[i].fd = pp->children[i].process.err; return 0; } @@ -1674,6 +1690,7 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout) static void pp_output(struct parallel_processes *pp) { int i = pp->output_owner; + if (pp->children[i].state == GIT_CP_WORKING && pp->children[i].err.len) { strbuf_write(&pp->children[i].err, stderr); @@ -1696,7 +1713,7 @@ static int pp_collect_finished(struct parallel_processes *pp) code = finish_command(&pp->children[i].process); - code = pp->task_finished(code, + code = pp->task_finished(code, pp->ungroup ? NULL : &pp->children[i].err, pp->data, pp->children[i].data); @@ -1707,10 +1724,13 @@ static int pp_collect_finished(struct parallel_processes *pp) pp->nr_processes--; pp->children[i].state = GIT_CP_FREE; - pp->pfd[i].fd = -1; + if (pp->pfd) + pp->pfd[i].fd = -1; child_process_init(&pp->children[i].process); - if (i != pp->output_owner) { + if (pp->ungroup) { + ; /* no strbuf_*() work to do here */ + } else if (i != pp->output_owner) { strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); strbuf_reset(&pp->children[i].err); } else { @@ -1747,9 +1767,14 @@ int run_processes_parallel(int n, int i, code; int output_timeout = 100; int spawn_cap = 4; + int ungroup = run_processes_parallel_ungroup; struct parallel_processes pp; - pp_init(&pp, n, get_next_task, start_failure, task_finished, pp_cb); + /* unset for the next API user */ + run_processes_parallel_ungroup = 0; + + pp_init(&pp, n, get_next_task, start_failure, task_finished, pp_cb, + ungroup); while (1) { for (i = 0; i < spawn_cap && !pp.shutdown && @@ -1766,8 +1791,15 @@ int run_processes_parallel(int n, } if (!pp.nr_processes) break; - pp_buffer_stderr(&pp, output_timeout); - pp_output(&pp); + if (ungroup) { + int i; + + for (i = 0; i < pp.max_processes; i++) + pp.children[i].state = GIT_CP_WAIT_CLEANUP; + } else { + pp_buffer_stderr(&pp, output_timeout); + pp_output(&pp); + } code = pp_collect_finished(&pp); if (code) { pp.shutdown = 1; diff --git a/run-command.h b/run-command.h index 07bed6c31b..21f1ac1c36 100644 --- a/run-command.h +++ b/run-command.h @@ -406,6 +406,9 @@ void check_pipe(int err); * pp_cb is the callback cookie as passed to run_processes_parallel. * You can store a child process specific callback cookie in pp_task_cb. * + * See run_processes_parallel() below for a discussion of the "struct + * strbuf *out" parameter. + * * Even after returning 0 to indicate that there are no more processes, * this function will be called again until there are no more running * child processes. @@ -424,9 +427,8 @@ typedef int (*get_next_task_fn)(struct child_process *cp, * This callback is called whenever there are problems starting * a new process. * - * You must not write to stdout or stderr in this function. Add your - * message to the strbuf out instead, which will be printed without - * messing up the output of the other parallel processes. + * See run_processes_parallel() below for a discussion of the "struct + * strbuf *out" parameter. * * pp_cb is the callback cookie as passed into run_processes_parallel, * pp_task_cb is the callback cookie as passed into get_next_task_fn. @@ -442,9 +444,8 @@ typedef int (*start_failure_fn)(struct strbuf *out, /** * This callback is called on every child process that finished processing. * - * You must not write to stdout or stderr in this function. Add your - * message to the strbuf out instead, which will be printed without - * messing up the output of the other parallel processes. + * See run_processes_parallel() below for a discussion of the "struct + * strbuf *out" parameter. * * pp_cb is the callback cookie as passed into run_processes_parallel, * pp_task_cb is the callback cookie as passed into get_next_task_fn. @@ -465,11 +466,26 @@ typedef int (*task_finished_fn)(int result, * * The children started via this function run in parallel. Their output * (both stdout and stderr) is routed to stderr in a manner that output - * from different tasks does not interleave. + * from different tasks does not interleave (but see "ungroup" below). * * start_failure_fn and task_finished_fn can be NULL to omit any * special handling. + * + * If the "ungroup" option isn't specified, the API will set the + * "stdout_to_stderr" parameter in "struct child_process" and provide + * the callbacks with a "struct strbuf *out" parameter to write output + * to. In this case the callbacks must not write to stdout or + * stderr as such output will mess up the output of the other parallel + * processes. If "ungroup" option is specified callbacks will get a + * NULL "struct strbuf *out" parameter, and are responsible for + * emitting their own output, including dealing with any race + * conditions due to writing in parallel to stdout and stderr. + * The "ungroup" option can be enabled by setting the global + * "run_processes_parallel_ungroup" to "1" before invoking + * run_processes_parallel(), it will be set back to "0" as soon as the + * API reads that setting. */ +extern int run_processes_parallel_ungroup; int run_processes_parallel(int n, get_next_task_fn, start_failure_fn, diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index f3b90aa834..34cce45b58 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -31,7 +31,11 @@ static int parallel_next(struct child_process *cp, return 0; strvec_pushv(&cp->args, d->args.v); - strbuf_addstr(err, "preloaded output of a child\n"); + if (err) + strbuf_addstr(err, "preloaded output of a child\n"); + else + fprintf(stderr, "preloaded output of a child\n"); + number_callbacks++; return 1; } @@ -41,7 +45,10 @@ static int no_job(struct child_process *cp, void *cb, void **task_cb) { - strbuf_addstr(err, "no further jobs available\n"); + if (err) + strbuf_addstr(err, "no further jobs available\n"); + else + fprintf(stderr, "no further jobs available\n"); return 0; } @@ -50,7 +57,10 @@ static int task_finished(int result, void *pp_cb, void *pp_task_cb) { - strbuf_addstr(err, "asking for a quick stop\n"); + if (err) + strbuf_addstr(err, "asking for a quick stop\n"); + else + fprintf(stderr, "asking for a quick stop\n"); return 1; } @@ -407,6 +417,12 @@ int cmd__run_command(int argc, const char **argv) if (!strcmp(argv[1], "run-command")) exit(run_command(&proc)); + if (!strcmp(argv[1], "--ungroup")) { + argv += 1; + argc -= 1; + run_processes_parallel_ungroup = 1; + } + jobs = atoi(argv[2]); strvec_clear(&proc.args); strvec_pushv(&proc.args, (const char **)argv + 3); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index ee281909bc..7b5423eebd 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -134,16 +134,34 @@ test_expect_success 'run_command runs in parallel with more jobs available than test_cmp expect actual ' +test_expect_success 'run_command runs ungrouped in parallel with more jobs available than tasks' ' + test-tool run-command --ungroup run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_line_count = 8 out && + test_line_count = 4 err +' + test_expect_success 'run_command runs in parallel with as many jobs as tasks' ' test-tool run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && test_cmp expect actual ' +test_expect_success 'run_command runs ungrouped in parallel with as many jobs as tasks' ' + test-tool run-command --ungroup run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_line_count = 8 out && + test_line_count = 4 err +' + test_expect_success 'run_command runs in parallel with more tasks than jobs available' ' test-tool run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && test_cmp expect actual ' +test_expect_success 'run_command runs ungrouped in parallel with more tasks than jobs available' ' + test-tool run-command --ungroup run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_line_count = 8 out && + test_line_count = 4 err +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop @@ -158,6 +176,12 @@ test_expect_success 'run_command is asked to abort gracefully' ' test_cmp expect actual ' +test_expect_success 'run_command is asked to abort gracefully (ungroup)' ' + test-tool run-command --ungroup run-command-abort 3 false >out 2>err && + test_must_be_empty out && + test_line_count = 6 err +' + cat >expect <<-EOF no further jobs available EOF @@ -167,6 +191,12 @@ test_expect_success 'run_command outputs ' ' test_cmp expect actual ' +test_expect_success 'run_command outputs (ungroup) ' ' + test-tool run-command --ungroup run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_cmp expect err +' + test_trace () { expect="$1" shift