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

From: Petr Baudis <pasky@suse.cz>
Date: May 28, 2007, 23:41 UTC
Message-ID: <20070528234103.GV4489@pasky.or.cz>
In-Reply-To: <87veecvgsn.fsf@rho.meyering.net>

  (I think that funnily enough, Linus is to a degree to the Git
community something like Al Viro and Chris Hellwig are to the Linux
kernel community. Don't get too derailed by his blunt^Whonest criticism,
which is however usually quite valid. ;-)

On Mon, May 28, 2007 at 10:23:20PM CEST, Jim Meyering wrote:

Show 18 quoted lines

> Marco Roeland <marco.roeland@xs4all.nl> wrote:
> > On monday May 28th 2007 at 20:19 Jim Meyering wrote:
> >> Also, to be consistent, don't ignore EPIPE write failures.
> >
> > In practice I agree with someone else on this thread that EPIPE _is_
> > different. In a way the responsibility doesn't lie with the writer but
> > with the reader.
> 
> Do you think it's ok for git-rev-list _not_ to diagnose an erroneous
> command like this (i.e., to exit(0)):
> 
>     git-rev-list HEAD | sync
> 
> where "sync" could be any command that exits successfully
> without reading any input?
> 
> Is it ok that it is currently *impossible* to diagnose that
> failure by looking at exit codes?

  Actually, yes!

  Because there's no "failure" per se. The command we piped the output
into just decided that he isn't actually interested in any (for whatever
reason; it might decide dynamically based on some parameters etc.). I
can't think of why it could be considered a failure for git-rev-list if
its customer doesn't happily eat all the output it generates. It's the
customer's job to report any real trouble that happenned and might be
cause of the premature end (or maybe the premature end was totally
valid).

  Maybe it could expose some (IMHO contrived) error scenarios, but in
most cases I think it will end up just spitting out bogus error
messages. And what will people do? They won't bother to filter out this
particular one (which isn't even that easy if the strerror() is
localized, furthermore). They will just 2>/dev/null it. And cause the
*real* error messages go to the land of void as well. There's enough of
impossible-to-diagnose-error-conditions-because-stderr-goes-to-null
scripts in the land of UNIX already and this patch, while actually
well-meant to do the opposite, might well actually increase their number
because of this.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett