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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christian Couder <christian.couder@xxxxxxxxx> writes:

>>> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]