On 11/7/2019 1:40 AM, Jeff King wrote: > On Wed, Nov 06, 2019 at 08:21:48AM -0500, Derrick Stolee wrote: > >>>> Now that we changed this method, very fast commands show no progess at >>>> all. This means we need to stop testing for seeing these progress lines >>>> in the test suite. >>> >>> I think this is OK for now, though it does make me wonder if >>> "--progress" ought to perhaps override "delayed" in some instances, >>> since it's a positive signal from the caller that they're interested in >>> seeing progress. >> >> I was thinking that we could start with a GIT_TEST_PROGRESS environment >> variable to force all delayed progress to act like non-delayed progress. >> That would at least give us confirmation on these kinds of tests. > > I think this could actually be a non-test variable. E.g., something like > this: > > diff --git a/progress.c b/progress.c > index 0063559aab..74b90e8898 100644 > --- a/progress.c > +++ b/progress.c > @@ -14,6 +14,7 @@ > #include "strbuf.h" > #include "trace.h" > #include "utf8.h" > +#include "config.h" > > #define TP_IDX_MAX 8 > > @@ -269,7 +270,8 @@ static struct progress *start_progress_delay(const char *title, uint64_t total, > > struct progress *start_delayed_progress(const char *title, uint64_t total) > { > - return start_progress_delay(title, total, 2, 0); > + int delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 2); > + return start_progress_delay(title, total, delay_in_secs, 0); > } > > struct progress *start_progress(const char *title, uint64_t total) I like this idea. It allows us to force the progress on in tests, and for users to tweak their preferred delay. That includes _increasing_ the delay if they want to. > which lets you ask for more verbose progress. There are times when I'd > use something like this for general debugging. Though these days I might > suggest that something like GIT_TRACE2_PERF hook the progress code to > output. That's a bit more complicated to implement, though. Would it make sense to make delay_in_secs a local static variable, so we remember it between calls? That would allow us to check the environment only once (not that it is usually expensive). -Stolee