From: Patrick Steinhardt <ps@pks.im>
To: John Cai via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Phillip Wood" <phillip.wood123@gmail.com>,
"Kristoffer Haugsbakk" <code@khaugsbakk.name>,
"Jeff King" <peff@peff.net>,
"Jean-Noël Avila" <avila.jn@gmail.com>,
"John Cai" <johncai86@gmail.com>
Subject: Re: [PATCH v2 3/3] show-ref: add --symbolic-name option
Date: Wed, 10 Apr 2024 08:53:04 +0200 [thread overview]
Message-ID: <ZhY3UCwJeMcG4fH1@tanuki> (raw)
In-Reply-To: <a9e6644327a04f1d309eca812ace9c4159781353.1712597893.git.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10047 bytes --]
On Mon, Apr 08, 2024 at 05:38:13PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
>
> For reftable development, it would be handy to have a tool to provide
> the direct value of any ref whether it be a symbolic ref or not.
> Currently there is git-symbolic-ref, which only works for symbolic refs,
> and git-rev-parse, which will resolve the ref. Let's teach show-ref a
> --symbolic-name option that will cause git-show-ref(1) to print out the
> value symbolic references points to.
I think it was Peff who shared a way to achieve this without actually
introducing a new option via `git for-each-ref --format=`. Can we maybe
provide some benchmarks to demonstrate that this variant is preferable
over what's already possible?
Patrick
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> Documentation/git-show-ref.txt | 21 ++++++++++++++++++-
> builtin/show-ref.c | 38 ++++++++++++++++++++++++----------
> refs.c | 6 ++++++
> refs.h | 2 +-
> t/t1403-show-ref.sh | 20 ++++++++++++++++++
> 5 files changed, 74 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
> index ba757470059..9627b34b37f 100644
> --- a/Documentation/git-show-ref.txt
> +++ b/Documentation/git-show-ref.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
> [verse]
> 'git show-ref' [--head] [-d | --dereference]
> [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]
> - [--heads] [--] [<pattern>...]
> + [--heads] [--symbolic-name] [--] [<pattern>...]
> 'git show-ref' --verify [-q | --quiet] [-d | --dereference]
> [-s | --hash[=<n>]] [--abbrev[=<n>]]
> [--] [<ref>...]
> @@ -58,6 +58,11 @@ OPTIONS
> Dereference tags into object IDs as well. They will be shown with `^{}`
> appended.
>
> +--symbolic-name::
> +
> + Print out the value the reference points to without dereferencing. This
> + is useful to know the reference that a symbolic ref is pointing to.
> +
> -s::
> --hash[=<n>]::
>
> @@ -146,6 +151,20 @@ $ git show-ref --heads --hash
> ...
> -----------------------------------------------------------------------------
>
> +When using `--symbolic-name`, the output is in the format:
> +
> +-----------
> +<oid> SP <ref> SP <symbolic-name>
> +-----------
> +
> +For example,
> +
> +-----------------------------------------------------------------------------
> +$ git show-ref --symbolic-name
> +b75428bae1d090f60bdd4b67185f814bc8f0819d refs/heads/SYMBOLIC_REF ref:refs/heads/main
> +...
> +-----------------------------------------------------------------------------
> +
> EXAMPLES
> --------
>
> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> index 1c15421e600..1d681505eac 100644
> --- a/builtin/show-ref.c
> +++ b/builtin/show-ref.c
> @@ -12,7 +12,7 @@
> static const char * const show_ref_usage[] = {
> N_("git show-ref [--head] [-d | --dereference]\n"
> " [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]\n"
> - " [--heads] [--] [<pattern>...]"),
> + " [--heads] [--symbolic-name] [--] [<pattern>...]"),
> N_("git show-ref --verify [-q | --quiet] [-d | --dereference]\n"
> " [-s | --hash[=<n>]] [--abbrev[=<n>]]\n"
> " [--] [<ref>...]"),
> @@ -26,10 +26,13 @@ struct show_one_options {
> int hash_only;
> int abbrev;
> int deref_tags;
> + int symbolic_name;
> };
>
> static void show_one(const struct show_one_options *opts,
> - const char *refname, const struct object_id *oid)
> + const char *refname,
> + const char *referent,
> + const struct object_id *oid, const int is_symref)
> {
> const char *hex;
> struct object_id peeled;
> @@ -44,7 +47,9 @@ static void show_one(const struct show_one_options *opts,
> hex = repo_find_unique_abbrev(the_repository, oid, opts->abbrev);
> if (opts->hash_only)
> printf("%s\n", hex);
> - else
> + else if (opts->symbolic_name & is_symref) {
> + printf("%s %s ref:%s\n", hex, refname, referent);
> + } else
> printf("%s %s\n", hex, refname);
>
> if (!opts->deref_tags)
> @@ -63,8 +68,11 @@ struct show_ref_data {
> int show_head;
> };
>
> -static int show_ref(const char *refname, const struct object_id *oid,
> - int flag UNUSED, void *cbdata)
> +static int show_ref_referent(struct repository *repo UNUSED,
> + const char *refname,
> + const char *referent,
> + const struct object_id *oid,
> + int flag, void *cbdata)
> {
> struct show_ref_data *data = cbdata;
>
> @@ -91,11 +99,17 @@ static int show_ref(const char *refname, const struct object_id *oid,
> match:
> data->found_match++;
>
> - show_one(data->show_one_opts, refname, oid);
> + show_one(data->show_one_opts, refname, referent, oid, flag & REF_ISSYMREF);
>
> return 0;
> }
>
> +static int show_ref(const char *refname, const struct object_id *oid,
> + int flag, void *cbdata)
> +{
> + return show_ref_referent(NULL, refname, NULL, oid, flag, cbdata);
> +}
> +
> static int add_existing(const char *refname,
> const struct object_id *oid UNUSED,
> int flag UNUSED, void *cbdata)
> @@ -171,10 +185,11 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts,
>
> while (*refs) {
> struct object_id oid;
> + int flags = 0;
>
> if ((starts_with(*refs, "refs/") || refname_is_safe(*refs)) &&
> - !read_ref(*refs, &oid)) {
> - show_one(show_one_opts, *refs, &oid);
> + !read_ref_full(*refs, 0, &oid, &flags)) {
> + show_one(show_one_opts, *refs, NULL, &oid, flags & REF_ISSYMREF);
> }
> else if (!show_one_opts->quiet)
> die("'%s' - not a valid ref", *refs);
> @@ -208,11 +223,11 @@ static int cmd_show_ref__patterns(const struct patterns_options *opts,
> head_ref(show_ref, &show_ref_data);
> if (opts->heads_only || opts->tags_only) {
> if (opts->heads_only)
> - for_each_fullref_in("refs/heads/", show_ref, &show_ref_data);
> + for_each_ref_all("refs/heads/", show_ref_referent, &show_ref_data);
> if (opts->tags_only)
> - for_each_fullref_in("refs/tags/", show_ref, &show_ref_data);
> + for_each_ref_all("refs/tags/", show_ref_referent, &show_ref_data);
> } else {
> - for_each_ref(show_ref, &show_ref_data);
> + for_each_ref_all("", show_ref_referent, &show_ref_data);
> }
> if (!show_ref_data.found_match)
> return 1;
> @@ -289,6 +304,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
> OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")),
> OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")),
> OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")),
> + OPT_BOOL(0, "symbolic-name", &show_one_opts.symbolic_name, N_("print out symbolic reference values")),
> OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
> "requires exact ref path")),
> OPT_HIDDEN_BOOL('h', NULL, &patterns_opts.show_head,
> diff --git a/refs.c b/refs.c
> index 77ae38ea214..9488ad9594d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1734,6 +1734,12 @@ int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_dat
> DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
> }
>
> +int for_each_ref_all(const char *prefix, each_repo_ref_fn fn, void *cb_data)
> +{
> + return do_for_each_repo_ref(the_repository, prefix, fn, 0,
> + 0, cb_data);
> +}
> +
> int for_each_namespaced_ref(const char **exclude_patterns,
> each_ref_fn fn, void *cb_data)
> {
> diff --git a/refs.h b/refs.h
> index 23e5aaba2e9..54b459375be 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -337,7 +337,6 @@ int refs_for_each_branch_ref(struct ref_store *refs,
> each_ref_fn fn, void *cb_data);
> int refs_for_each_remote_ref(struct ref_store *refs,
> each_ref_fn fn, void *cb_data);
> -
> /* just iterates the head ref. */
> int head_ref(each_ref_fn fn, void *cb_data);
>
> @@ -381,6 +380,7 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data);
> int for_each_branch_ref(each_ref_fn fn, void *cb_data);
> int for_each_remote_ref(each_ref_fn fn, void *cb_data);
> int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);
> +int for_each_ref_all(const char *prefix, each_repo_ref_fn fn, void *cb_data);
>
> /* iterates all refs that match the specified glob pattern. */
> int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
> diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
> index 33fb7a38fff..0aebe709dca 100755
> --- a/t/t1403-show-ref.sh
> +++ b/t/t1403-show-ref.sh
> @@ -286,4 +286,24 @@ test_expect_success '--exists with existing special ref' '
> git show-ref --exists FETCH_HEAD
> '
>
> +test_expect_success '--symbolic-name with a non symbolic ref' '
> + commit_oid=$(git rev-parse refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME) &&
> + cat >expect <<-EOF &&
> + $commit_oid refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> + EOF
> + git show-ref --symbolic-name refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success '--symbolic-name with symbolic ref' '
> + test_when_finished "git symbolic-ref -d refs/heads/SYMBOLIC_REF_A" &&
> + commit_oid=$(git rev-parse refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME) &&
> + cat >expect <<-EOF &&
> + $commit_oid refs/heads/SYMBOLIC_REF_A ref:refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> + EOF
> + git symbolic-ref refs/heads/SYMBOLIC_REF_A refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME &&
> + git show-ref --symbolic-name SYMBOLIC_REF_A >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
> --
> gitgitgadget
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-04-10 6:53 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-04 22:51 [PATCH] show-ref: add --unresolved option John Cai via GitGitGadget
2024-03-04 23:23 ` Junio C Hamano
2024-03-05 20:56 ` John Cai
2024-03-05 21:29 ` Junio C Hamano
2024-03-05 15:30 ` Phillip Wood
2024-03-05 17:01 ` Kristoffer Haugsbakk
2024-03-06 0:33 ` Jeff King
2024-03-06 2:19 ` Junio C Hamano
2024-03-06 0:41 ` Jeff King
2024-03-06 7:31 ` Patrick Steinhardt
2024-03-06 7:51 ` Jeff King
2024-03-06 16:48 ` Junio C Hamano
2024-03-06 9:36 ` Jean-Noël Avila
2024-04-08 17:38 ` [PATCH v2 0/3] show-ref: add --symbolic-name option John Cai via GitGitGadget
2024-04-08 17:38 ` [PATCH v2 1/3] refs: keep track of unresolved reference value in iterator John Cai via GitGitGadget
2024-04-08 23:02 ` Junio C Hamano
2024-04-09 20:29 ` John Cai
2024-04-10 6:52 ` Patrick Steinhardt
2024-04-10 15:26 ` Junio C Hamano
2024-04-11 9:11 ` Patrick Steinhardt
2024-04-08 17:38 ` [PATCH v2 2/3] refs: add referent to each_repo_ref_fn John Cai via GitGitGadget
2024-04-08 17:38 ` [PATCH v2 3/3] show-ref: add --symbolic-name option John Cai via GitGitGadget
2024-04-09 15:25 ` Phillip Wood
2024-04-11 19:57 ` John Cai
2024-04-12 9:37 ` phillip.wood123
2024-04-10 6:53 ` Patrick Steinhardt [this message]
2024-04-10 15:27 ` Junio C Hamano
2024-04-12 15:23 ` John Cai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZhY3UCwJeMcG4fH1@tanuki \
--to=ps@pks.im \
--cc=avila.jn@gmail.com \
--cc=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johncai86@gmail.com \
--cc=peff@peff.net \
--cc=phillip.wood123@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).