git/progress.c, branch v2.40.2

progress API: unify stop_progress{,_msg}(), fix trace2 bug

2022-02-03T23:39:59Z

Fix a bug that's been with us ever since 98a13647408 (trace2: log progress time and throughput, 2020-05-12), when the stop_progress_msg() API was used we didn't log a "region_leave" for the "region_enter" we start in "start_progress_delay()". The only user of the "stop_progress_msg()" function is "index-pack". Let's add a previously failing test to check that we have the same number of "region_enter" and "region_leave" events, with "-v" we'll log progress even in the test environment. In addition to that we've had a submarine bug here introduced with 9d81ecb52b5 (progress: add sparse mode to force 100% complete message, 2019-03-21). The "start_sparse_progress()" API would only do the right thing if the progress was ended with "stop_progress()", not "stop_progress_msg()". The only user of that API uses "stop_progress()", but let's still fix that along with the trace2 issue by making "stop_progress()" a trivial wrapper for "stop_progress_msg()". We can also drop the "if (progress)" test from "finish_if_sparse()". It's now a helper for the small "stop_progress_msg()" function. We'll already have returned from it if "progress" is "NULL". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

progress.c: refactor stop_progress{,_msg}() to use helpers

2022-02-03T23:39:55Z

Create two new static helpers for the stop_progress() and stop_progress_msg() functions. As we'll see in the subsequent commit having those two split up doesn't make much sense, and results in a bug in how we log to trace2. This narrow preparatory change makes the diff for that subsequent change smaller. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

progress.c: use dereferenced "progress" variable, not "(*p_progress)"

2022-02-03T23:39:55Z

Since 98a13647408 (trace2: log progress time and throughput, 2020-05-12) stop_progress() dereferences a "struct progress **" parameter in several places. Extract a dereferenced variable to reduce clutter and make it clearer who needs to write to this parameter. Now instead of using "*p_progress" several times in stop_progress() we check it once for NULL and then use a dereferenced "progress" variable thereafter. This uses the same pattern as the adjacent stop_progress_msg() function, see ac900fddb7f (progress: don't dereference before checking for NULL, 2020-08-10). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

use CALLOC_ARRAY

2021-03-14T00:00:09Z

Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

Merge branch 'ma/stop-progress-null-fix'

2020-08-18T00:02:48Z

NULL dereference fix. * ma/stop-progress-null-fix: progress: don't dereference before checking for NULL

progress: don't dereference before checking for NULL

2020-08-10T21:59:57Z

In `stop_progress()`, we're careful to check that `p_progress` is non-NULL before we dereference it, but by then we have already dereferenced it when calling `finish_if_sparse(*p_progress)`. And, for what it's worth, we'll go on to blindly dereference it again inside `stop_progress_msg()`. We could return early if we get a NULL-pointer, but let's go one step further and BUG instead. The progress API handles NULL just fine, but that's the NULL-ness of `*p_progress`, e.g., when running with `--no-progress`. If `p_progress` is NULL, chances are that's a mistake. For symmetry, let's do the same check in `stop_progress_msg()`, too. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano

progress: call trace2_region_leave() only after calling _enter()

2020-05-15T16:41:30Z

A user of progress API calls start_progress() conditionally and depends on the display_progress() and stop_progress() functions to become no-op when start_progress() hasn't been called. As we added a call to trace2_region_enter() to start_progress(), the calls to other trace2 API calls from the progress API functions must make sure that these trace2 calls are skipped when start_progress() hasn't been called on the progress struct. Specifically, do not call trace2_region_leave() from stop_progress() when we haven't called start_progress(), which would have called the matching trace2_region_enter(). Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

trace2: log progress time and throughput

2020-05-12T22:30:39Z

Rather than teaching only one operation, like 'git fetch', how to write down throughput to traces, we can learn about a wide range of user operations that may seem slow by adding tooling to the progress library itself. Operations which display progress are likely to be slow-running and the kind of thing we want to monitor for performance anyways. By showing object counts and data transfer size, we should be able to make some derived measurements to ensure operations are scaling the way we expect. Signed-off-by: Emily Shaffer Signed-off-by: Junio C Hamano

progress.c: silence cgcc suggestion about internal linkage

2020-04-27T18:21:28Z

Signed-off-by: Đoàn Trần Công Danh Reviewed-by: Ramsay Jones Signed-off-by: Junio C Hamano

progress: create GIT_PROGRESS_DELAY

2019-11-27T01:57:10Z

The start_delayed_progress() method is a preferred way to show optional progress to users as it ignores steps that take less than two seconds. However, this makes testing unreliable as tests expect to be very fast. In addition, users may want to decrease or increase this time interval depending on their preferences for terminal noise. Create the GIT_PROGRESS_DELAY environment variable to control the delay set during start_delayed_progress(). Set the value in some tests to guarantee their output remains consistent. Helped-by: Jeff King Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano