Linus Torvalds [off-list ref] wrote:
On Mon, 25 Jun 2007, Jim Meyering wrote:
quoted
Remember: I'm trying to improve existing code here.
You should save some of your ire for the person who wrote that code.
Ehh. Remind me who I should be pissed at when the old code was _better_
before your change?
If someone pointed out a bug in coreutils whereby a tool failed
to give an accurate diagnostic, I would thank them (and honestly,
I'd be grateful), even if their attempt at fixing it introduced
a horrible bug. Of course, if the original bug were mine, I might
be annoyed with myself, but certainly would not shoot the messenger.
With the current git.c, we report write errors quite well.
Yes, I'm glad (in spite of the tone of much of your feedback)
that my little improvements are making it into git.
We don't give
the exact output you want, but on a scale of 1-10, how important is that?
Pretty damn low on the list.
If you get a report of a git write error, you might well
appreciate knowing right away whether it was due to EIO or to ENOSPC.
And the reason I'm really really irritated at you is that you ignore me
when I tell you what your bugs are.
Be careful when throwing stones...
I mentioned (without making a fuss) a regression you introduced,
http://thread.gmane.org/gmane.comp.version-control.git/50742/focus=50917
and you included similar code in a snippet in the very message to which
I'm replying now. I do see that you omitted the troublesome ferror call
from your eventual patch.
- I *told* you that EPIPE is special. What did you do? Ignore my advice,
and made a broken patch that did exactly the opposite of what I told
you.
I did not ignore your advice. I considered it carefully and explained
why I hold the opposing view. We've already beaten this to death, but
for the record, I'd summarize it like this:
You and I have a difference of opinion. I firmly believe that it is
unnecessary and counterproductive (it is prohibited by POSIX for many
of the tools I maintain) to suppress an EPIPE diagnostic and to exit
successfully in spite of a write error. Since EPIPE doesn't even arise
in most normal situations, the only reason to do that is if you want
to pander to the needs of some theoretical, broken-by-design (i.e.,
SIGPIPE-ignoring) git Porcelain program. You want to add such code to
git, at every point where writing to stdout may fail.
On Wed, 27 Jun 2007, Jim Meyering wrote:
If someone pointed out a bug in coreutils whereby a tool failed
to give an accurate diagnostic
The thing is, I don't think stdio output is "important" enough.
In fact, by definition, if the data was really important, it had better
not use stdio.
So look at where git uses "real" IO, and where it uses stdio, and you'll
see where the "diagnostics" matter.
In other words, the diagnostics matter for:
- the actual git database (including refs).
- doing things like "git checkout" that writes the working tree
Those are the ones where we want to *guarantee* correctness. And that
means that they don't use stdio (and if any of them do, then that really
*is* a bug, and should be fixed asap)
In contrast, if you redirect stdout, that's a totally different level of
correctness, and all bets are off. Exactly because we use stdio. If we
really cared, we shouldn't use stdio.
Be careful when throwing stones...
I mentioned (without making a fuss) a regression you introduced,
http://thread.gmane.org/gmane.comp.version-control.git/50742/focus=50917
and you included similar code in a snippet in the very message to which
I'm replying now. I do see that you omitted the troublesome ferror call
from your eventual patch.
Right. There's a difference between an email saying "ok, this is how you
might want to do it" and then actually sitting down and thinking about it
and coding it.
If you look closer, you'll notice that my initial suggestion wouldn't even
*compile*.
By the time I sent out a patch, I had actually thought it through.
And no, I don't guarantee that I do that all the time. I try to, but hey,
mistakes happen. It's when the mistake is repeated over and over after
being pointed out that I get irritated.
You and I have a difference of opinion. I firmly believe that it is
unnecessary and counterproductive (it is prohibited by POSIX for many
of the tools I maintain) to suppress an EPIPE diagnostic and to exit
successfully in spite of a write error.
Quite frankly, the tools you maintain are mostly filters in the first
place! In other words, for you, stdout *matters*. That's the real data!
But look at something like "cp" one of these days, and ask yourself:
- do you check error returns for the IO operations that actually do the
copy (I hope yes).
I also hope to high heaven that you would never use stdio for that IO.
Because you *will* lose error information!
- do you check the error returns of informational messages like the ones
from the verbose output switch (cp -v).
If you answered "no" to the second question, you should really think about
that fact. Please realize that there's a *huge* difference in error
checking between "core data" and "random information".
For something like "cat", the core data is the actual stream. You care a
lot more about that. For git, the core data is the working tree and the
database. I care about _that_. That's the stuff that should be protected
and must never _ever_ fail silently.
The rest is really "statistics".
Let's get it right, but unlike core data, that's mostly a politeness
issue, much less important than "oops, we just corrupted your archive".
And when we talk about politeness, avoiding the _idiotic_ error messages
like "Broken pipe" is a big part of it! The "Broken pipe" message means
either:
- the receiver already got everything he wanted (and you should damn well
shut up about it)
- the receiver had some problem (and it's damn well *that* which matters,
since the sender was fine, and you shouldn't confuse the issue by
writing bogus error messages)
The fact that you have some broken POSIX spec to worry about is not git's
problem. I hold git to higher standards.
Linus