Thread (17 messages) flat view 17 messages, 5 authors, 2016-06-15

Re: [PATCH] git-rev-list: give better diagnostic for failed write

From: Jim Meyering <hidden>
Date: 2016-06-15 22:43:18

Possibly related (same subject, not in this thread)

Linus Torvalds [off-list ref] wrote:
On Mon, 25 Jun 2007, Jim Meyering wrote:
quoted
[this patch depends on the one I posted here:
 http://marc.info/?l=git&m=118280134031923&w=2 ]

Without this patch, git-rev-list unnecessarily omits strerror(errno)
from its diagnostic, upon write failure:
And this is a perfect example of what's wrong with the whole thing.

Dammit, how many times do I need to say this:

 - If you want reliable errors, don't use stdio!

That fflush is there FOR A REASON. You removed it FOR A MUCH LESS
IMPORTANT REASON!
Wow.  No need to curse and get into ALL_CAPS_MODE every time you
reply to me.  It does not advance your cause.

Remember: I'm trying to improve existing code here.
You should save some of your ire for the person who wrote that code.
That fflush is there exactly because WE DO NOT WANT TO BUFFER the list of
commits, because that thing is meant very much to be used for pipelines,
and it's quite common that the receiving end is going to do something
asynchronous with the result, and can - and does - want the results as
soon as possible.
That's good to know.  I'm glad you pointed it out.  It would have been
nice to have a comment.  However, wouldn't it be better at least to check
for and report fflush failure?  fflush usually does a write, after all.
Most of the rest of the code is careful to diagnose write errors at
the source.  Why not here?

I've posted a revised patch.
IOW, things like "gitk" use git-rev-list exactly to get the list of
commits, and they want that list *incrementally*. They don't want to wait
for git-rev-list to have filled up some 8kB buffer of commits. Especially
since generating those commits can be slow if we're talking about a big
tree and some path-limited stuff.

So for example, do something like

	git rev-list HEAD -- drivers/char/drm/Makefile

and if you don't see the result scroll a line at a time on a slower
machine, there's something *wrong*.

Junio, I'm NAK'ing this very forcefully!

Jim: I don't know what I'm doing wrong, but I'm apparently not reaching
you. So let me try one more time:

 - stdio really isn't very good with error handling

 - if you use stdio, YOU HAD BETTER ACCEPT THAT
Using stdio is fine, as long as you know and respect its limitations.

It's a real shame that you have to intersperse your often-valuable
feedback with such vitriol.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help