run-command: allow capturing of collated output

Some callers, for example server-side hooks which wish to relay hook
output to clients across a transport, want to capture what would
normally print to stderr and do something else with it. Allow that via a
callback.

By calling the callback regardless of whether there's output available,
we allow clients to send e.g. a keepalive if necessary.

Because we expose a strbuf, not a fd or FILE*, there's no need to create
a temporary pipe or similar - we can just skip the print to stderr and
instead hand it to the caller.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/run-command.c b/run-command.c
index a608d37fb22d983bc0431ae861a6cc1ba2f5a9bb..6b1e4a34533315e8de815d9942d789e53fc76333 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -1595,7 +1595,10 @@ static void pp_cleanup(struct parallel_processes *pp,
         * When get_next_task added messages to the buffer in its last
         * iteration, the buffered output is non empty.
         */
-       strbuf_write(&pp->buffered_output, stderr);
+       if (opts->consume_output)
+               opts->consume_output(&pp->buffered_output, opts->data);
+       else
+               strbuf_write(&pp->buffered_output, stderr);
        strbuf_release(&pp->buffered_output);
 
        sigchain_pop_common();
@@ -1734,13 +1737,17 @@ static void pp_buffer_stderr(struct parallel_processes *pp,
        }
 }
 
-static void pp_output(const struct parallel_processes *pp)
+static void pp_output(const struct parallel_processes *pp,
+                     const struct run_process_parallel_opts *opts)
 {
        size_t i = pp->output_owner;
 
        if (child_is_working(&pp->children[i]) &&
            pp->children[i].err.len) {
-               strbuf_write(&pp->children[i].err, stderr);
+               if (opts->consume_output)
+                       opts->consume_output(&pp->children[i].err, opts->data);
+               else
+                       strbuf_write(&pp->children[i].err, stderr);
                strbuf_reset(&pp->children[i].err);
        }
 }
@@ -1788,11 +1795,15 @@ static int pp_collect_finished(struct parallel_processes *pp,
                } else {
                        const size_t n = opts->processes;
 
-                       strbuf_write(&pp->children[i].err, stderr);
+                       /* Output errors, then all other finished child processes */
+                       if (opts->consume_output) {
+                               opts->consume_output(&pp->children[i].err, opts->data);
+                               opts->consume_output(&pp->buffered_output, opts->data);
+                       } else {
+                               strbuf_write(&pp->children[i].err, stderr);
+                               strbuf_write(&pp->buffered_output, stderr);
+                       }
                        strbuf_reset(&pp->children[i].err);
-
-                       /* Output all other finished child processes */
-                       strbuf_write(&pp->buffered_output, stderr);
                        strbuf_reset(&pp->buffered_output);
 
                        /*
@@ -1829,7 +1840,7 @@ static void pp_handle_child_IO(struct parallel_processes *pp,
                                pp->children[i].state = GIT_CP_WAIT_CLEANUP;
        } else {
                pp_buffer_stderr(pp, opts, output_timeout);
-               pp_output(pp);
+               pp_output(pp, opts);
        }
 }
 
@@ -1852,6 +1863,9 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
                                           "max:%"PRIuMAX,
                                           (uintmax_t)opts->processes);
 
+       if (opts->ungroup && opts->consume_output)
+               BUG("ungroup and reading output are mutualy exclusive");
+
        /*
         * Child tasks might receive input via stdin, terminating early (or not), so
         * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which

diff --git a/run-command.h b/run-command.h
index e1ca965b5b19882ef1c16b9407eec13ff3e926fe..7093252863966fa7c476b15d8a38be961224e855 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -435,6 +435,17 @@ typedef int (*feed_pipe_fn)(int child_in,
                                void *pp_cb,
                                void *pp_task_cb);
 
+/**
+ * If this callback is provided, output is collated into a new pipe instead
+ * of the process stderr. Then `consume_output_fn` will be called repeatedly
+ * with output contained in the `output` arg. It will also be called with an
+ * empty `output` to allow for keepalives or similar operations if necessary.
+ *
+ * pp_cb is the callback cookie as passed into run_processes_parallel.
+ * No task cookie is provided because the callback receives collated output.
+ */
+typedef void (*consume_output_fn)(struct strbuf *output, void *pp_cb);
+
 /**
  * This callback is called on every child process that finished processing.
  *
@@ -494,6 +505,12 @@ struct run_process_parallel_opts
         */
        feed_pipe_fn feed_pipe;
 
+       /*
+        * consume_output: see consume_output_fn() above. This can be NULL
+        * to omit any special handling.
+        */
+       consume_output_fn consume_output;
+
        /**
         * task_finished: See task_finished_fn() above. This can be
         * NULL to omit any special handling.

diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 4a56456894ccff9b38aa5bfb3c286897083ca6a5..49eace8dce196167784ed2187561072324a80086 100644 (file)
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -58,6 +58,16 @@ static int no_job(struct child_process *cp UNUSED,
        return 0;
 }
 
+static void test_divert_output(struct strbuf *output, void *cb UNUSED)
+{
+       FILE *output_file;
+
+       output_file = fopen("./output_file", "a");
+
+       strbuf_write(output, output_file);
+       fclose(output_file);
+}
+
 static int task_finished(int result UNUSED,
                         struct strbuf *err,
                         void *pp_cb UNUSED,
@@ -198,6 +208,7 @@ static int testsuite(int argc, const char **argv)
                .get_next_task = next_test,
                .start_failure = test_failed,
                .feed_pipe = test_stdin_pipe_feed,
+               .consume_output = test_divert_output,
                .task_finished = test_finished,
                .data = &suite,
        };
@@ -514,6 +525,10 @@ int cmd__run_command(int argc, const char **argv)
                opts.get_next_task = parallel_next;
                opts.task_finished = task_finished_quiet;
                opts.feed_pipe = test_stdin_pipe_feed;
+       } else if (!strcmp(argv[1], "run-command-divert-output")) {
+               opts.get_next_task = parallel_next;
+               opts.consume_output = test_divert_output;
+               opts.task_finished = task_finished_quiet;
        } else {
                ret = 1;
                fprintf(stderr, "check usage\n");

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 2f77fde0d964c8afd2bd175b77263226aea92d2f..74529e219e2aefaa26cf6dcaa2163a12d2af6ff4 100755 (executable)
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -164,6 +164,13 @@ test_expect_success 'run_command runs ungrouped in parallel with more tasks than
        test_line_count = 4 err
 '
 
+test_expect_success 'run_command can divert output' '
+       test_when_finished rm output_file &&
+       test-tool run-command run-command-divert-output 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual &&
+       test_must_be_empty actual &&
+       test_cmp expect output_file
+'
+
 test_expect_success 'run_command listens to stdin' '
        cat >expect <<-\EOF &&
        preloaded output of a child