Re: [PATCH v2 00/94] libify apply and use lib in am

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 13, 2016, 06:32 UTC
Message-ID: <alpine.DEB.2.20.1605130820160.4092@virtualbox>
In-Reply-To: <20160511131745.2914-1-chriscool@tuxfamily.org>

Hi Chris,

On Wed, 11 May 2016, Christian Couder wrote:

Show 6 quoted lines

> 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.