On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt <j6t@xxxxxxxx> 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. >> >> Cc: Stephen Boyd <bebarino@xxxxxxxxx> >> Signed-off-by: Chris Packham <judge.packham@xxxxxxxxx> >> --- >> 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 > I did notice some oddities when attempting to reproduce this issue. They would be explained by the output fitting into the buffer. So yes perhaps this solution has just changed enough so that it no longer triggers on the particular patch I was testing with. It still seems a bit funny that we start re-reading the input part way through processing it. Perhaps putting the tr outside the whole block would be a better solution? -- 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