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

From: Junio C Hamano <gitster@pobox.com>
Date: May 14, 2016, 18:31 UTC
Message-ID: <xmqqwpmw7aow.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAP8UFD376guRRPpMVjC36K0dvWy17xdaHvV1WoRjMaNpdp0bqQ@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 20 quoted lines

>>> So it looks to me that --quiet means something like "don't tell the
>>> story of your life, but in case of problem you are allowed to
>>> complain". In other word --quiet generally doesn't suppress error
>>> messages from error() or die().
>>
>> Right.
>>
>> And if you care to take a step back, this is most likely what we want in
>> libified code.
>
> In the previous discussion, Junio asked:
>
> "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?"
>
> So it looks to me that the goal is to have something that replicate
> the current behavior, which is to not even display messages from
> error().

Note that I was not endorsing the approach to use set-error-routine, one of whose downside is that it is easy to silence everything unconditionally. Between --quiet/--silent, I wasn't making any choice, as I hadn't even formed an opinion back when I wrote it.

I was merely asking "is use of set-error-routine the worst thing you need to do, or do you need to do anything more gross?", half anticipating "no, it turns out that I need to longjmp(3) from my die-routine to come back from deep in the existing code because I do not think I'd bother propagating the error status all the way along the call stack", at which point my response would have been "Ugh, use of set-error-routine to avoid properly refactoring, even if we assume it is OK to squelch the errors unconditionally, is bad enough. If you need to longjmp(3), that's no longer a libification. You are better off using the run_command() interface to give the caller and the callee a proper isolation".

Libification is not just "now it runs inside a single process" at all. It is more about "it has proper abstraction and separation so that callers have _more_ control than they used to (compared to interacting with a spawned process) while retaining the control of their own environment".

And as this thread showed, when turning an implementation that uses run-command with program's standard error sent to /dev/null into a "libified" implementation, we _gain_ a possibility to be more selective in what we silence. I think that exactly falls into "callers have more control" category of libification benefit.

Quite honestly, I was sort of surprised by the quality of the argument for proper libification Dscho has been making in this thread, and that was why I didn't say much here.

Having said all that, as to --quiet/--silent, another way to surface the "even more quiet" used is with "-q -q" (i.e. multiple levels of quietness). I am just saying this for the record without suggesting it is better or worse suited for this case than what has been discussed.

In any case, I do see the need for what your --silent option does internally when a caller like "am" calls into the libified "apply" machinery. When we _know_ we will run a "fallback" invocation after an initial "more strict" apply attempt fails, we do want the initial invocation to be totally silent, not even with any error message, because we know that it is sufficient to let the fallback invocation show its error message (if it fails). The fallback may even succeed in which case error messages from the initial invocation serves only as an explanation of the reason why we used fallback, which is not interesting to the end user at all.

I however do not see a reason why you need to expose that feature to the users of "git apply". So I am not sure if any of us care deeply the choice among --silent, --quiet and -q -q.