Re: [PATCH 80/83] run-command: make dup_devnull() non static

Duy Nguyen [off-list ref] writes:

I vote one step at a time, leave multi-thread support for future.
There's a lot more shared state than file descriptors anyway, at least
there are object db and index access and probably a couple of hidden
static variables somewhere. And I'm not sure if multi-thread really
helps here. Are we really CPU-bound? If object inflation causes that
(wild guess), can we just inflate ahead in some separate process and
pass the result back?

I do not particularly care about multi-thread issues, but I have to
agree with Dscho that the updated code that claims to be "libified"
that futz with the file descriptors like the way this patch does is
not a proper libification.

Unfortunately, the anticipated caller of this code that does "this
may fail and it is OK because it is merely one of the attempts, so
let's not show the errors" is not something we call only when we are
falling back, so "why not do this rare codepath via the usual
run_command() interface to spawn 'apply' as a separate process?",
which would be the most sensible "one step at a time" suggestion if
it were the case, would not apply.

As you will be passing the apply state structure throughout the
callchain, would it be a viable and reasonable endgame state to have
a strbuf in it that accumulates the errors?  That is, instead of
dup()ing the standard error stream out, you would accumulate the
errors for a caller that asks their errors not directly sent to
the standard error stream, so that it can choose to either show it
at the end, or ignore it altogether.

How far can you go with just set-error-routine?  Are there things,
other than the file descriptors, that you need to futz with in order
to covert that "we'd fallback, so this early round must be silent"
codepath?