Re: [GSoC PATCH v5 2/5] repo: add the field references.format
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Jul 22, 2025, 19:25 UTC
- Message-ID
- <ldomqfgzts2fs3zuzuyfpsp4jsuec7a6ooisztqx6pe2373jzx@mqzh62weo2jm>
- In-Reply-To
- <20250722002835.33428-3-lucasseikioshiro@gmail.com>
On 25/07/21 09:28PM, Lucas Seiki Oshiro wrote:
Show 67 quoted lines
> This commit is part of the series that introduces the new subcommand
> git-repo-info.
>
> The flag `--show-ref-format` from git-rev-parse is used for retrieving
> the reference format (i.e. `files` or `reftable`). This way, it is
> used for querying repository metadata, fitting in the purpose of
> git-repo-info.
>
> Then, add a new field `references.format` to the repo-info subcommand
> containing that information.
>
> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Justin Tobler <jltobler@gmail.com>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> ---
> Documentation/git-repo.adoc | 4 ++
> builtin/repo.c | 75 ++++++++++++++++++++++++++++++++++++-
> t/meson.build | 1 +
> t/t1900-repo.sh | 50 +++++++++++++++++++++++++
> 4 files changed, 128 insertions(+), 2 deletions(-)
> create mode 100755 t/t1900-repo.sh
>
> diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
> index caee7d8aef..cf8483ec49 100644
> --- a/Documentation/git-repo.adoc
> +++ b/Documentation/git-repo.adoc
> @@ -29,6 +29,10 @@ INFO KEYS
> The set of data that `git repo` can return is grouped into the following
> categories:
>
> +`references`::
> +Reference-related data:
> +* `format`: the reference storage format
> +
> SEE ALSO
> --------
> linkgit:git-rev-parse[1]
> diff --git a/builtin/repo.c b/builtin/repo.c
> index d4f01e35e2..5beae0f781 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -1,12 +1,83 @@
> #include "builtin.h"
> #include "parse-options.h"
> +#include "refs.h"
>
> -static int repo_info(int argc UNUSED, const char **argv UNUSED,
> - const char *prefix UNUSED, struct repository *repo UNUSED)
> +typedef const char *get_value_fn(struct repository *repo);
> +
> +struct field {
> + const char *key;
> + get_value_fn *add_field_callback;
> +};
> +
> +static const char *get_references_format(struct repository *repo)
> +{
> + return ref_storage_format_to_name(repo->ref_storage_format);
> +}
> +
> +/* repo_info_fields keys should be in lexicographical order */
> +static const struct field repo_info_fields[] = {
> + { "references.format", get_references_format },
> +};Ok, so each key has a corresponding callback that is used to get its value. This works fine when we have one operation/callback per key, but I could see this being a bit inflexible in cases where performing a single operation could be expected to generate multiple keys worth of information at a time.
I certainly see this being the case with git-repo-stats where, for example, interating over references will produce multiple keyvalues indicating the number of branches, tags, remotes, etc. But, maybe for git-repo-info this will not be as much of a concern?
Show 50 quoted lines
> +
> +static int repo_info_fields_cmp(const void *va, const void *vb)
> +{
> + const struct field *a = va;
> + const struct field *b = vb;
> +
> + return strcmp(a->key, b->key);
> +}
> +
> +static get_value_fn *get_value_callback(const char *key)
> {
> + const struct field search_key = { key, NULL };
> + const struct field *found = bsearch(&search_key, repo_info_fields,
> + ARRAY_SIZE(repo_info_fields),
> + sizeof(struct field),
> + repo_info_fields_cmp);
> + return found ? found->add_field_callback : NULL;
> +}
> +
> +static int qsort_strcmp(const void *va, const void *vb)
> +{
> + const char *a = *(const char **)va;
> + const char *b = *(const char **)vb;
> +
> + return strcmp(a, b);
> +}
> +
> +static int print_fields(int argc, const char **argv, struct repository *repo)
> +{
> + const char *last = "";
> +
> + QSORT(argv, argc, qsort_strcmp);
> +
> + for (int i = 0; i < argc; i++) {
> + get_value_fn *callback;
> + const char *key = argv[i];
> + const char *value;
> +
> + if (!strcmp(key, last))
> + continue;
> +
> + callback = get_value_callback(key);
> +
> + if (!callback)
> + return error("key %s not found", key);
> +
> + value = callback(repo);
> + printf("%s=%s\n", key, value);
> + last = key;
> + }If the user does not input any keys, we simply do nothing. I do wonder if this is really the best default behavior. Maybe instead we should error out? Or maybe treat it as though all keys were requested?
-Justin
Show 13 quoted lines
> +
> return 0;
> }
>
> +static int repo_info(int argc, const char **argv, const char *prefix UNUSED,
> + struct repository *repo)
> +{
> + return print_fields(argc - 1, argv + 1, repo);
> +}
> +
> int cmd_repo(int argc, const char **argv, const char *prefix,
> struct repository *repo)
> {