Re: [PATCH] Don't ignore write failure from git-diff, git-log, etc.

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: May 27, 2007, 16:17 UTC
Message-ID: <alpine.LFD.0.98.0705270904240.26602@woody.linux-foundation.org>
In-Reply-To: <87odk6y6cd.fsf@rho.meyering.net>

On Sun, 27 May 2007, Jim Meyering wrote:

> 
> I have to disagree.  There may be precedent for hiding EPIPE errors,
> but that is not the norm among command line tools wrt piped stdout.

.. and this is a PROBLEM. Which is why I think your patch was really wrong.

I don't know how many people remember all the _stupid_ problems we had exactly because many versions of bash are crap, crap, crap, and people (including you) don't realize that EPIPE is _different_ from other write errors.

Just do a google search for

	"broken pipe" bash

and not only will you see a lot of complaints, but the #5 entry is a complaint for a git issue that we had tons of problems with. See for example

	http://www.gelato.unsw.edu.au/archives/git/0504/2602.html

The reason? Some _idiotic_ versions of bash don't have DONT_REPORT_SIGPIPE on by default.

So I do get upset when people then make the same error with git.

Show 10 quoted lines

> Do you really want git-log to continue to do this?
> 
>     $ (trap '' PIPE; git-log; echo $? >&2 ) | :
>     0
> 
> With my patch, it does this:
> 
>     $ (trap '' PIPE; ./git-log; echo $? >&2 ) | :
>     fatal: write failure on standard output: Broken pipe
>     128

That error return is fine. The annoying error report, however, is NOT.

For _exactly_ the same reason that a bash that doesn't have DONT_REPORT_SIGPIPE enabled is a piece of crap.

And your arguments that "others do it wrong, so we can too" is so broken as to be really really sad. If you cannot see the serious problem with that argument, I don't know what to tell you.

Try this:

	trap '' PIPE; ./git-log | head

and dammit, if you get an error message from that, your program is BROKEN.

And if you cannot understand that, then I don't even know what to say.

But _exiting_ is fine. It's the bogus error reporting that isn't. The above command like should NOT cause the user to have to skip stderr - because no error happened!

(Whether the error code is 0 or some error, I dunno. I'd argue that if you ignore SIGPIPE, you'd probably also want to do "exit(0)" for EPIPE, but it's not nearly as annoying as writing bogus error messages to stderr.

			Linus