Using one of the start_delayed_*() functions, clients of the progress
API can request that a progress meter is only shown after some time.
To do that, the implementation intends to count down the number of
seconds stored in struct progress by observing flag progress_update,
which the timer interrupt handler sets when a second has elapsed. This
works during the first second of the delay. But the code forgets to
reset the flag to zero, so that subsequent calls of display_progress()
think that another second has elapsed and decrease the count again
until zero is reached. Due to the frequency of the calls, this happens
without an observable delay in practice, so that the effective delay is
always just one second.
This bug has been with us since the inception of the feature. Despite
having been touched on various occasions, such as 8aade107dd84
(progress: simplify "delayed" progress API), 9c5951cacf5c (progress:
drop delay-threshold code), and 44a4693bfcec (progress: create
GIT_PROGRESS_DELAY), the short delay went unnoticed.
Copy the flag state into a local variable and reset the global flag
right away so that we can detect the next clock tick correctly.
Since we have not had any complaints that the delay of one second is
too short nor that GIT_PROGRESS_DELAY is ignored, people seem to be
comfortable with the status quo. Therefore, set the default to 1 to
keep the current behavior.
Signed-off-by: Johannes Sixt <j6t@xxxxxxxx>
---
Compared to the first round, this replaces sig_atomic_t by int. I
didn't use bool just in case the patch goes on top of a maintenance
track that does not have the "bool is allowed" policy.
Documentation/git.adoc | 2 +-
progress.c | 12 +++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 743b7b00e4..03e9e69d25 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -684,7 +684,7 @@ other
`GIT_PROGRESS_DELAY`::
A number controlling how many seconds to delay before showing
- optional progress indicators. Defaults to 2.
+ optional progress indicators. Defaults to 1.
`GIT_EDITOR`::
This environment variable overrides `$EDITOR` and `$VISUAL`.
diff --git a/progress.c b/progress.c
index 8d5ae70f3a..8315bdc3d4 100644
--- a/progress.c
+++ b/progress.c
@@ -114,16 +114,19 @@ static void display(struct progress *progress, uint64_t n, const char *done)
const char *tp;
struct strbuf *counters_sb = &progress->counters_sb;
int show_update = 0;
+ int update = !!progress_update;
int last_count_len = counters_sb->len;
- if (progress->delay && (!progress_update || --progress->delay))
+ progress_update = 0;
+
+ if (progress->delay && (!update || --progress->delay))
return;
progress->last_value = n;
tp = (progress->throughput) ? progress->throughput->display.buf : "";
if (progress->total) {
unsigned percent = n * 100 / progress->total;
- if (percent != progress->last_percent || progress_update) {
+ if (percent != progress->last_percent || update) {
progress->last_percent = percent;
strbuf_reset(counters_sb);
@@ -133,7 +136,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
tp);
show_update = 1;
}
- } else if (progress_update) {
+ } else if (update) {
strbuf_reset(counters_sb);
strbuf_addf(counters_sb, "%"PRIuMAX"%s", (uintmax_t)n, tp);
show_update = 1;
@@ -166,7 +169,6 @@ static void display(struct progress *progress, uint64_t n, const char *done)
}
fflush(stderr);
}
- progress_update = 0;
}
}
@@ -281,7 +283,7 @@ static int get_default_delay(void)
static int delay_in_secs = -1;
if (delay_in_secs < 0)
- delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 2);
+ delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 1);
return delay_in_secs;
}
--
2.51.0.205.g9a02ae2892
