Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Sep 5, 2014, 20:26 UTC
- Message-ID
- <540A1C7B.80109@kdbg.org>
- In-Reply-To
- <1409911611-20370-2-git-send-email-judge.packham@gmail.com>
Am 05.09.2014 12:06, schrieb Chris Packham:
Show 32 quoted lines
> 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