Hi Chris,

On Wed, 11 May 2016, Christian Couder wrote:

I consider that the apply functionality is properly libified before
these patches, and that they should be in a separate series, but
unfortunately using the libified apply in "git am" unmasks the fact that
"git am", since it was a shell script, has been silencing the apply
functionality by "futzing with file descriptors". And unfortunately this
makes some reviewers unhappy.

It is a misrepresentation to claim that this makes some reviewers unhappy.
Speaking for myself, I am very happy. Despite having had to point out
that the previous iteration of this patch series had a serious flaw.

It is also incorrect to say that the shell script had been "futzing with
the file descriptors". You see, the shell script's *own* file descriptors
had been left completely unaffected by the redirection of the spawned
process' output. That was perfectly fine a thing to do, even if it
possibly hid fatal errors. Shell scripts are simply very limiting. The
problem was introduced by v1 of this patch series, which changed *the
caller's file descriptors* back and forth simply because the called code
no longer runs in a separate process. And *that* was, and is, improper.

By the way there are no tests yet for this new feature, and I am not
sure at all that "--silent" and "be_silent" are good names.

If you want to follow existing code's example, we typically call this
option "quiet".

Sorry if this patch series is long. I can split it into two or more
series if it is prefered.

It is preferred. Much.

Ciao,
Dscho

P.S.: Even two ~40-strong patch series are *really* painful to review. I
believe you can do a much better job at cutting this monster down into
palatable chunks, each with its own sweet little story.