Subject: Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice

From: Junio C Hamano <[email-masked]>
Date: Fri, 05 Sep 2014 15:00:43 -0700

Chris Packham <[email-masked]> writes:

> On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt <[email-masked]> wrote:
>> Am 05.09.2014 12:06, schrieb Chris Packham:
>>> In check_patch_format we feed $1 to a block that attempts to determine
>>> the patch format. Since we've already redirected $1 to stdin there is no
>>> need to redirect it again when we invoke tr. This prevents the following
>>> errors when invoking git am
>>>
>>>   $ git am patch.patch
>>>   tr: write error: Broken pipe
>>>   tr: write error
>>>   Patch format detection failed.
>>> ...
>>
>> I wonder why tr (assuming it is *this* instance of tr) dies with a write
>> error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then
>> the tr invocation inherits this "ignore SIGPIPE" setting?
>> ...
> Perhaps putting the tr outside the whole block would be a better solution?

Perhaps fixing the root cause of your (but not other people's) "tr"
failing is the right solution, no?

Also,

>>> -                     tr -d '\015' <"$1" |
>>>                       sed -n -e '/^$/q' -e '/^[       ]/d' -e p |
>>>                       sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>>>                       patch_format=mbox

as the tr is at an upsteam of this pipeline, it does not really
matter to the outcome if it gives a write-error error message or not
(the downstream sane_egrep would have decided, based on the data it
was given, if the payload was mbox format), so...

An easier workaround may be to update the sed script downstream of
tr.  It stops reading as soon as it finished to save cycles, and tr
should know that it does not have to produce any more output.  For a
broken tr installation, the sed script could be taught to slurp
everything in the message body (without passing it to downstream
sane_egrep, of course), and your "tr" would not see a broken pipe.

But that is still a workaround, not a fix, and an expensive one at
that.