Re: [PATCH v5 0/5] for-each-ref: add '--include-root-refs' option
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 27, 2024, 07:39 UTC
- Message-ID
- <Zd2Ru7LWYyGprvcr@tanuki>
- In-Reply-To
- <20240223100112.44127-1-karthik.188@gmail.com>
On Fri, Feb 23, 2024 at 11:01:07AM +0100, Karthik Nayak wrote:
Show 30 quoted lines
> This is the fifth version of my patch series to print root refs > in git-for-each-ref(1). > > With the introduction of the reftable backend, it becomes ever > so important to provide the necessary tooling for printing all refs > associated with a worktree. > > While regular refs stored within the "refs/" namespace are currently > supported by multiple commands like git-for-each-ref(1), > git-show-ref(1). Neither support printing root refs within the worktree. > > This patch series is a follow up to the RFC/discussion we had earlier on > the list [1]. > > The first 4 commits add the required functionality to ensure we can print > all refs (regular, pseudo, HEAD). The 5th commit modifies the > git-for-each-ref(1) command to add the "--include-root-refs" command which > will include HEAD and pseudorefs with regular "refs/" refs. > > [1]: https://lore.kernel.org/git/20231221170715.110565-1-karthik.188@gmail.com/#t > > Changes from v4: > 1. Fixed erratic whitespace > 2. Remove braces from single line block > 3. Starting the comments with a capital and also adding more context. > 4. Removed a duplicate check. > > Thanks for the reviews. > > Range diff against v4:
The range-diff looks as expected, so this patch series looks good to me. Thanks!
Patrick
Show 97 quoted lines
> 1: 98130a7ad7 ! 1: 6016042965 refs: introduce `is_pseudoref()` and `is_headref()`
> @@ refs.c: static int is_pseudoref_syntax(const char *refname)
> +
> + if (ends_with(refname, "_HEAD")) {
> + refs_resolve_ref_unsafe(refs, refname,
> -+ RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> -+ &oid, NULL);
> -+ return !is_null_oid(&oid);
> ++ RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> ++ &oid, NULL);
> ++ return !is_null_oid(&oid);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++)
> + if (!strcmp(refname, irregular_pseudorefs[i])) {
> + refs_resolve_ref_unsafe(refs, refname,
> -+ RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> -+ &oid, NULL);
> ++ RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> ++ &oid, NULL);
> + return !is_null_oid(&oid);
> + }
> +
> 2: 060ab08af5 = 2: acaa014841 refs: extract out `loose_fill_ref_dir_regular_file()`
> 3: 40d2375ad9 = 3: f51c5bc307 refs: introduce `refs_for_each_include_root_refs()`
> 4: b4b9435505 = 4: 7c004db6e7 ref-filter: rename 'FILTER_REFS_ALL' to 'FILTER_REFS_REGULAR'
> 5: ee99ac41ae ! 5: 53f6c0a6db for-each-ref: add new option to include root refs
> @@ builtin/for-each-ref.c: int cmd_for_each_ref(int argc, const char **argv, const
> filter.name_patterns = argv;
> }
>
> -+ if (include_root_refs) {
> ++ if (include_root_refs)
> + flags |= FILTER_REFS_ROOT_REFS;
> -+ }
> +
> filter.match_as_path = 1;
> - filter_and_format_refs(&filter, FILTER_REFS_REGULAR, sorting, &format);
> @@ ref-filter.c: static int for_each_fullref_in_pattern(struct ref_filter *filter,
> void *cb_data)
> {
> + if (filter->kind == FILTER_REFS_KIND_MASK) {
> -+ /* in this case, we want to print all refs including root refs. */
> ++ /* In this case, we want to print all refs including root refs. */
> + return refs_for_each_include_root_refs(get_main_ref_store(the_repository),
> + cb, cb_data);
> + }
> @@ ref-filter.c: static struct ref_array_item *apply_ref_filter(const char *refname
>
> /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */
> kind = filter_ref_kind(filter, refname);
> +- if (!(kind & filter->kind))
> +
> + /*
> -+ * 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.
> ++ * Generally HEAD refs are printed with special description denoting a rebase,
> ++ * detached state and so forth. This is useful when only printing the HEAD ref
> ++ * But when it is being printed along with other pseudorefs, it makes sense to
> ++ * keep the formatting consistent. So we mask the type to act like a pseudoref.
> + */
> + if (filter->kind == FILTER_REFS_KIND_MASK && kind == FILTER_REFS_DETACHED_HEAD)
> + kind = FILTER_REFS_PSEUDOREFS;
> + else if (!(kind & filter->kind))
> -+ return NULL;
> -+
> - if (!(kind & filter->kind))
> return NULL;
>
> + if (!filter_pattern_match(filter, refname))
> @@ ref-filter.c: 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)
>
>
> Karthik Nayak (5):
> refs: introduce `is_pseudoref()` and `is_headref()`
> refs: extract out `loose_fill_ref_dir_regular_file()`
> refs: introduce `refs_for_each_include_root_refs()`
> ref-filter: rename 'FILTER_REFS_ALL' to 'FILTER_REFS_REGULAR'
> for-each-ref: add new option to include root refs
>
> Documentation/git-for-each-ref.txt | 5 +-
> builtin/for-each-ref.c | 10 ++-
> ref-filter.c | 30 ++++++-
> ref-filter.h | 7 +-
> refs.c | 48 +++++++++++
> refs.h | 9 ++
> refs/files-backend.c | 127 +++++++++++++++++++++--------
> refs/refs-internal.h | 6 ++
> refs/reftable-backend.c | 11 ++-
> t/t6302-for-each-ref-filter.sh | 31 +++++++
> 10 files changed, 237 insertions(+), 47 deletions(-)
>
> --
> 2.43.GIT
>