The callbacks in the parallel processing API were given the contract, that
they are not allowed to print anything to stdout/err, but the API will take
care of their output needs instead.

In case a child process is started, the callback can first add its messages
to the buffer and then the child process output is buffered just in the
same buffer, and so the output will be taken care of eventually once the
child process is done.

When no child process is run, we also need to fulfill our promise to
output the buffer eventually. So when no child process is started, we need
to amend the contents of the buffer passed to the child to our buffer for
finished processes. We cannot output directly at that point in time as
another process may be in the middle of its output.

The buffer for finished child processes then also needs to be flushed in
the cleanup phase as it may contain data.

Signed-off-by: Stefan Beller <sbel...@google.com>
---

Another issue I just found. This goes on top of the series
    [PATCH 0/5] Fixes for the parallel processing
    
Thanks,
Stefan

 run-command.c          | 11 ++++++++++-
 t/t0061-run-command.sh |  9 +++++++++
 test-run-command.c     | 13 +++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 108b930..707e0a6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -979,6 +979,12 @@ static void pp_cleanup(struct parallel_processes *pp)
 
        free(pp->children);
        free(pp->pfd);
+
+       /*
+        * When get_next_task added messages to the buffer in its last
+        * iteration, the buffered output is non empty.
+        */
+       fputs(pp->buffered_output.buf, stderr);
        strbuf_release(&pp->buffered_output);
 
        sigchain_pop_common();
@@ -1016,8 +1022,11 @@ static int pp_start_one(struct parallel_processes *pp)
        if (!pp->get_next_task(&pp->children[i].data,
                               &pp->children[i].process,
                               &pp->children[i].err,
-                              pp->data))
+                              pp->data)) {
+               strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
+               strbuf_reset(&pp->children[i].err);
                return 1;
+       }
 
        if (start_command(&pp->children[i].process)) {
                int code = pp->start_failure(&pp->children[i].process,
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index f27ada7..12228b4 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -91,4 +91,13 @@ test_expect_success 'run_command is asked to abort 
gracefully' '
        test_cmp expect actual
 '
 
+cat >expect <<-EOF
+no further jobs available
+EOF
+
+test_expect_success 'run_command outputs ' '
+       test-run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello 
World" 2>actual &&
+       test_cmp expect actual
+'
+
 test_done
diff --git a/test-run-command.c b/test-run-command.c
index 44710c3..91e94e8 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -34,6 +34,15 @@ static int parallel_next(void** task_cb,
        return 1;
 }
 
+static int no_job(void** task_cb,
+                 struct child_process *cp,
+                 struct strbuf *err,
+                 void *cb)
+{
+       strbuf_addf(err, "no further jobs available\n");
+       return 0;
+}
+
 static int task_finished(int result,
                         struct child_process *cp,
                         struct strbuf *err,
@@ -73,6 +82,10 @@ int main(int argc, char **argv)
                exit(run_processes_parallel(jobs, parallel_next,
                                            NULL, task_finished, &proc));
 
+       if (!strcmp(argv[1], "run-command-no-jobs"))
+               exit(run_processes_parallel(jobs, no_job,
+                                           NULL, task_finished, &proc));
+
        fprintf(stderr, "check usage\n");
        return 1;
 }
-- 
2.5.0.273.gfd9d19f.dirty

--
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

Reply via email to