Re: [PATCH v4 5/5] for-each-ref: add new option to include root refs

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 9044 bytes --]

On Sun, Feb 11, 2024 at 07:39:23PM +0100, Karthik Nayak wrote:
> The git-for-each-ref(1) command doesn't provide a way to print root refs
> i.e pseudorefs and HEAD with the regular "refs/" prefixed refs.
> 
> This commit adds a new option "--include-root-refs" to
> git-for-each-ref(1). When used this would also print pseudorefs and HEAD
> for the current worktree.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  5 ++++-
>  builtin/for-each-ref.c             | 11 ++++++++---
>  ref-filter.c                       | 27 +++++++++++++++++++++++++-
>  ref-filter.h                       |  5 ++++-
>  refs/reftable-backend.c            | 11 +++++++----
>  t/t6302-for-each-ref-filter.sh     | 31 ++++++++++++++++++++++++++++++
>  6 files changed, 80 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 3a9ad91b7a..c1dd12b93c 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  [verse]
>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>  		   [(--sort=<key>)...] [--format=<format>]
> -		   [ --stdin | <pattern>... ]
> +		   [--include-root-refs] [ --stdin | <pattern>... ]
>  		   [--points-at=<object>]
>  		   [--merged[=<object>]] [--no-merged[=<object>]]
>  		   [--contains[=<object>]] [--no-contains[=<object>]]
> @@ -105,6 +105,9 @@ TAB %(refname)`.
>  	any excluded pattern(s) are shown. Matching is done using the
>  	same rules as `<pattern>` above.
>  
> +--include-root-refs::
> +	List root refs (HEAD and pseudorefs) apart from regular refs.
> +
>  FIELD NAMES
>  -----------
>  
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 23d352e371..9ed146dad3 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -20,10 +20,10 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  {
>  	struct ref_sorting *sorting;
>  	struct string_list sorting_options = STRING_LIST_INIT_DUP;
> -	int icase = 0;
> +	int icase = 0, include_root_refs = 0, from_stdin = 0;
>  	struct ref_filter filter = REF_FILTER_INIT;
>  	struct ref_format format = REF_FORMAT_INIT;
> -	int from_stdin = 0;
> +	unsigned int flags = FILTER_REFS_REGULAR;
>  	struct strvec vec = STRVEC_INIT;
>  
>  	struct option opts[] = {
> @@ -53,6 +53,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")),
>  		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
>  		OPT_BOOL(0, "stdin", &from_stdin, N_("read reference patterns from stdin")),
> +		OPT_BOOL(0, "include-root-refs", &include_root_refs, N_("also include HEAD ref and pseudorefs")),
>  		OPT_END(),
>  	};
>  
> @@ -96,8 +97,12 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		filter.name_patterns = argv;
>  	}
>  
> +	if (include_root_refs) {
> +		flags |= FILTER_REFS_ROOT_REFS;
> +	}

Nit: we don't use braces for single-line blocks.

> +
>  	filter.match_as_path = 1;
> -	filter_and_format_refs(&filter, FILTER_REFS_REGULAR, sorting, &format);
> +	filter_and_format_refs(&filter, flags, sorting, &format);
>  
>  	ref_filter_clear(&filter);
>  	ref_sorting_release(sorting);
> diff --git a/ref-filter.c b/ref-filter.c
> index acb960e35c..0e83e29390 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -2628,6 +2628,12 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
>  				       each_ref_fn cb,
>  				       void *cb_data)
>  {
> +	if (filter->kind == FILTER_REFS_KIND_MASK) {
> +		/* in this case, we want to print all refs including root refs. */

Nit: s/in/In, as this is a full sentence.

> +		return refs_for_each_include_root_refs(get_main_ref_store(the_repository),
> +						       cb, cb_data);
> +	}
> +
>  	if (!filter->match_as_path) {
>  		/*
>  		 * in this case, the patterns are applied after
> @@ -2750,6 +2756,9 @@ static int ref_kind_from_refname(const char *refname)
>  			return ref_kind[i].kind;
>  	}
>  
> +	if (is_pseudoref(get_main_ref_store(the_repository), refname))
> +		return FILTER_REFS_PSEUDOREFS;
> +
>  	return FILTER_REFS_OTHERS;
>  }
>  
> @@ -2781,6 +2790,16 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const struct
>  
>  	/* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */
>  	kind = filter_ref_kind(filter, refname);
> +
> +	/*
> +	 * When printing HEAD with all other refs, we want to apply the same formatting
> +	 * rules as the other refs, so we simply ask it to be treated as a pseudoref.
> +	 */
> +	if (filter->kind == FILTER_REFS_KIND_MASK && kind == FILTER_REFS_DETACHED_HEAD)
> +		kind = FILTER_REFS_PSEUDOREFS;

I'm not sure why exactly we need to munge the kind here. Would be great
if the comment explained what the actual difference would be.

> +	else if (!(kind & filter->kind))
> +		return NULL;
> +
>  	if (!(kind & filter->kind))
>  		return NULL;

This condition here is duplicated now, isn't it?

Patrick

> @@ -3049,7 +3068,13 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
>  			ret = for_each_fullref_in("refs/tags/", fn, cb_data);
>  		else if (filter->kind & FILTER_REFS_REGULAR)
>  			ret = for_each_fullref_in_pattern(filter, fn, cb_data);
> -		if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
> +
> +		/*
> +		 * When printing all ref types, HEAD is already included,
> +		 * so we don't want to print HEAD again.
> +		 */
> +		if (!ret && (filter->kind != FILTER_REFS_KIND_MASK) &&
> +		    (filter->kind & FILTER_REFS_DETACHED_HEAD))
>  			head_ref(fn, cb_data);
>  	}
>  
> diff --git a/ref-filter.h b/ref-filter.h
> index 5416936800..0ca28d2bba 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -22,7 +22,10 @@
>  #define FILTER_REFS_REGULAR        (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \
>  				    FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)
>  #define FILTER_REFS_DETACHED_HEAD  0x0020
> -#define FILTER_REFS_KIND_MASK      (FILTER_REFS_REGULAR | FILTER_REFS_DETACHED_HEAD)
> +#define FILTER_REFS_PSEUDOREFS     0x0040
> +#define FILTER_REFS_ROOT_REFS      (FILTER_REFS_DETACHED_HEAD | FILTER_REFS_PSEUDOREFS)
> +#define FILTER_REFS_KIND_MASK      (FILTER_REFS_REGULAR | FILTER_REFS_DETACHED_HEAD | \
> +				    FILTER_REFS_PSEUDOREFS)
>  
>  struct atom_value;
>  struct ref_sorting;
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index a14f2ad7f4..c23a516ac2 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -364,12 +364,15 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
>  			break;
>  
>  		/*
> -		 * The files backend only lists references contained in
> -		 * "refs/". We emulate the same behaviour here and thus skip
> -		 * all references that don't start with this prefix.
> +		 * The files backend only lists references contained in "refs/" unless
> +		 * the root refs are to be included. We emulate the same behaviour here.
>  		 */
> -		if (!starts_with(iter->ref.refname, "refs/"))
> +		if (!starts_with(iter->ref.refname, "refs/") &&
> +		    !(iter->flags & DO_FOR_EACH_INCLUDE_ROOT_REFS &&
> +		     (is_pseudoref(&iter->refs->base, iter->ref.refname) ||
> +		      is_headref(&iter->refs->base, iter->ref.refname)))) {
>  			continue;
> +		}
>  
>  		if (iter->prefix &&
>  		    strncmp(iter->prefix, iter->ref.refname, strlen(iter->prefix))) {
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> index 82f3d1ea0f..948f1bb5f4 100755
> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -31,6 +31,37 @@ test_expect_success 'setup some history and refs' '
>  	git update-ref refs/odd/spot main
>  '
>  
> +test_expect_success '--include-root-refs pattern prints pseudorefs' '
> +	cat >expect <<-\EOF &&
> +	HEAD
> +	ORIG_HEAD
> +	refs/heads/main
> +	refs/heads/side
> +	refs/odd/spot
> +	refs/tags/annotated-tag
> +	refs/tags/doubly-annotated-tag
> +	refs/tags/doubly-signed-tag
> +	refs/tags/four
> +	refs/tags/one
> +	refs/tags/signed-tag
> +	refs/tags/three
> +	refs/tags/two
> +	EOF
> +	git update-ref ORIG_HEAD main &&
> +	git for-each-ref --format="%(refname)" --include-root-refs >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--include-root-refs with other patterns' '
> +	cat >expect <<-\EOF &&
> +	HEAD
> +	ORIG_HEAD
> +	EOF
> +	git update-ref ORIG_HEAD main &&
> +	git for-each-ref --format="%(refname)" --include-root-refs "*HEAD" >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'filtering with --points-at' '
>  	cat >expect <<-\EOF &&
>  	refs/heads/main
> -- 
> 2.43.GIT
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]