Re: [PATCH v7 14/17] run-command: add stdin callback for parallelization

On Sun, Jan 31, 2021 at 10:51:53PM -0800, Jonathan Tan wrote:

quoted

If a user of the run_processes_parallel() API wants to pipe a large
amount of information to stdin of each parallel command, that
information could exceed the buffer of the pipe allocated for that
process's stdin.  Generally this is solved by repeatedly writing to
child_process.in between calls to start_command() and finish_command();
run_processes_parallel() did not provide users an opportunity to access
child_process at that time.

[snip]

quoted

diff --git a/run-command.h b/run-command.h
index 6472b38bde..e058c0e2c8 100644
--- a/run-command.h
+++ b/run-command.h

@@ -436,6 +436,20 @@ typedef int (*start_failure_fn)(struct strbuf *out,
 				void *pp_cb,
 				void *pp_task_cb);
 
+/**
+ * This callback is called repeatedly on every child process who requests
+ * start_command() to create a pipe by setting child_process.in < 0.
+ *
+ * pp_cb is the callback cookie as passed into run_processes_parallel, and
+ * pp_task_cb is the callback cookie as passed into get_next_task_fn.
+ * The contents of 'send' will be read into the pipe and passed to the pipe.
+ *
+ * Return nonzero to close the pipe.
+ */
+typedef int (*feed_pipe_fn)(struct strbuf *pipe,
+			    void *pp_cb,
+			    void *pp_task_cb);
+

As you mention above in the commit message, I think the clearest API to
support what we need is to just have a callback (that has access to
child_process) that is executed between process start and finish.

As it is, I think this callback is too specific in that it takes a
struct strbuf. I think that this struct strbuf will just end up being
unnecessary copying much of the time, when the user could have just
written to the fd directly.

Since the rest of the run_processes_parallel() API passes strings around
with strbufs, I'd prefer to leave it as-is to match the general API
expectations and style.

 - Emily