Marco Roeland [off-list ref] wrote:
On monday May 28th 2007 at 20:19 Jim Meyering wrote:
quoted
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?
But just out of curiosity is there an easy way to test the EPIPE
behaviour? I cite a piece of the "changelog.Debian" file from the
There are some examples here:
http://thread.gmane.org/gmane.comp.version-control.git/48469/focus=48617
...
The mentioned "test-case" as used in "git log -n1 | exit 1" doesn't
produce an error in my Debian 'sid' bash, either with or without your
patch, so it doesn't seem to have any effect there? Whereas probably in
a "default" bash (don't know if upstream has changed it's mind already!)
with your patch (i.e. the EPIPE special casing removal) it will again
probably introduce these annoying (for interactive use) errors.
As I just said in reply to Linus, the EPIPE handling difference
is independent of what version of bash you use.
Thanks for your patch anyway, the "fcntl" diagnosis is a really useful
technique to know, and IMVHO also useful for git; although perhaps not
very portable for all platforms.
It appears to be portable enough. fcntl/F_GETFD support is required
by POSIX, and has been around for ages. FWIW, it's also used in git's
daemon.c and sha1_file.c.
(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:
Marco Roeland [off-list ref] wrote:
quoted
On monday May 28th 2007 at 20:19 Jim Meyering wrote:
quoted
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