Re: [PATCH 2/4] builtin/repo: add object counts in stats output
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 23, 2025, 10:52 UTC
- Message-ID
- <aNJ8BvTZ_yNSrBA6@pks.im>
- In-Reply-To
- <20250923025700.3046260-3-jltobler@gmail.com>
On Mon, Sep 22, 2025 at 09:56:58PM -0500, Justin Tobler wrote:
Show 15 quoted lines
> diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc > index 7762329551..2a67abfca8 100644 > --- a/Documentation/git-repo.adoc > +++ b/Documentation/git-repo.adoc > @@ -45,8 +45,9 @@ supported: > `-z` is an alias for `--format=nul`. > > stats:: > - Retrieve stats about the current repository. All references in the > - repository are categorized and counted accordingly. > + Retrieve stats about the current repository. All references and > + reachable objects in the repository are categorized and counted > + accordingly. > + > The table output format may change and is not intended for machine parsing.
I already wanted to mention this on the first commit, but would it maybe make sense if this was a bulleted list of information that we surface right from the start? Then we don't have to reflow the whole paragraph every time we surface new information.
Show 10 quoted lines
> diff --git a/builtin/repo.c b/builtin/repo.c
> index 15899dd74c..a24ea0e66b 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -159,13 +161,25 @@ static int repo_info(int argc, const char **argv, const char *prefix,
> return print_fields(argc, argv, repo, format);
> }
>
> -struct stats {
> +struct ref_stats {Nit: let's call it `ref_stats` right from the start instead of renaming.
Show 14 quoted lines
> size_t branches;
> size_t remotes;
> size_t tags;
> size_t others;
> };
>
> +struct object_stats {
> + size_t tags;
> + size_t commits;
> + size_t trees;
> + size_t blobs;
> +};
> +
> +struct stats {I'd maybe call this `struct repo_stats`. `stats` feels quite generic and very close to a collision with `struct stat`.
Show 6 quoted lines
> @@ -207,15 +221,27 @@ static void stats_table_add_count(struct stats_table *table, const char *name,
>
> static void stats_table_setup(struct stats_table *table, struct stats *stats)
> {
> + struct object_stats objects = stats->objects;
> + struct ref_stats refs = stats->refs;We can avoid the copies by making these pointers. Not that it'd really matter all that much.
Show 19 quoted lines
> + size_t object_total;
> size_t ref_total;
>
> - ref_total = stats->branches + stats->remotes + stats->tags + stats->others;
> + ref_total = refs.branches + refs.remotes + refs.tags + refs.others;
> stats_table_add(table, _("* References"), NULL);
> stats_table_add_count(table, _(" * Count"), ref_total);
> - stats_table_add_count(table, _(" * Branches"), stats->branches);
> - stats_table_add_count(table, _(" * Tags"), stats->tags);
> - stats_table_add_count(table, _(" * Remotes"), stats->remotes);
> - stats_table_add_count(table, _(" * Others"), stats->others);
> + stats_table_add_count(table, _(" * Branches"), refs.branches);
> + stats_table_add_count(table, _(" * Tags"), refs.tags);
> + stats_table_add_count(table, _(" * Remotes"), refs.remotes);
> + stats_table_add_count(table, _(" * Others"), refs.others);
> +
> + object_total = objects.commits + objects.trees + objects.blobs + objects.tags;
> + stats_table_add(table, "", NULL);
> + stats_table_add(table, _("* Objects"), NULL);Should we maybe say "Reachable objects" here to clarify that this doesn't count unreachable ones?
Show 23 quoted lines
> @@ -282,25 +308,80 @@ static void stats_count_references(struct stats *stats, struct ref_array *refs)
> }
> }
>
> +static int count_objects(const char *path UNUSED, struct oid_array *oids,
> + enum object_type type, void *data)
> +{
> + struct object_stats *stats = data;
> +
> + switch (type) {
> + case OBJ_TAG:
> + stats->tags += oids->nr;
> + break;
> + case OBJ_COMMIT:
> + stats->commits += oids->nr;
> + break;
> + case OBJ_TREE:
> + stats->trees += oids->nr;
> + break;
> + case OBJ_BLOB:
> + stats->blobs += oids->nr;
> + break;
> + default:Let's `BUG()` here. This case should never happen, and if it does something is seriously wrong.
Show 31 quoted lines
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void stats_count_objects(struct object_stats *stats,
> + struct ref_array *refs, struct rev_info *revs)
> +{
> + struct path_walk_info info = PATH_WALK_INFO_INIT;
> +
> + info.revs = revs;
> + info.path_fn = count_objects;
> + info.path_fn_data = stats;
> +
> + for (int i = 0; i < refs->nr; i++) {
> + struct ref_array_item *ref = refs->items[i];
> +
> + switch (ref->kind) {
> + case FILTER_REFS_BRANCHES:
> + case FILTER_REFS_TAGS:
> + case FILTER_REFS_REMOTES:
> + case FILTER_REFS_OTHERS:
> + add_pending_oid(revs, NULL, &ref->objectname, 0);
> + break;
> + }
> + }
> +
> + walk_objects_by_path(&info);
> + path_walk_info_clear(&info);
> +}I guess this can take a while, so having a progress meter would be great to have to give the user some info what's happening. I guess it doesn't have to be part of the first iteration thuogh as long as this is something we plan to add at a later point.
Show 62 quoted lines
> diff --git a/t/t1901-repo-stats.sh b/t/t1901-repo-stats.sh > index 27c32ec45f..c6a7f08be5 100755 > --- a/t/t1901-repo-stats.sh > +++ b/t/t1901-repo-stats.sh > @@ -20,6 +20,13 @@ test_expect_success 'empty repository stats' ' > | * Tags | 0 | > | * Remotes | 0 | > | * Others | 0 | > + | | | > + | * Objects | | > + | * Count | 0 | > + | * Commits | 0 | > + | * Trees | 0 | > + | * Blobs | 0 | > + | * Tags | 0 | > EOF > > test_cmp expect out && > @@ -49,6 +56,45 @@ test_expect_success 'repository stats with references' ' > | * Tags | 1 | > | * Remotes | 1 | > | * Others | 1 | > + | | | > + | * Objects | | > + | * Count | 5 | > + | * Commits | 2 | > + | * Trees | 2 | > + | * Blobs | 1 | > + | * Tags | 0 | > + EOF > + > + test_cmp expect out && > + test_line_count = 0 err > + ) > +' > + > +test_expect_success 'repository stats with objects' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit_bulk 42 && > + git tag -a foo -m bar && > + git repo stats >out 2>err && > + > + cat >expect <<-EOF && > + | Repository stats | Value | > + | ---------------- | ----- | > + | * References | | > + | * Count | 2 | > + | * Branches | 1 | > + | * Tags | 1 | > + | * Remotes | 0 | > + | * Others | 0 | > + | | | > + | * Objects | | > + | * Count | 127 | > + | * Commits | 42 | > + | * Trees | 42 | > + | * Blobs | 42 | > + | * Tags | 1 | > EOF
I quite like the output format, by the way. It's nice to read and makes it sufficiently clear that this is not expected to be parsed by a machine.
Patrick