[PATCH v6 0/7] builtin/repo: introduce structure subcommand

[Justin Tobler <jltobler@gmail.com>]

Greetings,

The structure of a repository's history can have huge impacts on the
performance and health of the repository itself. Currently, Git lacks a
means to surface repository metrics regarding its structure/shape via a
single command. Acquiring this information requires users to be familiar
with the relevant data points and the various Git commands required to
surface them. To fill this gap, supplemental tools such as git-sizer(1)
have been developed.

To allow users to more readily identify repository structure related
information, introduce the "structure" subcommand in git-repo(1). The
goal of this subcommand is to eventually provide similar functionality
to git-sizer(1), but natively in Git.

In this initial version, the "structure" subcommand only surfaces counts
of the various reference and object types in a repository. In a
follow-up series, I would like to introduce additional data points that
are present in git-sizer(1) such as largest objects, combined object
sizes by type, and other general repository shape information.

Some other general features that would be nice to introduce eventually:

- A "level of concern" meter for reported stats. This could indicate to
  users which stats may be worth looking into further.
- Links to OIDs of interesting objects that correspond to certain stats.
- Options to limit which references to use when evaluating the
  repository.

Changes since V5:
- Instead of using `filter_refs()` to get an array of all references, we
  now use `refs_for_each_ref()` to count references, and setup OIDs for
  the path walk, in place. Doing this not only allows us to avoid
  wasting memory storing all the reference info, but also to display
  progress info to the user while iterating across the references
  initially.
- Add a prepatory patch to export `ref_kind_from_refname()` via
  "ref_filter.h" so we can reuse logic to categorize references while
  counting.

Changes since V4:
- The subcommand was renamed from "stats" to "structure". This was done
  to define a more narrow scope for the types of stats that would be
  outtputted. This also also for other types of stat-related subcommands
  to be implemented in the future that may cover different aspects of
  the repository.
- Table column widths are now stored as ints to avoid the unneeded back
  and forth conversions.
- Dropped the clang-format patch as it has been upstreamed separately.
- Updated commit messages accordingly.

Changes since V3:
- Changed from using strlen() to utf8_strlen() to take into
  consideration that translatable strings may have characters that are
  more than one byte.

Changes since V2:
- Added clang-format patch to address false postive triggered in this
  series.
- Use varargs for stats_table_add() family of functions.
- Print to stdout directly instead of using strbuf.
- Add parse_option() earlier in the series.
- Use start_delayed_progress() instead of start_progress().
- Add test to validate --[no-]progress options.
- Some other small fixes.

Changes since V1:
- Translatable terms displayed in the table have formatting separated
  out.
- Squashed the `keyvalue` and `nul` output format patches into one.
- Added a progress meter to provide users with more feedback.
- Updated docs to outline to outline reported data in a bulleted list.
- Combined similar tests together to reduce repetitive setup.
- Added patch to improve ref-filter interface so we don't have to create
  a dummy patterns array.
- Many other renames and cleanups to improve patch clarity.

Thanks,
-Justin

Justin Tobler (7):
  builtin/repo: rename repo_info() to cmd_repo_info()
  ref-filter: allow NULL filter pattern
  ref-filter: export ref_kind_from_refname()
  builtin/repo: introduce structure subcommand
  builtin/repo: add object counts in structure output
  builtin/repo: add keyvalue and nul format for structure stats
  builtin/repo: add progress meter for structure stats

 Documentation/git-repo.adoc |  30 +++
 builtin/repo.c              | 380 +++++++++++++++++++++++++++++++++++-
 ref-filter.c                |   6 +-
 ref-filter.h                |   2 +
 t/meson.build               |   1 +
 t/t1901-repo-structure.sh   | 129 ++++++++++++
 6 files changed, 542 insertions(+), 6 deletions(-)
 create mode 100755 t/t1901-repo-structure.sh

Range-diff against v5:
1:  ed04168562 = 1:  ed04168562 builtin/repo: rename repo_info() to cmd_repo_info()
2:  6aa76d1323 = 2:  6aa76d1323 ref-filter: allow NULL filter pattern
-:  ---------- > 3:  aee696c69b ref-filter: export ref_kind_from_refname()
3:  eda1afbe3d ! 4:  4ad599d0ec builtin/repo: introduce structure subcommand
    @@ builtin/repo.c: static int cmd_repo_info(int argc, const char **argv, const char
     +	va_end(ap);
     +}
     +
    ++static inline size_t get_total_reference_count(struct ref_stats *stats)
    ++{
    ++	return stats->branches + stats->remotes + stats->tags + stats->others;
    ++}
    ++
     +static void stats_table_setup_structure(struct stats_table *table,
     +					struct ref_stats *refs)
     +{
     +	size_t ref_total;
     +
    -+	ref_total = refs->branches + refs->remotes + refs->tags + refs->others;
    ++	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"));
    @@ builtin/repo.c: static int cmd_repo_info(int argc, const char **argv, const char
     +	string_list_clear(&table->rows, 1);
     +}
     +
    -+static void structure_count_references(struct ref_stats *stats,
    -+				       struct ref_array *refs)
    ++static int count_references(const char *refname,
    ++			    const char *referent UNUSED,
    ++			    const struct object_id *oid UNUSED,
    ++			    int flags UNUSED, void *cb_data)
     +{
    -+	for (int i = 0; i < refs->nr; i++) {
    -+		struct ref_array_item *ref = refs->items[i];
    -+
    -+		switch (ref->kind) {
    -+		case FILTER_REFS_BRANCHES:
    -+			stats->branches++;
    -+			break;
    -+		case FILTER_REFS_REMOTES:
    -+			stats->remotes++;
    -+			break;
    -+		case FILTER_REFS_TAGS:
    -+			stats->tags++;
    -+			break;
    -+		case FILTER_REFS_OTHERS:
    -+			stats->others++;
    -+			break;
    -+		default:
    -+			BUG("unexpected reference type");
    -+		}
    ++	struct ref_stats *stats = cb_data;
    ++
    ++	switch (ref_kind_from_refname(refname)) {
    ++	case FILTER_REFS_BRANCHES:
    ++		stats->branches++;
    ++		break;
    ++	case FILTER_REFS_REMOTES:
    ++		stats->remotes++;
    ++		break;
    ++	case FILTER_REFS_TAGS:
    ++		stats->tags++;
    ++		break;
    ++	case FILTER_REFS_OTHERS:
    ++		stats->others++;
    ++		break;
    ++	default:
    ++		BUG("unexpected reference type");
     +	}
    ++
    ++	return 0;
    ++}
    ++
    ++static void structure_count_references(struct ref_stats *stats,
    ++				       struct repository *repo)
    ++{
    ++	refs_for_each_ref(get_main_ref_store(repo), count_references, &stats);
     +}
     +
     +static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
    -+			      struct repository *repo UNUSED)
    ++			      struct repository *repo)
     +{
    -+	struct ref_filter filter = REF_FILTER_INIT;
     +	struct stats_table table = {
     +		.rows = STRING_LIST_INIT_DUP,
     +	};
     +	struct ref_stats stats = { 0 };
    -+	struct ref_array refs = { 0 };
     +	struct option options[] = { 0 };
     +
     +	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
     +	if (argc)
     +		usage(_("too many arguments"));
     +
    -+	if (filter_refs(&refs, &filter, FILTER_REFS_REGULAR))
    -+		die(_("unable to filter refs"));
    -+
    -+	structure_count_references(&stats, &refs);
    ++	structure_count_references(&stats, repo);
     +
     +	stats_table_setup_structure(&table, &stats);
     +	stats_table_print_structure(&table);
     +
     +	stats_table_clear(&table);
    -+	ref_array_clear(&refs);
     +
     +	return 0;
     +}
4:  503af885d3 ! 5:  4d37f65331 builtin/repo: add object counts in structure output
    @@ builtin/repo.c: struct ref_stats {
      struct stats_table {
      	struct string_list rows;
      
    -@@ builtin/repo.c: static void stats_table_count_addf(struct stats_table *table, size_t value,
    - 	va_end(ap);
    +@@ builtin/repo.c: static inline size_t get_total_reference_count(struct ref_stats *stats)
    + 	return stats->branches + stats->remotes + stats->tags + stats->others;
      }
      
     +static inline size_t get_total_object_count(struct object_stats *stats)
    @@ builtin/repo.c: static void stats_table_count_addf(struct stats_table *table, si
     +	size_t object_total;
      	size_t ref_total;
      
    - 	ref_total = refs->branches + refs->remotes + refs->tags + refs->others;
    + 	ref_total = get_total_reference_count(refs);
     @@ builtin/repo.c: static void stats_table_setup_structure(struct stats_table *table,
      	stats_table_count_addf(table, refs->tags, "    * %s", _("Tags"));
      	stats_table_count_addf(table, refs->remotes, "    * %s", _("Remotes"));
    @@ builtin/repo.c: static void stats_table_setup_structure(struct stats_table *tabl
      }
      
      static void stats_table_print_structure(const struct stats_table *table)
    -@@ builtin/repo.c: static void structure_count_references(struct ref_stats *stats,
    +@@ builtin/repo.c: static void stats_table_clear(struct stats_table *table)
    + 	string_list_clear(&table->rows, 1);
    + }
    + 
    ++struct count_references_data {
    ++	struct ref_stats *stats;
    ++	struct rev_info *revs;
    ++};
    ++
    + static int count_references(const char *refname,
    + 			    const char *referent UNUSED,
    +-			    const struct object_id *oid UNUSED,
    ++			    const struct object_id *oid,
    + 			    int flags UNUSED, void *cb_data)
    + {
    +-	struct ref_stats *stats = cb_data;
    ++	struct count_references_data *data = cb_data;
    ++	struct ref_stats *stats = data->stats;
    + 
    + 	switch (ref_kind_from_refname(refname)) {
    + 	case FILTER_REFS_BRANCHES:
    +@@ builtin/repo.c: static int count_references(const char *refname,
    + 		BUG("unexpected reference type");
      	}
    + 
    ++	/*
    ++	 * While iterating through references for counting, also add OIDs in
    ++	 * preparation for the path walk.
    ++	 */
    ++	add_pending_oid(data->revs, NULL, oid, 0);
    ++
    + 	return 0;
      }
      
    + static void structure_count_references(struct ref_stats *stats,
    ++				       struct rev_info *revs,
    + 				       struct repository *repo)
    + {
    +-	refs_for_each_ref(get_main_ref_store(repo), count_references, &stats);
    ++	struct count_references_data data = {
    ++		.stats = stats,
    ++		.revs = revs,
    ++	};
    ++
    ++	refs_for_each_ref(get_main_ref_store(repo), count_references, &data);
    ++}
    ++
    ++
     +static int count_objects(const char *path UNUSED, struct oid_array *oids,
     +			 enum object_type type, void *cb_data)
     +{
    @@ builtin/repo.c: static void structure_count_references(struct ref_stats *stats,
     +}
     +
     +static void structure_count_objects(struct object_stats *stats,
    -+				    struct ref_array *refs,
     +				    struct rev_info *revs)
     +{
     +	struct path_walk_info info = PATH_WALK_INFO_INIT;
    @@ builtin/repo.c: static void structure_count_references(struct ref_stats *stats,
     +	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;
    -+		default:
    -+			BUG("unexpected reference type");
    -+		}
    -+	}
    -+
     +	walk_objects_by_path(&info);
     +	path_walk_info_clear(&info);
    -+}
    -+
    + }
    + 
      static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
    --			      struct repository *repo UNUSED)
    -+			      struct repository *repo)
    - {
    - 	struct ref_filter filter = REF_FILTER_INIT;
    +@@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
      	struct stats_table table = {
      		.rows = STRING_LIST_INIT_DUP,
      	};
     -	struct ref_stats stats = { 0 };
     +	struct repo_structure stats = { 0 };
    - 	struct ref_array refs = { 0 };
     +	struct rev_info revs;
      	struct option options[] = { 0 };
      
    @@ builtin/repo.c: static void structure_count_references(struct ref_stats *stats,
      	if (argc)
      		usage(_("too many arguments"));
      
    +-	structure_count_references(&stats, repo);
     +	repo_init_revisions(repo, &revs, prefix);
    - 	if (filter_refs(&refs, &filter, FILTER_REFS_REGULAR))
    - 		die(_("unable to filter refs"));
    - 
    --	structure_count_references(&stats, &refs);
    -+	structure_count_references(&stats.refs, &refs);
    -+	structure_count_objects(&stats.objects, &refs, &revs);
    ++
    ++	structure_count_references(&stats.refs, &revs, repo);
    ++	structure_count_objects(&stats.objects, &revs);
      
      	stats_table_setup_structure(&table, &stats);
      	stats_table_print_structure(&table);
      
      	stats_table_clear(&table);
     +	release_revisions(&revs);
    - 	ref_array_clear(&refs);
      
      	return 0;
    + }
     
      ## t/t1901-repo-structure.sh ##
     @@ t/t1901-repo-structure.sh: test_expect_success 'empty repository' '
5:  b336578445 ! 6:  3d42929434 builtin/repo: add keyvalue and nul format for structure stats
    @@ builtin/repo.c: static void stats_table_clear(struct stats_table *table)
     +	fflush(stdout);
     +}
     +
    - static void structure_count_references(struct ref_stats *stats,
    - 				       struct ref_array *refs)
    - {
    + struct count_references_data {
    + 	struct ref_stats *stats;
    + 	struct rev_info *revs;
     @@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
      	struct stats_table table = {
      		.rows = STRING_LIST_INIT_DUP,
      	};
     +	enum output_format format = FORMAT_TABLE;
      	struct repo_structure stats = { 0 };
    - 	struct ref_array refs = { 0 };
      	struct rev_info revs;
     -	struct option options[] = { 0 };
     +	struct option options[] = {
    @@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const
      	argc = parse_options(argc, argv, prefix, options, repo_usage, 0);
      	if (argc)
     @@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
    - 	structure_count_references(&stats.refs, &refs);
    - 	structure_count_objects(&stats.objects, &refs, &revs);
    + 	structure_count_references(&stats.refs, &revs, repo);
    + 	structure_count_objects(&stats.objects, &revs);
      
     -	stats_table_setup_structure(&table, &stats);
     -	stats_table_print_structure(&table);
6:  70c0b7e200 ! 7:  67d7d8eb8c builtin/repo: add progress meter for structure stats
    @@ builtin/repo.c
      #include "ref-filter.h"
      #include "refs.h"
     @@ builtin/repo.c: static void structure_keyvalue_print(struct repo_structure *stats,
    + struct count_references_data {
    + 	struct ref_stats *stats;
    + 	struct rev_info *revs;
    ++	struct progress *progress;
    + };
    + 
    + static int count_references(const char *refname,
    +@@ builtin/repo.c: static int count_references(const char *refname,
    + {
    + 	struct count_references_data *data = cb_data;
    + 	struct ref_stats *stats = data->stats;
    ++	size_t ref_count;
    + 
    + 	switch (ref_kind_from_refname(refname)) {
    + 	case FILTER_REFS_BRANCHES:
    +@@ builtin/repo.c: static int count_references(const char *refname,
    + 	 */
    + 	add_pending_oid(data->revs, NULL, oid, 0);
    + 
    ++	ref_count = get_total_reference_count(stats);
    ++	display_progress(data->progress, ref_count);
    ++
    + 	return 0;
      }
      
      static void structure_count_references(struct ref_stats *stats,
    --				       struct ref_array *refs)
    -+				       struct ref_array *refs,
    + 				       struct rev_info *revs,
    +-				       struct repository *repo)
     +				       struct repository *repo,
     +				       int show_progress)
      {
    -+	struct progress *progress = NULL;
    -+
    -+	if (show_progress)
    -+		progress = start_delayed_progress(repo, _("Counting references"),
    -+						  refs->nr);
    -+
    - 	for (int i = 0; i < refs->nr; i++) {
    - 		struct ref_array_item *ref = refs->items[i];
    + 	struct count_references_data data = {
    + 		.stats = stats,
    + 		.revs = revs,
    + 	};
      
    -@@ builtin/repo.c: static void structure_count_references(struct ref_stats *stats,
    - 		default:
    - 			BUG("unexpected reference type");
    - 		}
    -+
    -+		display_progress(progress, i + 1);
    - 	}
    ++	if (show_progress)
    ++		data.progress = start_delayed_progress(repo,
    ++						       _("Counting references"), 0);
     +
    -+	stop_progress(&progress);
    + 	refs_for_each_ref(get_main_ref_store(repo), count_references, &data);
    ++	stop_progress(&data.progress);
      }
      
     +struct count_objects_data {
     +	struct object_stats *stats;
     +	struct progress *progress;
     +};
    -+
    + 
      static int count_objects(const char *path UNUSED, struct oid_array *oids,
      			 enum object_type type, void *cb_data)
      {
    @@ builtin/repo.c: static int count_objects(const char *path UNUSED, struct oid_arr
      }
      
      static void structure_count_objects(struct object_stats *stats,
    --				    struct ref_array *refs,
     -				    struct rev_info *revs)
    -+				    struct ref_array *refs, struct rev_info *revs,
    ++				    struct rev_info *revs,
     +				    struct repository *repo, int show_progress)
      {
      	struct path_walk_info info = PATH_WALK_INFO_INIT;
    @@ builtin/repo.c: static int count_objects(const char *path UNUSED, struct oid_arr
      	info.path_fn = count_objects;
     -	info.path_fn_data = stats;
     +	info.path_fn_data = &data;
    - 
    - 	for (int i = 0; i < refs->nr; i++) {
    - 		struct ref_array_item *ref = refs->items[i];
    -@@ builtin/repo.c: static void structure_count_objects(struct object_stats *stats,
    - 		}
    - 	}
    - 
    ++
     +	if (show_progress)
     +		data.progress = start_delayed_progress(repo, _("Counting objects"), 0);
    -+
    + 
      	walk_objects_by_path(&info);
      	path_walk_info_clear(&info);
     +	stop_progress(&data.progress);
    @@ builtin/repo.c: static void structure_count_objects(struct object_stats *stats,
      
      static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
     @@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
    + 	enum output_format format = FORMAT_TABLE;
      	struct repo_structure stats = { 0 };
    - 	struct ref_array refs = { 0 };
      	struct rev_info revs;
     +	int show_progress = -1;
      	struct option options[] = {
    @@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const
      	};
      
     @@ builtin/repo.c: static int cmd_repo_structure(int argc, const char **argv, const char *prefix,
    - 	if (filter_refs(&refs, &filter, FILTER_REFS_REGULAR))
    - 		die(_("unable to filter refs"));
      
    --	structure_count_references(&stats.refs, &refs);
    --	structure_count_objects(&stats.objects, &refs, &revs);
    + 	repo_init_revisions(repo, &revs, prefix);
    + 
    +-	structure_count_references(&stats.refs, &revs, repo);
    +-	structure_count_objects(&stats.objects, &revs);
     +	if (show_progress < 0)
     +		show_progress = isatty(2);
     +
    -+	structure_count_references(&stats.refs, &refs, repo, show_progress);
    -+	structure_count_objects(&stats.objects, &refs, &revs, repo, show_progress);
    ++	structure_count_references(&stats.refs, &revs, repo, show_progress);
    ++	structure_count_objects(&stats.objects, &revs, repo, show_progress);
      
      	switch (format) {
      	case FORMAT_TABLE:
    @@ t/t1901-repo-structure.sh: test_expect_success 'keyvalue and nul format' '
     +		GIT_PROGRESS_DELAY=0 git repo structure --progress >out 2>err &&
     +
     +		test_file_not_empty out &&
    -+		test_grep "Counting references: 100% (2/2), done." err &&
    ++		test_grep "Counting references: 2, done." err &&
     +		test_grep "Counting objects: 3, done." err &&
     +
     +		GIT_PROGRESS_DELAY=0 git repo structure --no-progress >out 2>err &&

base-commit: ca2559c1d630eb4f04cdee2328aaf1c768907a9e
-- 
2.51.0.193.g4975ec3473b