[RFH] Flush progress message buffer in display().

[RFH] Flush progress message buffer in display().

[Johannes Sixt]

There are cases where the stderr of the program that reports progress
is not connected to a terminal (e.g. pack-objects when invoked by
upload-pack). In this case, the output is buffered by stdio. Consequently,
a considerable amount of progress report is accumulated before it is
written (in the upload-pack case to the remote end). A fflush() after
each progress display gives a nice continuous progress.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---

I need this patch on Windows because appearently progress output is buffered
by stdio. Why doesn't Linux/glibc's stdio buffer output that goes to a pipe?
If I inspect the data that upload-pack sends over the wire, then on Linux
it looks like this (the linebreaks don't appear on the wire):

0024^BCounting objects: 20946, done.
002c^BCompressing objects:   0% (1/15804)   ^M
002e^BCompressing objects:   1% (159/15804)   ^M
002e^BCompressing objects:   2% (317/15804)   ^M
002e^BCompressing objects:   3% (475/15804)   ^M
002e^BCompressing objects:   4% (633/15804)   ^M
002e^BCompressing objects:   5% (791/15804)   ^M

But on Windows it looks more like this:

0085^BCompressing objects:   0% (1/15804)   ^MCompressing objects:   1% (159/15804)   ^MCompressing objects:   2% (317/15804)   ^MCompr
0085^Bessing objects:   3% (475/15804)   ^MCompressing objects:   4% (633/15804)   ^MCompressing objects:   5% (791/15804)   ^MCompress
0085^Bing objects:   6% (990/16487)   ...

The 0085 is obviously the length of progress[128] in create_pack_file()
plus 5 bytes overhead. This happens because pack-objects buffers the
progress output and upload-pack reads it out in pieces of 128 bytes.

Why doesn't the same happen on Linux? What is flushing the progress
output?

BTW, to reproduce, use this in your favorite git repo:

#!/bin/sh
git upload-pack . <<EOF | less
0040want $(git rev-parse HEAD) side-band-64k
00000009done
0000
EOF

-- Hannes

 progress.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/progress.c b/progress.c
index 4bd650f..d19f80c 100644
--- a/progress.c
+++ b/progress.c
@@ -98,11 +98,13 @@ static int display(struct progress *progress, unsigned n, const char *done)
 			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
 				progress->title, percent, n,
 				progress->total, tp, eol);
+			fflush(stderr);
 			progress_update = 0;
 			return 1;
 		}
 	} else if (progress_update) {
 		fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol);
+		fflush(stderr);
 		progress_update = 0;
 		return 1;
 	}
@@ -207,6 +209,7 @@ struct progress *start_progress_delay(const char *title, unsigned total,
 	if (!progress) {
 		/* unlikely, but here's a good fallback */
 		fprintf(stderr, "%s...\n", title);
+		fflush(stderr);
 		return NULL;
 	}
 	progress->title = title;
-- 
1.5.3.5.739.gcac53

Re: [RFH] Flush progress message buffer in display().

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> I need this patch on Windows because appearently progress output is buffered
> by stdio. Why doesn't Linux/glibc's stdio buffer output that goes to a pipe?
> ...
> ... What is flushing the progress
> output?

The standard error stream is not "fully buffered":

    http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_05.html#tag_02_05

Not "fully buffered" means either "unbuffered" or "line
buffered".  Because the progress display does not terminate its
line, it means that the additional fflush(stderr) you added are
needed if the stream is line buffered (still conforming).

Re: [RFH] Flush progress message buffer in display().

[Nicolas Pitre]

On Mon, 19 Nov 2007, Junio C Hamano wrote:

> Johannes Sixt <johannes.sixt@telecom.at> writes:
> 
> > I need this patch on Windows because appearently progress output is buffered
> > by stdio. Why doesn't Linux/glibc's stdio buffer output that goes to a pipe?
> > ...
> > ... What is flushing the progress
> > output?
> 
> The standard error stream is not "fully buffered":
> 
>     http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_05.html#tag_02_05
> 
> Not "fully buffered" means either "unbuffered" or "line
> buffered".  Because the progress display does not terminate its
> line, it means that the additional fflush(stderr) you added are
> needed if the stream is line buffered (still conforming).

Maybe stdio on Linux considers \r as a line termination.

Flushing stderr should not hurt anyway.


Nicolas

Re: [RFH] Flush progress message buffer in display().

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> On Mon, 19 Nov 2007, Junio C Hamano wrote:
>
>> The standard error stream is not "fully buffered":
>> 
>>     http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_05.html#tag_02_05
>> 
>> Not "fully buffered" means either "unbuffered" or "line
>> buffered".  Because the progress display does not terminate its
>> line, it means that the additional fflush(stderr) you added are
>> needed if the stream is line buffered (still conforming).
>
> Maybe stdio on Linux considers \r as a line termination.

Or totally unbufferred.  Running strace on this:

        #include <stdio.h>

        int
        main(int ac, char **av)
        {
                fprintf(stderr, "foo");
                fprintf(stderr, "bar");
                return 0;
        }

will tell you.  I bet you will see two write(2) of 3 bytes
each.

> Flushing stderr should not hurt anyway.

True, especially if the rate is limited, which it is.