Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 5, 2014, 22:00 UTC
- Message-ID
- <xmqqoautpw1g.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <CAFOYHZBct1CRA+NumVMvbbuELWTRoGL5FkhBfHD2Wk7QZVe1fA@mail.gmail.com>
Chris Packham <judge.packham@gmail.com> writes:
Show 18 quoted lines
> On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt <j6t@kdbg.org> 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.