On Thu, Nov 21, 2019 at 03:51:55PM +0000, Derrick Stolee via GitGitGadget wrote:
> @@ -267,9 +268,19 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,
> return progress;
> }
>
> +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);
> +
> + return delay_in_secs;
> +}
Thanks, this factoring out looks good.
Since the only callers of start_progress_delay() pass in either the
result of this function or "0", it _could_ become a bool flag and we
could just resolve it inside that function. But I don't think there's a
big advantage to doing so.
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index d42b3efe39..0824857e1f 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -132,7 +132,7 @@ test_expect_success 'commit-graph write progress off for redirected stderr' '
>
> test_expect_success 'commit-graph write force progress on for stderr' '
> cd "$TRASH_DIRECTORY/full" &&
> - git commit-graph write --progress 2>err &&
> + GIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&
> test_file_not_empty err
> '
I'm tempted to suggest that we should just set GIT_PROGRESS_DELAY=0 for
the whole test suite. That would root out any potentially racy tests,
though given that the default is 2 seconds, it would probably take a
pretty horribly loaded system to trigger such a race.
Curiously, doing this:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 46c4440843..63357ed6c4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -423,6 +423,9 @@ export EDITOR
GIT_TRACE_BARE=1
export GIT_TRACE_BARE
+GIT_PROGRESS_DELAY=0
+export GIT_PROGRESS_DELAY
+
check_var_migration () {
# the warnings and hints given from this helper depends
# on end-user settings, which will disrupt the self-test
results in a few test failures. It looks like unpack-trees is eager to
print the "Updating files" progress meter even when stderr is redirected
to a file. Which seems like a bug. I don't mind if we put that off for
now, though, in order to get your fix here merged more quickly.
-Peff
