Lucas Seiki Oshiro <lucasseikioshiro@xxxxxxxxx> writes:
> This commit is part of the series that introduce 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.
>
Nit: s/Then, add/Add
>
> Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
> Helped-by: Justin Tobler <jltobler@xxxxxxxxx>
> Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> Mentored-by: Patrick Steinhardt <ps@xxxxxx>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@xxxxxxxxx>
> ---
> Documentation/git-repo.adoc | 4 ++
> builtin/repo.c | 92 +++++++++++++++++++++++++++++++++++--
> t/meson.build | 1 +
> t/t1900-repo.sh | 47 +++++++++++++++++++
> 4 files changed, 140 insertions(+), 4 deletions(-)
> create mode 100755 t/t1900-repo.sh
>
> diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
> index 6f8fe3f6ea..b7af6f45a4 100644
> --- a/Documentation/git-repo.adoc
> +++ b/Documentation/git-repo.adoc
> @@ -45,6 +45,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, either `files` or `reftable`.
> +
Nit: I would omit the '`files` or `reftable`' here, because while this
is currently true. This might not hold up in the future. So better to
not go into the details of the supported systems.
> SEE ALSO
> --------
> linkgit:git-rev-parse[1]
> diff --git a/builtin/repo.c b/builtin/repo.c
> index a1787a3cc5..dcda0d6d61 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -1,11 +1,95 @@
> #include "builtin.h"
> #include "parse-options.h"
> +#include "strbuf.h"
> +#include "refs.h"
>
> -static int repo_info(int argc UNUSED,
> - const char **argv UNUSED,
> +typedef void add_field_fn(struct strbuf *buf, struct repository *repo);
> +
> +struct field {
> + const char *key;
> + add_field_fn *add_field_callback;
> +};
> +
> +static void add_string(struct strbuf *buf,
> + const char *key, const char *value)
> +{
> + strbuf_addf(buf, "%s\n%s%c", key, value, '\0');
> +}
> +
I like the table design used here, makes things much simpler. I do think
that each field shouldn't worry about the formatting, in fact, I would
say that we can move all of this logic to `print_fields`.
So each field would only be incharge of providing the output data. Then
`print_fields` would take the key, the output data and format it as
needed. This would also make it much easier to use a new format if
needed in the future.
> +static void add_references_format(struct strbuf *buf,
> + struct repository *repo)
> +{
> + add_string(buf, "references.format",
> + 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", add_references_format},
> +};
> +
> +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 add_field_fn *get_append_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 void print_fields(int argc, const char **argv, struct repository *repo) {
> + const char *last = "";
> + struct strbuf buf;
> + strbuf_init(&buf, 256);
> +
> + QSORT(argv, argc, qsort_strcmp);
> +
> + for (int i = 0; i < argc; i++) {
> + add_field_fn *callback;
> + const char *key = argv[i];
> +
> + if (!strcmp(key, last))
> + continue;
> +
> + callback = get_append_callback(key);
> +
> + if (!callback) {
> + error("key %s not found", key);
> + strbuf_release(&buf);
> + exit(1);
> + }
> +
> + callback(&buf, repo);
> + last = key;
> + }
> +
> + fwrite(buf.buf, 1, buf.len, stdout);
> + strbuf_release(&buf);
> +}
> +
> +static int repo_info(int argc,
> + const char **argv,
> const char *prefix UNUSED,
> - struct repository *repo UNUSED)
> + struct repository *repo)
> {
> +
> + print_fields(argc - 1 , argv + 1, repo);
> return 0;
> }
>
> @@ -16,7 +100,7 @@ int cmd_repo(int argc,
> {
> parse_opt_subcommand_fn *fn = NULL;
> const char *const repo_usage[] = {
> - "git repo info",
> + "git repo info [<key>...]",
Shouldn't this be part of the previous commit?
> NULL
> };
> struct option options[] = {
> diff --git a/t/meson.build b/t/meson.build
> index 1af289425d..8693e6abc4 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -245,6 +245,7 @@ integration_tests = [
> 't1700-split-index.sh',
> 't1701-racy-split-index.sh',
> 't1800-hook.sh',
> + 't1900-repo.sh',
> 't2000-conflict-when-checking-files-out.sh',
> 't2002-checkout-cache-u.sh',
> 't2003-checkout-cache-mkdir.sh',
> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> new file mode 100755
> index 0000000000..b80fc6b78b
> --- /dev/null
> +++ b/t/t1900-repo.sh
> @@ -0,0 +1,47 @@
> +#!/bin/sh
> +
> +test_description='test git repo-info'
> +
> +. ./test-lib.sh
> +
> +# Test if a field is correctly returned in the null-terminated format
> +#
> +# Usage: test_repo_info <label> <init command> <key> <expected value>
> +#
> +# Arguments:
> +# label: the label of the test
> +# init command: a command that creates a repository called 'repo', configured
> +# accordingly to what is being tested
> +# key: the key of the field that is being tested
> +# expected value: the value that the field should contain
> +test_repo_info () {
> + label=$1
> + init_command=$2
> + key=$3
> + expected_value=$4
> +
> + test_expect_success "$label" '
> + test_when_finished "rm -rf repo" &&
> + eval "$init_command" &&
> + echo "$expected_value" | lf_to_nul >expected &&
> + git -C repo repo info "$key" >output &&
> + tail -n 1 output >actual &&
> + test_cmp expected actual
> + '
> +}
> +
> +test_repo_info 'ref format files is retrieved correctly' '
> + git init --ref-format=files repo' 'references.format' 'files'
> +
> +test_repo_info 'ref format reftable is retrieved correctly' '
> + git init --ref-format=reftable repo' 'references.format' 'reftable'
> +
> +test_expect_success "only one value is returned if the same key is requested twice" '
> + echo "references.format" > expected &&
> + git rev-parse --show-ref-format > ref-format &&
> + lf_to_nul <ref-format >>expected &&
> + git repo info references.format references.format > actual &&
> + test_cmp expected actual
> +'
> +
> +test_done
> --
> 2.39.5 (Apple Git-154)
Attachment:
signature.asc
Description: PGP signature
