[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:
$ ./git-rev-list --max-count=1 HEAD > /dev/full
fatal: write failure on standard output
With the patch, git reports the desired ENOSPC diagnostic:
fatal: write failure on standard output: No space left on device
* builtin-rev-list (show_commit): Don't fflush stdout here.
Instead, let the fclose in main do it, so there's a better
chance the underlying cause (errno) will be reported.
Signed-off-by: Jim Meyering <redacted>
---
builtin-rev-list.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 813aadf..62f0ba9 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -100,7 +100,6 @@ static void show_commit(struct commit *commit)
printf("%s%c", buf, hdr_termination);
free(buf);
}
- fflush(stdout);
if (commit->parents) {
free_commit_list(commit->parents);
commit->parents = NULL;
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!
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.
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
- don't screw up basic functionality in your *insane* quest to get stdio
to give you ENOSPC. It's not going to happen. Not that way. Just face
the fact that stdio *will* throw error numbers away.
The whole notion of "buffered IO" and "reliable errors" is simply not
something that goes well together.
Linus