git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags

From
Patrick Steinhardt <ps@pks.im>
Date
Nov 7, 2023, 10:50 UTC
Message-ID
<ZUoWWo7IEKsiSx-C@tanuki>
In-Reply-To
<352b5c42ac39d5d2646a1b6d47d6d707637db539.1699320362.git.gitgitgadget@gmail.com>
On Tue, Nov 07, 2023 at 01:26:00AM +0000, Victoria Dye via GitGitGadget wrote:
Show 24 quoted lines
> From: Victoria Dye <vdye@github.com>
> 
> 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
Show 162 quoted lines
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
>  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
> 
>
Previous: Victoria Dye via GitGitGadgetNext: Victoria Dye
Message 21 of 49 in “for-each-ref optimizations & usability improvements”
  1. 0/9 for-each-ref optimizations & usability improvementsVictoria Dye via GitGitGadget, Nov 7, 2023
  2. 2/9 for-each-ref: clarify interaction of --omit-empty & --countVictoria Dye via GitGitGadget, Nov 7, 2023
  3. Øystein WalleNov 7, 2023
  4. Victoria DyeNov 7, 2023
  5. Øystein WalleNov 8, 2023
  6. Kristoffer HaugsbakkNov 8, 2023
  7. 1/9 ref-filter.c: really don't sort when using --no-sortVictoria Dye via GitGitGadget, Nov 7, 2023
  8. Patrick SteinhardtNov 7, 2023
  9. Victoria DyeNov 7, 2023
  10. 3/9 ref-filter.h: add max_count and omit_empty to ref_formatVictoria Dye via GitGitGadget, Nov 7, 2023
  11. 4/9 ref-filter.h: move contains caches into filterVictoria Dye via GitGitGadget, Nov 7, 2023
  12. Patrick SteinhardtNov 7, 2023
  13. 5/9 ref-filter.h: add functions for filter/format & format-onlyVictoria Dye via GitGitGadget, Nov 7, 2023
  14. 6/9 ref-filter.c: refactor to create common helper functionsVictoria Dye via GitGitGadget, Nov 7, 2023
  15. Patrick SteinhardtNov 7, 2023
  16. Victoria DyeNov 7, 2023
  17. 7/9 ref-filter.c: filter & format refs in the same callbackVictoria Dye via GitGitGadget, Nov 7, 2023
  18. Patrick SteinhardtNov 7, 2023
  19. Victoria DyeNov 7, 2023
  20. 8/9 for-each-ref: add option to fully dereference tagsVictoria Dye via GitGitGadget, Nov 7, 2023
  21. Patrick SteinhardtNov 7, 2023
  22. Victoria DyeNov 8, 2023
  23. Junio C HamanoNov 8, 2023
  24. Patrick SteinhardtNov 8, 2023
  25. Victoria DyeNov 8, 2023
  26. Junio C HamanoNov 9, 2023
  27. Junio C HamanoNov 9, 2023
  28. Junio C HamanoNov 9, 2023
  29. 9/9 t/perf: add perf tests for for-each-refVictoria Dye via GitGitGadget, Nov 7, 2023
  30. Junio C HamanoNov 7, 2023
  31. Victoria DyeNov 7, 2023
  32. Junio C HamanoNov 7, 2023
  33. Patrick SteinhardtNov 7, 2023
  34. Victoria DyeNov 8, 2023
  35. 00/10 for-each-ref optimizations & usability improvementsVictoria Dye via GitGitGadget, Nov 14, 2023
  36. 01/10 ref-filter.c: really don't sort when using --no-sortVictoria Dye via GitGitGadget, Nov 14, 2023
  37. Junio C HamanoNov 16, 2023
  38. 02/10 ref-filter.h: add max_count and omit_empty to ref_formatVictoria Dye via GitGitGadget, Nov 14, 2023
  39. Øystein WalleNov 16, 2023
  40. 03/10 ref-filter.h: move contains caches into filterVictoria Dye via GitGitGadget, Nov 14, 2023
  41. 04/10 ref-filter.h: add functions for filter/format & format-onlyVictoria Dye via GitGitGadget, Nov 14, 2023
  42. Junio C HamanoNov 16, 2023
  43. 05/10 ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'Victoria Dye via GitGitGadget, Nov 14, 2023
  44. 06/10 ref-filter.c: refactor to create common helper functionsVictoria Dye via GitGitGadget, Nov 14, 2023
  45. 07/10 ref-filter.c: filter & format refs in the same callbackVictoria Dye via GitGitGadget, Nov 14, 2023
  46. 08/10 for-each-ref: clean up documentation of --formatVictoria Dye via GitGitGadget, Nov 14, 2023
  47. 09/10 ref-filter.c: use peeled tag for '*' format fieldsVictoria Dye via GitGitGadget, Nov 14, 2023
  48. Junio C HamanoNov 16, 2023
  49. 10/10 t/perf: add perf tests for for-each-refVictoria Dye via GitGitGadget, Nov 14, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.