Re: [PATCH 2/4] builtin/repo: add object counts in stats output

> On Mon, Sep 22, 2025 at 09:56:58PM -0500, Justin Tobler wrote:
> > 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.

> > 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.

> >  	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`.

> 
> > @@ -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.

> > +	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?

> > @@ -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.

> > +		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.