[PATCH v2 00/17] refs: unify `refs_for_each_*()` functions
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 23, 2026, 11:59 UTC
- Message-ID
- <20260223-pks-refs-for-each-unification-v2-0-515d48c8087b@pks.im>
- In-Reply-To
- <20260220-pks-refs-for-each-unification-v1-0-17170bd99de1@pks.im>
Hi,
we currently have 14 different `refs_for_each_*()` functions, with each of them doing slightly different things. This makes for a confusing API surface, and because the API is not built for extension we have to add a new function every now and then to handle another esoteric edge case that will ultimately only have at most a handful of callers.
This design isn't really sensible in my opinion, and this patch series aims to fix that. Instead of having a dozen different functions, it introduces a new `refs_for_each_ref_ext()` function that simply takes an options structure as input. From thereon, callers can mix and match the parameters that they care about.
The patch series is structured like this:
- Patches 1 to 5 introduce some preliminary cleanups.
- Patches 6 to 9 introduce `refs_for_each_ref_ext()` and move
more functionality into it. This also fixes a performance bug that
we have in one of the implementations. - Patch 10 adds some more verification for options that would have
caught the bugs in ps/for-each-ref-in-fixes. - The remaining patches drop 7 out of 14 functions and replace them
with `refs_for_each_ref_ext()`. It results in a bit of churn, so
while I think this churn is worth it, I consider these patches to be
optional.The patch series is built on top of 73fd77805f (The 5th batch, 2026-02-17) with ps/for-each-ref-in-fixes at 6375a00ef1 (bisect: simplify string_list memory handling, 2026-02-19) merged into it.
Changes in v2:
- Move the removal of `refs_for_each_include_root_ref()` to the
beginning of the series to avoid some unnecessary churn.
- Some commit message improvements.
- Make the converted version of `refs_for_each_glob_ref_in()` fit into
the new calling conventions a bit better. The function was still
stripping the prefix unconditionally for example, which I've now
changed.
- Link to v1: https://lore.kernel.org/r/20260220-pks-refs-for-each-unification-v1-0-17170bd99de1@pks.imThanks!
Patrick
---
Patrick Steinhardt (17):
refs: remove unused `refs_for_each_include_root_ref()`
refs: move `refs_head_ref_namespaced()`
refs: move `do_for_each_ref_flags` further up
refs: rename `do_for_each_ref_flags`
refs: rename `each_ref_fn`
refs: introduce `refs_for_each_ref_ext`
refs: speed up `refs_for_each_glob_ref_in()`
refs: generalize `refs_for_each_namespaced_ref()`
refs: generalize `refs_for_each_fullref_in_prefixes()`
refs: improve verification for-each-ref options
refs: replace `refs_for_each_ref_in()`
refs: replace `refs_for_each_rawref()`
refs: replace `refs_for_each_rawref_in()`
refs: replace `refs_for_each_glob_ref_in()`
refs: replace `refs_for_each_glob_ref()`
refs: replace `refs_for_each_namespaced_ref()`
refs: replace `refs_for_each_fullref_in()`bisect.c | 16 ++- builtin/bisect.c | 37 +++++-- builtin/describe.c | 7 +- builtin/fetch.c | 7 +- builtin/fsck.c | 7 +- builtin/receive-pack.c | 8 +- builtin/remote.c | 8 +- builtin/rev-parse.c | 38 ++++--- builtin/show-ref.c | 21 ++-- fetch-pack.c | 15 ++- http-backend.c | 8 +- ls-refs.c | 11 +- notes.c | 7 +- pack-bitmap.c | 15 +-- pack-bitmap.h | 2 +- ref-filter.c | 19 ++-- refs.c | 271 ++++++++++++++++++++++------------------------ refs.h | 199 +++++++++++++++++----------------- refs/files-backend.c | 19 ++-- refs/iterator.c | 2 +- refs/packed-backend.c | 8 +- refs/reftable-backend.c | 10 +- revision.c | 49 ++++++--- submodule.c | 2 +- t/helper/test-ref-store.c | 15 ++- upload-pack.c | 13 ++- worktree.c | 2 +- worktree.h | 2 +- 28 files changed, 457 insertions(+), 361 deletions(-)
Range-diff versus v1:
-: ---------- > 1: 97473a19a8 refs: remove unused `refs_for_each_include_root_ref()`
1: 312fde9bc7 ! 2: 625d8bde9d refs: move `refs_head_ref_namespaced()`
@@ Commit message
The function `refs_head_ref_namespaced()` is somewhat special when
compared to most of the other functions that take a callback function:
- while `refs_for_each_*()` functions yield multiple refs, we only yield
- at most the HEAD ref of the current function. As such, the function is
- related to `refs_head_ref()` and not to the for-each functions.
+ while `refs_for_each_*()` functions yield multiple refs,
+ `refs_heasd_ref_namespaced()` will only yield at most the HEAD ref of
+ the current namespace. As such, the function is related to
+ `refs_head_ref()` and not to the for-each functions.
Move the function to be located next to `refs_head_ref()` to clarify.
2: fd0fa20a37 = 3: 2f5a6e7d27 refs: move `do_for_each_ref_flags` further up
3: f11af1c8e7 ! 4: a817b11091 refs: rename `do_for_each_ref_flags`
@@ refs.c: int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix,
+ REFS_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
- int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn,
- void *cb_data)
- {
- return do_for_each_ref(refs, "", NULL, fn, 0,
-- DO_FOR_EACH_INCLUDE_ROOT_REFS, cb_data);
-+ REFS_FOR_EACH_INCLUDE_ROOT_REFS, cb_data);
- }
-
static int qsort_strcmp(const void *va, const void *vb)
@@ refs.c: enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
4: 37be4c5f59 ! 5: f06b4fc5e4 refs: rename `each_ref_fn`
@@ refs.c: int refs_for_each_namespaced_ref(struct ref_store *refs,
{
return do_for_each_ref(refs, prefix, NULL, fn, 0,
REFS_FOR_EACH_INCLUDE_BROKEN, cb_data);
- }
-
--int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn,
-+int refs_for_each_include_root_refs(struct ref_store *refs, refs_for_each_cb fn,
- void *cb_data)
- {
- return do_for_each_ref(refs, "", NULL, fn, 0,
@@ refs.c: int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store,
const char *namespace,
const char **patterns,
@@ refs.h: int refs_for_each_glob_ref_in(struct ref_store *refs, each_ref_fn fn,
+ refs_for_each_cb fn, void *cb_data);
/*
- * Iterates over all refs including root refs, i.e. pseudorefs and HEAD.
- */
--int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn,
-+int refs_for_each_include_root_refs(struct ref_store *refs, refs_for_each_cb fn,
- void *cb_data);
-
- /*
+ * Normalizes partial refs to their fully qualified form.
@@ refs.h: void ref_iterator_free(struct ref_iterator *ref_iterator);
* iterator style.
*/
5: d70867c5f6 < -: ---------- refs: remove unused `refs_for_each_include_root_ref()`
6: b22f654698 = 6: 58620a64dd refs: introduce `refs_for_each_ref_ext`
7: 0a050b61f7 ! 7: 2ca0ddb23a refs: speed up `refs_for_each_glob_ref_in()`
@@ Commit message
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## refs.c ##
+@@ refs.c: char *refs_resolve_refdup(struct ref_store *refs,
+ /* The argument to for_each_filter_refs */
+ struct for_each_ref_filter {
+ const char *pattern;
+- const char *prefix;
++ size_t trim_prefix;
+ refs_for_each_cb *fn;
+ void *cb_data;
+ };
+@@ refs.c: static int for_each_filter_refs(const struct reference *ref, void *data)
+
+ if (wildmatch(filter->pattern, ref->name, 0))
+ return 0;
+- if (filter->prefix) {
++ if (filter->trim_prefix) {
+ struct reference skipped = *ref;
+- skip_prefix(skipped.name, filter->prefix, &skipped.name);
++ if (strlen(skipped.name) <= filter->trim_prefix)
++ BUG("attempt to trim too many characters");
++ skipped.name += filter->trim_prefix;
+ return filter->fn(&skipped, filter->cb_data);
+ } else {
+ return filter->fn(ref, filter->cb_data);
@@ refs.c: void normalize_glob_ref(struct string_list_item *item, const char *prefix,
strbuf_release(&normalized_pattern);
}
@@ refs.c: void normalize_glob_ref(struct string_list_item *item, const char *prefi
+ struct refs_for_each_ref_options opts = {
+ .pattern = pattern,
+ .prefix = prefix,
++ .trim_prefix = prefix ? strlen(prefix) : 0,
+ };
+ return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
}
@@ refs.c: int refs_for_each_ref_ext(struct ref_store *refs,
+ struct strbuf real_pattern = STRBUF_INIT;
+ struct for_each_ref_filter filter;
struct ref_iterator *iter;
++ size_t trim_prefix = opts->trim_prefix;
+ int ret;
if (!refs)
@@ refs.c: int refs_for_each_ref_ext(struct ref_store *refs,
+ }
+
+ filter.pattern = real_pattern.buf;
-+ filter.prefix = opts->prefix;
++ filter.trim_prefix = opts->trim_prefix;
+ filter.fn = cb;
+ filter.cb_data = cb_data;
+
++ /*
++ * We need to trim the prefix in the callback function as the
++ * pattern is expected to match on the full refname.
++ */
++ trim_prefix = 0;
++
+ cb = for_each_filter_refs;
+ cb_data = &filter;
+ }
+
iter = refs_ref_iterator_begin(refs, opts->prefix ? opts->prefix : "",
opts->exclude_patterns,
- opts->trim_prefix, opts->flags);
+- opts->trim_prefix, opts->flags);
++ trim_prefix, opts->flags);
- return do_for_each_ref_iterator(iter, cb, cb_data);
+ ret = do_for_each_ref_iterator(iter, cb, cb_data);
8: b15d334f14 ! 8: 01a640a61d refs: generalize `refs_for_each_namespaced_ref()`
@@ refs.c: int refs_for_each_ref_ext(struct ref_store *refs,
struct strbuf real_pattern = STRBUF_INIT;
struct for_each_ref_filter filter;
struct ref_iterator *iter;
+ size_t trim_prefix = opts->trim_prefix;
+ const char **exclude_patterns;
+ const char *prefix;
int ret;
@@ refs.c: int refs_for_each_ref_ext(struct ref_store *refs,
+ }
+
+ iter = refs_ref_iterator_begin(refs, prefix, exclude_patterns,
- opts->trim_prefix, opts->flags);
+ trim_prefix, opts->flags);
ret = do_for_each_ref_iterator(iter, cb, cb_data);
+
@@ refs.h: struct refs_for_each_ref_options {
+ /*
+ * If set, only yield refs part of the configured namespace. Exclude
-+ * patterns will be rewritten to apply to the namespace.
++ * patterns will be rewritten to apply to the namespace, and the prefix
++ * will be considered relative to the namespace.
+ */
+ const char *namespace;
+
9: 2e63b1ab88 = 9: 241030d7ad refs: generalize `refs_for_each_fullref_in_prefixes()`
10: b408e5c1f0 ! 10: 548aae78f0 refs: improve verification for-each-ref options
@@ refs.c: int refs_for_each_ref_ext(struct ref_store *refs,
if (!refs)
- return 0;
-+ BUG("no refs passed");
++ BUG("no ref store passed");
+
+ if (opts->trim_prefix) {
+ size_t prefix_len;
11: 5c9401df32 = 11: b0a5c835be refs: replace `refs_for_each_ref_in()`
12: 39a5f1ef21 = 12: 59f5632719 refs: replace `refs_for_each_rawref()`
13: 5568ee95d0 = 13: 8b99f6e38c refs: replace `refs_for_each_rawref_in()`
14: d73e6362ae ! 14: ed75a64569 refs: replace `refs_for_each_glob_ref_in()`
@@ builtin/bisect.c: static void bisect_status(struct bisect_state *state,
+ struct refs_for_each_ref_options opts = {
+ .pattern = good_glob,
+ .prefix = "refs/bisect/",
++ .trim_prefix = strlen("refs/bisect/"),
+ };
if (refs_ref_exists(get_main_ref_store(the_repository), bad_ref))
@@ builtin/bisect.c: static int add_bisect_ref(const struct reference *ref, void *c
{
+ struct refs_for_each_ref_options opts = {
+ .prefix = "refs/bisect/",
++ .trim_prefix = strlen("refs/bisect/"),
+ };
int res = 0;
struct add_bisect_ref_data cb = { revs };
@@ builtin/bisect.c: static int verify_good(const struct bisect_terms *terms, const
+ struct refs_for_each_ref_options opts = {
+ .pattern = good_glob,
+ .prefix = "refs/bisect/",
++ .trim_prefix = strlen("refs/bisect/"),
+ };
- refs_for_each_glob_ref_in(get_main_ref_store(the_repository),
@@ builtin/rev-parse.c: static int opt_with_value(const char *arg, const char *opt,
+ struct refs_for_each_ref_options opts = {
+ .pattern = pattern,
+ .prefix = prefix,
++ .trim_prefix = prefix ? strlen(prefix) : 0,
+ };
+ refs_for_each_ref_ext(get_main_ref_store(the_repository),
+ show_reference, NULL, &opts);
@@ refs.c: void normalize_glob_ref(struct string_list_item *item, const char *prefi
- struct refs_for_each_ref_options opts = {
- .pattern = pattern,
- .prefix = prefix,
+- .trim_prefix = prefix ? strlen(prefix) : 0,
- };
- return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
-}
@@ revision.c: static int handle_revision_pseudo_opt(struct rev_info *revs,
} else if (skip_prefix(arg, "--branches=", &optarg)) {
+ struct refs_for_each_ref_options opts = {
+ .prefix = "refs/heads/",
++ .trim_prefix = strlen("refs/heads/"),
+ .pattern = optarg,
+ };
struct all_refs_cb cb;
@@ revision.c: static int handle_revision_pseudo_opt(struct rev_info *revs,
} else if (skip_prefix(arg, "--tags=", &optarg)) {
+ struct refs_for_each_ref_options opts = {
+ .prefix = "refs/tags/",
++ .trim_prefix = strlen("refs/tags/"),
+ .pattern = optarg,
+ };
struct all_refs_cb cb;
@@ revision.c: static int handle_revision_pseudo_opt(struct rev_info *revs,
} else if (skip_prefix(arg, "--remotes=", &optarg)) {
+ struct refs_for_each_ref_options opts = {
+ .prefix = "refs/remotes/",
++ .trim_prefix = strlen("refs/remotes/"),
+ .pattern = optarg,
+ };
struct all_refs_cb cb;
15: c957d80f2d = 15: 6256a04ecf refs: replace `refs_for_each_glob_ref()`
16: 229d69d91c = 16: ef356544bf refs: replace `refs_for_each_namespaced_ref()`
17: 4e0fe9f805 = 17: 4616cdc618 refs: replace `refs_for_each_fullref_in()`--- base-commit: dbbe43524e0814c1f93325795ed6aa26eb6e587e change-id: 20260220-pks-refs-for-each-unification-7572c694cfc0