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

> 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.
>>
>> Cc: Stephen Boyd <bebarino@gmail.com>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> ---
>> Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425
>>
>>  git-am.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-am.sh b/git-am.sh
>> index ee61a77..fade7f8 100755
>> --- a/git-am.sh
>> +++ b/git-am.sh
>> @@ -250,7 +250,7 @@ check_patch_format () {
>>                       # discarding the indented remainder of folded lines,
>>                       # and see if it looks like that they all begin with the
>>                       # header field names...
>> -                     tr -d '\015' <"$1" |
>> +                     tr -d '\015' |
>>                       sed -n -e '/^$/q' -e '/^[       ]/d' -e p |
>>                       sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
>>                       patch_format=mbox
>>
>
> I think this change is wrong. This pipeline checks whether one of the
> lines at the top of the file contains something that looks like an email
> header. With your change, the first three lines would not be looked at
> because they were already consumed earlier.
>
> 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?
>
> The only thing your version changes is that tr writes a bit less text
> into the pipe. Perhaps its just sufficient that the output fits into the
> pipe buffer, and no error occurs anymore? Then the new version is not a
> real fix: make the patch text a bit longer, and the error is back.
>
> -- Hannes
>