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

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 <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/progress.c b/progress.c
index 6cc7f902f5e1d4030dcf39633ccc4ba345bdd5d2..0cdd875d37f166bedbbeb5f0e889046674ed58be 100644 (file)
--- a/progress.c
+++ b/progress.c
@@ -311,8 +311,7 @@ struct progress *start_delayed_sparse_progress(const char *title,
 
 static void finish_if_sparse(struct progress *progress)
 {
-       if (progress &&
-           progress->sparse &&
+       if (progress->sparse &&
            progress->last_value != progress->total)
                display_progress(progress, progress->total);
 }
@@ -347,22 +346,6 @@ static void log_trace2(struct progress *progress)
        trace2_region_leave("progress", progress->title, the_repository);
 }
 
-void stop_progress(struct progress **p_progress)
-{
-       struct progress *progress;
-
-       if (!p_progress)
-               BUG("don't provide NULL to stop_progress");
-       progress = *p_progress;
-
-       finish_if_sparse(progress);
-
-       if (progress)
-               log_trace2(*p_progress);
-
-       stop_progress_msg(p_progress, _("done"));
-}
-
 void stop_progress_msg(struct progress **p_progress, const char *msg)
 {
        struct progress *progress;
@@ -375,8 +358,10 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
                return;
        *p_progress = NULL;
 
+       finish_if_sparse(progress);
        if (progress->last_value != -1)
                force_last_update(progress, msg);
+       log_trace2(progress);
 
        clear_progress_signal();
        strbuf_release(&progress->counters_sb);

diff --git a/progress.h b/progress.h
index 4f6806904a86d2d062d8772a4435a2186169ad34..3a945637c81c22734b563325b66956ee4fb33b0b 100644 (file)
--- a/progress.h
+++ b/progress.h
@@ -1,5 +1,6 @@
 #ifndef PROGRESS_H
 #define PROGRESS_H
+#include "gettext.h"
 
 struct progress;
 
@@ -19,5 +20,8 @@ struct progress *start_delayed_progress(const char *title, uint64_t total);
 struct progress *start_delayed_sparse_progress(const char *title,
                                               uint64_t total);
 void stop_progress_msg(struct progress **p_progress, const char *msg);
-void stop_progress(struct progress **p_progress);
+static inline void stop_progress(struct progress **p_progress)
+{
+       stop_progress_msg(p_progress, _("done"));
+}
 #endif

diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh
index 759169d0742c999adb33f91bc1ae1283877cb3db..bbe2e69c758d136ebc93bae8c2d5f3460435cf86 100755 (executable)
--- a/t/t5316-pack-delta-depth.sh
+++ b/t/t5316-pack-delta-depth.sh
@@ -61,7 +61,11 @@ test_expect_success 'create series of packs' '
                        echo $cur
                        echo "$(git rev-parse :file) file"
                } | git pack-objects --stdout >tmp &&
-               git index-pack --stdin --fix-thin <tmp || return 1
+               GIT_TRACE2_EVENT=$PWD/trace \
+               git index-pack -v --stdin --fix-thin <tmp || return 1 &&
+               grep -c region_enter.*progress trace >enter &&
+               grep -c region_leave.*progress trace >leave &&
+               test_cmp enter leave &&
                prev=$cur
        done
 '