On Tue, Nov 07, 2023 at 01:26:00AM +0000, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@xxxxxxxxxx>
>
> Add a boolean flag '--full-deref' that, when enabled, fills '%(*fieldname)'
> format fields using the fully peeled target of tag objects, rather than the
> immediate target.
>
> In other builtins ('rev-parse', 'show-ref'), "dereferencing" tags typically
> means peeling them down to their non-tag target. Unlike these commands,
> 'for-each-ref' dereferences only one "level" of tags in '*' format fields
> (like "%(*objectname)"). For most annotated tags, one level of dereferencing
> is enough, since most tags point to commits or trees. However, nested tags
> (annotated tags whose target is another annotated tag) dereferenced once
> will point to their target tag, different a full peel to e.g. a commit.
>
> Currently, if a user wants to filter & format refs and include information
> about the fully dereferenced tag, they can do so with something like
> 'cat-file --batch-check':
>
> git for-each-ref --format="%(objectname)^{} %(refname)" <pattern> |
> git cat-file --batch-check="%(objectname) %(rest)"
>
> But the combination of commands is inefficient. So, to improve the
> efficiency of this use case, add a '--full-deref' option that causes
> 'for-each-ref' to fully dereference tags when formatting with '*' fields.
I do wonder whether it would make sense to introduce this feature in the
form of a separate field prefix, as you also mentioned in your cover
letter. It would buy the user more flexibility, but the question is
whether such flexibility would really ever be needed.
The only thing I could really think of where it might make sense is to
distinguish tags that peel to a commit immediately from ones that don't.
That feels rather esoteric to me and doesn't seem to be of much use. But
regardless of whether or not we can see the usefulness now, if this
wouldn't be significantly more complex I wonder whether it would make
more sense to use a new field prefix instead anyway.
In any case, I think it would be helpful if this was discussed in the
commit message.
Patrick
> Signed-off-by: Victoria Dye <vdye@xxxxxxxxxx>
> ---
> Documentation/git-for-each-ref.txt | 9 ++++++++
> builtin/for-each-ref.c | 2 ++
> ref-filter.c | 26 ++++++++++++++---------
> ref-filter.h | 1 +
> t/t6300-for-each-ref.sh | 34 ++++++++++++++++++++++++++++++
> 5 files changed, 62 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 407f624fbaa..2714a87088e 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -11,6 +11,7 @@ SYNOPSIS
> 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
> [(--sort=<key>)...] [--format=<format>]
> [ --stdin | <pattern>... ]
> + [--full-deref]
> [--points-at=<object>]
> [--merged[=<object>]] [--no-merged[=<object>]]
> [--contains[=<object>]] [--no-contains[=<object>]]
> @@ -77,6 +78,14 @@ OPTIONS
> the specified host language. This is meant to produce
> a scriptlet that can directly be `eval`ed.
>
> +--full-deref::
> + Populate dereferenced format fields (indicated with an asterisk (`*`)
> + prefix before the fieldname) with information about the fully-peeled
> + target object of a tag ref, rather than its immediate target object.
> + This only affects the output for nested annotated tags, where the tag's
> + immediate target is another tag but its fully-peeled target is another
> + object type (e.g. a commit).
> +
> --points-at=<object>::
> Only list refs which points at the given object.
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 1c19cd5bd34..7a2127a3bc4 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -43,6 +43,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
> OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")),
> OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")),
> OPT__COLOR(&format.use_color, N_("respect format colors")),
> + OPT_BOOL(0, "full-deref", &format.full_deref,
> + N_("fully dereference tags to populate '*' format fields")),
> OPT_REF_FILTER_EXCLUDE(&filter),
> OPT_REF_SORT(&sorting_options),
> OPT_CALLBACK(0, "points-at", &filter.points_at,
> diff --git a/ref-filter.c b/ref-filter.c
> index 384cf1595ff..a66ac7921b1 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -237,7 +237,14 @@ static struct used_atom {
> char *head;
> } u;
> } *used_atom;
> -static int used_atom_cnt, need_tagged, need_symref;
> +static int used_atom_cnt, need_symref;
> +
> +enum tag_dereference_mode {
> + NO_DEREF = 0,
> + DEREF_ONE,
> + DEREF_ALL
> +};
> +static enum tag_dereference_mode need_tagged;
>
> /*
> * Expand string, append it to strbuf *sb, then return error code ret.
> @@ -1066,8 +1073,8 @@ static int parse_ref_filter_atom(struct ref_format *format,
> memset(&used_atom[at].u, 0, sizeof(used_atom[at].u));
> if (valid_atom[i].parser && valid_atom[i].parser(format, &used_atom[at], arg, err))
> return -1;
> - if (*atom == '*')
> - need_tagged = 1;
> + if (*atom == '*' && !need_tagged)
> + need_tagged = format->full_deref ? DEREF_ALL : DEREF_ONE;
> if (i == ATOM_SYMREF)
> need_symref = 1;
> return at;
> @@ -2511,14 +2518,13 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
> * If it is a tag object, see if we use a value that derefs
> * the object, and if we do grab the object it refers to.
> */
> - oi_deref.oid = *get_tagged_oid((struct tag *)obj);
> + if (need_tagged == DEREF_ALL) {
> + if (peel_iterated_oid(&obj->oid, &oi_deref.oid))
> + die("bad tag");
> + } else {
> + oi_deref.oid = *get_tagged_oid((struct tag *)obj);
> + }
>
> - /*
> - * NEEDSWORK: This derefs tag only once, which
> - * is good to deal with chains of trust, but
> - * is not consistent with what deref_tag() does
> - * which peels the onion to the core.
> - */
> return get_object(ref, 1, &obj, &oi_deref, err);
> }
>
> diff --git a/ref-filter.h b/ref-filter.h
> index 0ce5af58ab3..0caa39ecee5 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -92,6 +92,7 @@ struct ref_format {
> const char *rest;
> int quote_style;
> int use_color;
> + int full_deref;
>
> /* Internal state to ref-filter */
> int need_color_reset_at_eol;
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 0613e5e3623..3c2af785cdb 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -1839,6 +1839,40 @@ test_expect_success 'git for-each-ref with non-existing refs' '
> test_must_be_empty actual
> '
>
> +test_expect_success 'git for-each-ref with nested tags' '
> + git tag -am "Normal tag" nested/base HEAD &&
> + git tag -am "Nested tag" nested/nest1 refs/tags/nested/base &&
> + git tag -am "Double nested tag" nested/nest2 refs/tags/nested/nest1 &&
> +
> + head_oid="$(git rev-parse HEAD)" &&
> + base_tag_oid="$(git rev-parse refs/tags/nested/base)" &&
> + nest1_tag_oid="$(git rev-parse refs/tags/nested/nest1)" &&
> + nest2_tag_oid="$(git rev-parse refs/tags/nested/nest2)" &&
> +
> + # Without full dereference
> + cat >expect <<-EOF &&
> + refs/tags/nested/base $base_tag_oid tag $head_oid commit
> + refs/tags/nested/nest1 $nest1_tag_oid tag $base_tag_oid tag
> + refs/tags/nested/nest2 $nest2_tag_oid tag $nest1_tag_oid tag
> + EOF
> +
> + git for-each-ref --format="%(refname) %(objectname) %(objecttype) %(*objectname) %(*objecttype)" \
> + refs/tags/nested/ >actual &&
> + test_cmp expect actual &&
> +
> + # With full dereference
> + cat >expect <<-EOF &&
> + refs/tags/nested/base $base_tag_oid tag $head_oid commit
> + refs/tags/nested/nest1 $nest1_tag_oid tag $head_oid commit
> + refs/tags/nested/nest2 $nest2_tag_oid tag $head_oid commit
> + EOF
> +
> + git for-each-ref --full-deref \
> + --format="%(refname) %(objectname) %(objecttype) %(*objectname) %(*objecttype)" \
> + refs/tags/nested/ >actual &&
> + test_cmp expect actual
> +'
> +
> GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
> TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
>
> --
> gitgitgadget
>
>
Attachment:
signature.asc
Description: PGP signature
