Stefan Beller <sbel...@google.com> writes:
> * If you do not die() in start_failure_fn or return_value_fn, you
> don't want to write to stderr directly as you would destroy the fine
> ordering of the processes output. So make the err strbuf available in
> both these functions, and make sure the strbuf is appended to the
> buffered output in both cases
I think that is a sensible change. Don't we want the same for the
other failure handler, though. Capture any message from it and
append it to the output of the process that just finished, or
something?
By the way, I understand that these two are solely for early review
and I'll be getting them as either new patches or part of updated
patches in the next reroll (i.e. you are not expecting me to split
these apart and do "rebase -i" for you to the last-posted version).
Asking only to make sure we are on the same wavelength.
Thanks.
>
> Signed-off-by: Junio C Hamano <gits...@pobox.com>
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
> run-command.c | 43 ++++++++++++++++++++++++++++++-------------
> run-command.h | 1 +
> 2 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index 494e1f8..0d22291 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -907,6 +907,7 @@ void default_start_failure(void *data,
>
> void default_return_value(void *data,
> struct child_process *cp,
> + struct strbuf *err,
> int result)
> {
> int i;
> @@ -977,7 +978,7 @@ static void set_nonblocking(int fd)
> "output will be degraded");
> }
>
> -/* returns 1 if a process was started, 0 otherwise */
> +/* return 0 if get_next_task() ran out of things to do, non-zero otherwise */
> static int pp_start_one(struct parallel_processes *pp)
> {
> int i;
> @@ -991,26 +992,30 @@ static int pp_start_one(struct parallel_processes *pp)
> if (!pp->get_next_task(pp->data,
> &pp->children[i].process,
> &pp->children[i].err))
> - return 1;
> + return 0;
>
> - if (start_command(&pp->children[i].process))
> + if (start_command(&pp->children[i].process)) {
> pp->start_failure(pp->data,
> &pp->children[i].process,
> &pp->children[i].err);
> + strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
> + strbuf_reset(&pp->children[i].err);
> + return -1;
> + }
>
> set_nonblocking(pp->children[i].process.err);
>
> pp->nr_processes++;
> pp->children[i].in_use = 1;
> pp->pfd[i].fd = pp->children[i].process.err;
> - return 0;
> + return 1;
> }
>
> -static void pp_buffer_stderr(struct parallel_processes *pp)
> +static void pp_buffer_stderr(struct parallel_processes *pp, int
> output_timeout)
> {
> int i;
>
> - while ((i = poll(pp->pfd, pp->max_processes, 100)) < 0) {
> + while ((i = poll(pp->pfd, pp->max_processes, output_timeout)) < 0) {
> if (errno == EINTR)
> continue;
> pp_cleanup(pp);
> @@ -1069,7 +1074,8 @@ static void pp_collect_finished(struct
> parallel_processes *pp)
> error("waitpid is confused (%s)",
> pp->children[i].process.argv[0]);
>
> - pp->return_value(pp->data, &pp->children[i].process, code);
> + pp->return_value(pp->data, &pp->children[i].process,
> + &pp->children[i].err, code);
>
> argv_array_clear(&pp->children[i].process.args);
> argv_array_clear(&pp->children[i].process.env_array);
> @@ -1111,15 +1117,26 @@ int run_processes_parallel(int n, void *data,
> return_value_fn return_value)
> {
> struct parallel_processes pp;
> - pp_init(&pp, n, data, get_next_task, start_failure, return_value);
>
> + pp_init(&pp, n, data, get_next_task, start_failure, return_value);
> while (1) {
> - while (pp.nr_processes < pp.max_processes &&
> - !pp_start_one(&pp))
> - ; /* nothing */
> - if (!pp.nr_processes)
> + int no_more_task, cnt;
> + int output_timeout = 100;
> + int spawn_cap = 4;
> +
> + for (cnt = spawn_cap, no_more_task = 0;
> + cnt && pp.nr_processes < pp.max_processes;
> + cnt--) {
> + if (!pp_start_one(&pp)) {
> + no_more_task = 1;
> + break;
> + }
> + }
> +
> + if (no_more_task && !pp.nr_processes)
> break;
> - pp_buffer_stderr(&pp);
> + pp_buffer_stderr(&pp, output_timeout);
> +
> pp_output(&pp);
> pp_collect_finished(&pp);
> }
> diff --git a/run-command.h b/run-command.h
> index 3807fd1..f7035cb 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -138,6 +138,7 @@ typedef void (*start_failure_fn)(void *data,
>
> typedef void (*return_value_fn)(void *data,
> struct child_process *cp,
> + struct strbuf *err,
> int result);
>
> /**
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
