Re: [PATCH v2 6/6] builtin/repo: add progress meter for stats
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Sep 25, 2025, 13:20 UTC
- Message-ID
- <ehjxy2qvwhrtmyvmjv6hu5ej2pslomjn476mlkkodmbxwutel5@yx6an6nj75p4>
- In-Reply-To
- <aNTVgnEGE2qUwZwK@pks.im>
On 25/09/25 07:39AM, Patrick Steinhardt wrote:
Show 21 quoted lines
> On Wed, Sep 24, 2025 at 04:24:26PM -0500, Justin Tobler wrote:
> > diff --git a/builtin/repo.c b/builtin/repo.c
> > index fe7d43f78e..fdc8af92dc 100644
> > --- a/builtin/repo.c
> > +++ b/builtin/repo.c
> > @@ -344,8 +345,14 @@ static void stats_keyvalue_print(struct repo_stats *stats, char key_delim,
> > strbuf_release(&buf);
> > }
> >
> > -static void stats_count_references(struct ref_stats *stats, struct ref_array *refs)
> > +static void stats_count_references(struct ref_stats *stats, struct ref_array *refs,
> > + struct repository *repo, int show_progress)
> > {
> > + struct progress *progress = NULL;
> > +
> > + if (show_progress)
> > + progress = start_progress(repo, _("Counting references"), refs->nr);
>
> We tend to use `start_delayed_progress()` so that the progress meter is
> not displayed when the action takes less than a second. The delay can be
> disabled in our tests by using `GIT_PROGRESS_DELAY=0`.Good to know. I'll update in the next version.
[snip]
Show 9 quoted lines
> > + object_count = stats->tags + stats->commits + stats->trees + stats->blobs;
>
> We have this computation in two locations now. Maybe we should
> deduplicate it via something like:
>
> static inline size_t stats_get_total_object_count()
> {
> return stats->tags + stats->commits + stats->trees + stats->blobs;
> }Make sense. Will add.
Show 9 quoted lines
>
> > @@ -417,8 +442,12 @@ static void stats_count_objects(struct object_stats *stats,
> > }
> > }
> >
> > + if (show_progress)
> > + data.progress = start_progress(repo, _("Counting Objects"), 0);
>
> s/Objects/objects/Will fix in the next version.
[snip]
Show 8 quoted lines
> > + stats_count_references(&stats.refs, &refs, repo, show_progress);
> > + stats_count_objects(&stats.objects, &refs, &revs, repo, show_progress);
> >
> > switch (format) {
> > case FORMAT_TABLE:
>
> Should our tests be updated to verify that we know to print progress
> depending on whether or not `--progress` is passed?That seems reasonable to me. I'll add a test for this in the next version.
-Justin