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

[Jim Meyering <jim@meyering.net>]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, 25 Jun 2007, Jim Meyering wrote:
>>
>> [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.