An earlier version of that series simply changed the API to pass an
"opts" struct instead:
https://lore.kernel.org/git/patch-v2-2.8-5f0a6e9925f-20220518T195858Z-avarab@gmail.com/ (local)

I really should have submitted those post-release cleanup patches
already, and I'm not sure whether the right thing at this point is to
take this & do the cleanup for "ungroup" *and* this new argument later.

But maybe you're interested in cherry-picking & adjusting the relevant
part of that series for this one? I.e. we're not in some post-release
regression hurry, so rather than extending the use of this odd interface
we could (and maybe should) just fix how we're doing it first.

I'll go ahead and give this a try. I was also a little bit surprised that
"ungroup" was set this way, but didn't realize it was for a quick fix.

On the implementation:

quoted

+ * If the "pipe_output" option is specified, the output will be piped
+ * to task_finished_fn in the "struct strbuf *out" variable. The output
+ * will still be printed unless the callback resets the strbuf. The
+ * "pipe_output" option can be enabled by setting the global
+ * "run_processes_parallel_pipe_output" to "1" before invoking
+ * run_processes_parallel(), it will be set back to "0" as soon as the
+ * API reads that setting.

...okey, but...

quoted

+static int task_finished_pipe_output(int result,
+                      struct strbuf *err,
+                      void *pp_cb,
+                      void *pp_task_cb)
+{
+     if (err && pipe_output) {
+             fprintf(stderr, "%s", err->buf);
+             strbuf_reset(err);

...my memory's hazy, and I haven't re-logged in any detail, but is it
really the API interface here that the "output" callback function is
responsible for resetting the strbuf that the API gives to it?

That seems backwards to me, and e.g. a look at "start_failure" shows
that we strbuf_reset() the "err".

What's the point of doing it in the API consumer? If it doesn't do it
we'll presumably keep accumulating output. Is there a use-case for that?

Or perhaps it's not needed & this is really just misleading boilerplate?

Ultimately it is not needed -- I added it as an example to showcase that
the output is correctly being piped to "task_finished_pipe_output". The
reset is necessary in this case to prevent the output from being printed
twice. I'm not sure how exactly else I would go about testing "pipe_output".

quoted

@@ -140,6 +140,11 @@ test_expect_success 'run_command runs ungrouped in parallel with more jobs avail
      test_line_count = 4 err
 '

+test_expect_success 'run_command runs pipe_output in parallel with more jobs available than tasks' '
+     test-tool run-command --pipe-output run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual &&
+     test_cmp expect actual
+'
+

Like the global argument, the copy/pasting for "ungroup" was mostly a
matter of expediency.

But at least in that case we have a different assertion (test_cmp
v.s. test_line_count).

But here this test case seems to be exactly the same as for the
"vanilla" version.

So can't we make this some:

        for opt in '' '--pipe-output'
        do
                test_expect_success ...
        done

?

Yes we can -- but I may need to rethink how instead I should be testing
this option?