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

[Patrick Steinhardt <ps@pks.im>]

On Tue, Oct 13, 2026 at 01:28:51PM -0400, Mark C. Chu-Carroll wrote:
> index 84e012f83f..f45dea06d5 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -947,16 +953,20 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
>  		OPT_END()
>  	};
>  
> -	argc = parse_options(argc, argv, prefix, options, repo_structure_usage, 0);
> -	if (argc)
> -		usage(_("too many arguments"));
> -
> -	repo_init_revisions(repo, &revs, prefix);
> -
> +	argc = parse_options(argc, argv, prefix, options, repo_structure_usage,
> +			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
>  	if (show_progress < 0)
>  		show_progress = isatty(2);
>  
> -	structure_count_references(&stats.refs, &revs, repo, show_progress);
> +	repo_init_revisions(repo, &revs, prefix);
> +	if (argc > 1) {
> +		argc = setup_revisions(argc, argv, &revs, NULL);
> +		included_revision_args = true;
> +		if (argc > 1)
> +			usage(_("too many arguments"));

Nit: it's not "too many arguments", it's unrecognized arguments here as
the number of arguments can theoretically be unlimited. Copying patterns
from other builtins:

	argc = setup_revisions(argc, argv, &revs, NULL);
	if (argc > 1)
		return error(_("unrecognized argument: %s"), argv[1]);

> diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh
> index 02cc2b594a..1d25ee14e5 100755
> --- a/t/t1901-repo-structure.sh
> +++ b/t/t1901-repo-structure.sh
> @@ -140,10 +140,208 @@ test_expect_success SHA1 'repository with references and objects' '
[snip]
> +test_expect_success 'filter by object type' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit_bulk 1005 &&
> +		git tag -a foo -m bar &&
> +
> +		oid="$(git rev-parse HEAD)" &&
> +		git update-ref refs/remotes/origin/foo "$oid" &&
> +		git checkout -b grobble &&
> +		test_commit_bulk --ref=refs/heads/grobble 20 &&
> +		git checkout master &&
> +		test_commit_bulk 20 &&
> +		# Also creates a commit, tree, and blob.
> +		git notes add -m foo &&
> +
> +		cat >expect <<-\EOF &&
> +		objects.commits.count=0
> +		objects.trees.count=0
> +		objects.blobs.count=1006
> +		objects.tags.count=0
> +		objects.commits.inflated_size=0
> +		objects.trees.inflated_size=0
> +		objects.blobs.inflated_size=11962
> +		objects.tags.inflated_size=0
> +		objects.commits.disk_size=0
> +		objects.trees.disk_size=0
> +		objects.blobs.disk_size=21022
> +		objects.tags.disk_size=0
> +		objects.commits.max_size=0
> +		objects.commits.max_size_oid=0000000000000000000000000000000000000000
> +		objects.trees.max_size=0
> +		objects.trees.max_size_oid=0000000000000000000000000000000000000000
> +		objects.blobs.max_size=13
> +		objects.blobs.max_size_oid=97d808e45116bf02103490294d3d46dad7a2ac62
> +		objects.tags.max_size=0
> +		objects.tags.max_size_oid=0000000000000000000000000000000000000000
> +		objects.commits.max_parents=0
> +		objects.commits.max_parents_oid=0000000000000000000000000000000000000000
> +		objects.trees.max_entries=0
> +		objects.trees.max_entries_oid=0000000000000000000000000000000000000000
> +		EOF
> +		git repo structure --format=lines --objects --all --filter=object:type=blob >actual 2>err &&
> +		test_grep "^objects" actual >actual.objects &&
> +		test_cmp expect actual.objects &&
> +		test_must_be_empty err
>  	)
>  '
>  
> +
>  test_expect_success SHA1 'lines and nul format' '
>  	test_when_finished "rm -rf repo" &&
>  	git init repo &&

There's a trailing newline being added here.

> @@ -186,13 +384,13 @@ test_expect_success SHA1 'lines and nul format' '
>  		git repo structure --format=lines >out 2>err &&
>  
>  		test_cmp expect out &&
> -		test_line_count = 0 err &&
> +		test_must_be_empty err &&
>  
>  		git repo structure --format=nul >out 2>err &&
>  		tr "\012\000" "=\012" <out >actual &&
>  
>  		test_cmp expect actual &&
> -		test_line_count = 0 err &&
> +		test_must_be_empty err &&
>  
>  		# "-z", as a synonym to "--format=nul", participates in the
>  		# usual "last one wins" rule.
> @@ -200,7 +398,7 @@ test_expect_success SHA1 'lines and nul format' '
>  		tr "\012\000" "=\012" <out >actual &&
>  
>  		test_cmp expect actual &&
> -		test_line_count = 0 err
> +		test_must_be_empty err
>  	)
>  '
>  
> @@ -220,7 +418,7 @@ test_expect_success 'progress meter option' '
>  		GIT_PROGRESS_DELAY=0 git repo structure --no-progress >out 2>err &&
>  
>  		test_file_not_empty out &&
> -		test_line_count = 0 err
> +		test_must_be_empty err
>  	)
>  '

I'd drop these while-at-it changes. If you want to include them they
should be part of a preparatory commit.

But that's all I've got now, other than those nits I'm happy with this
version. Thanks!

Patrick