Re: [PATCH 1/1] repo: add filtering options to "repo structure"

[Patrick Steinhardt <ps@pks.im>]

On Thu, Sep 24, 2026 at 12:45:03PM -0400, Mark C. Chu-Carroll wrote:
> "git repo structure" provides a collection of useful information
> about the information stored in a repo. In particular, it's
> valuable for diagnosing performance issues caused by large objects
> stored in a repo.
> 
> The current implementation of "git repo stucture" provides summary
> information about everything in the repository - all of the
> branches, remotes, tags, stashes, and notes. But sometimes
> to properly diagnose a problem, it's useful to be able to exclude
> refs that are known to not be relevant to the issue at hand.

Yes, indeed. Sometimes you may for example want to figure out where
exactly the storage size of a particular repository is going. Or in the
case of GitLab for example, we may have bookkeeping references that are
not controllable by customers. So we may only want to get the structure
for all the customer-controllable branches there.

> Add a set of flags that allow a user to selective exclude
> reference types from the report generated by "git repo structure".
> When a ref type is excluded by the filter, it no longer appears
> in the report (ie, if "--no-tags" is passed, the report line
> for "Branches" will no longer appear under "* References").
> Following the pattern of flags that are only used to
> disable functionality (eg, "--no-verify" in "builtins/push.c"),
> only the "--no-<reftype>" syntax is listed in the updated
> documentation.

Hmm, okay. I would have expected that the user can essentially pass
arbitrary revisions as understood by git-log(1) et al. And if they pass
any such revisions, we should not enumerate anything but what they have
passed, so the flags shouldn't only be used to exclude.

So, for example:

    $ git repo structure --branches
    $ git repo structure master
    $ git repo structure --all --not --branches

I would hope that git-repo(1) can achieve that rather easily because I
expect that it uses `struct rev_info`, but let's read on.

> Overview of the changes:
> - Add an enum to represent the structure flags.
> - Add structure flags to the options for the "repo structure" commands.
> - For each reference flag, add a conditional in "count_references"
>   which decides whether or not to add a ref to the pending list.
>   If an references is not added to the pending list, the things it
>   transitively references will not be added to the stats.
> - Add a set of test cases to verify that reference counts
>   in the repo structure report correctly omit the specified
>   resource types.
> - Update the documentation for git-repo to include the new options.

Note that we typically don't have lists of what exactly has changed in
the commit. That kind of information is already visible from the diff
itself. So what the commit message itself should focus on is whether any
of these changes are non-obvious or whethere there's any dragons to be
found.

So in summary: everything that may surprise the reader should be part of
the commit message, everything that's just obvious plumbing doesn't
really have to be mentioned.

> diff --git a/builtin/repo.c b/builtin/repo.c
> index 84e012f83f..c8f6e38011 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -490,7 +504,8 @@ static inline size_t get_total_object_values(struct object_values *values)
>  }
>  
>  static void stats_table_setup_structure(struct stats_table *table,
> -					struct repo_structure *stats)
> +					struct repo_structure *stats,
> +					enum repo_structure_filter_flags flags)
>  {
>  	struct object_stats *objects = &stats->objects;
>  	struct ref_stats *refs = &stats->refs;
> @@ -502,9 +517,15 @@ static void stats_table_setup_structure(struct stats_table *table,
>  	ref_total = get_total_reference_count(refs);
>  	stats_table_addf(table, "* %s", _("References"));
>  	stats_table_count_addf(table, ref_total, "  * %s", _("Count"));
> -	stats_table_count_addf(table, refs->branches, "    * %s", _("Branches"));
> -	stats_table_count_addf(table, refs->tags, "    * %s", _("Tags"));
> -	stats_table_count_addf(table, refs->remotes, "    * %s", _("Remotes"));
> +	if (flags & REPO_STRUCTURE_FILTER_BRANCHES) {
> +		stats_table_count_addf(table, refs->branches, "    * %s", _("Branches"));
> +	}
> +	if (flags & REPO_STRUCTURE_FILTER_TAGS) {
> +		stats_table_count_addf(table, refs->tags, "    * %s", _("Tags"));
> +	}
> +	if (flags & REPO_STRUCTURE_FILTER_REMOTES) {
> +		stats_table_count_addf(table, refs->remotes, "    * %s", _("Remotes"));
> +	}
>  	stats_table_count_addf(table, refs->others, "    * %s", _("Others"));
>  
>  	object_count_total = get_total_object_values(&objects->type_counts);

Coding style: we don't use curly braces around single-line statements.

But more importantly, I think this is where the mismatch in expectations
comes from that I was pointing out further up. My expectation was that
what we want to achieve is to filter the reachable objects by revisions,
which I think is a much more useful thing to do. But what the flags do
instead us to filter the output in the "References" count.

I think that we should rather go into the direction of filtering objects
and not the ref output, as the latter isn't all that useful. It _may_
make sense to maybe make some sections of the output optional, but
excluding individual ref types is arguably too fine-grained.

In any case, to go into the direction of filtering objects you'd want to
adapt `parse_options()` so that it accepts unknown options (you can
achieve that by passing `PARSE_OPT_KEEP_ARGV0 |
PARSE_OPT_KEEP_UNKNOWN_OPT`) and then pass argv to `setup_revisions()`.
And I think that _should_ already achieve proper filtering of objects by
revisions.

Thanks!

Patrick