refs: introduce `refs_for_each_ref_ext`

In the refs subsystem we have a proliferation of functions that all
iterate through references. (Almost) all of these functions internally
call `do_for_each_ref()` and provide slightly different arguments so
that one can control different aspects of its behaviour. This approach
doesn't really scale: every time there is a slightly different use case
for iterating through refs we create another new function.

This combinatorial explosion doesn't make a lot of sense: it leads to
confusing interfaces and heightens the maintenance burden.

Refactor the code to become more composable by:

  - Exposing `do_for_each_ref()` as `refs_for_each_ref_ext()`.

  - Introducing an options structure that lets the caller control
    individual options.

This gives us a much better foundation to build on going forward.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index a45cc612111840d63145ba32f11c509b4fe1981b..ec9e4663810c39c1583448c01708ca76faff5bfc 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1858,62 +1858,76 @@ struct ref_iterator *refs_ref_iterator_begin(
        return iter;
 }
 
-static int do_for_each_ref(struct ref_store *refs, const char *prefix,
-                          const char **exclude_patterns,
-                          refs_for_each_cb fn, int trim,
-                          enum refs_for_each_flag flags, void *cb_data)
+int refs_for_each_ref_ext(struct ref_store *refs,
+                         refs_for_each_cb cb, void *cb_data,
+                         const struct refs_for_each_ref_options *opts)
 {
        struct ref_iterator *iter;
 
        if (!refs)
                return 0;
 
-       iter = refs_ref_iterator_begin(refs, prefix, exclude_patterns, trim,
-                                      flags);
+       iter = refs_ref_iterator_begin(refs, opts->prefix ? opts->prefix : "",
+                                      opts->exclude_patterns,
+                                      opts->trim_prefix, opts->flags);
 
-       return do_for_each_ref_iterator(iter, fn, cb_data);
+       return do_for_each_ref_iterator(iter, cb, cb_data);
 }
 
-int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data)
+int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data)
 {
-       return do_for_each_ref(refs, "", NULL, fn, 0, 0, cb_data);
+       struct refs_for_each_ref_options opts = { 0 };
+       return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
 }
 
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
-                        refs_for_each_cb fn, void *cb_data)
+                        refs_for_each_cb cb, void *cb_data)
 {
-       return do_for_each_ref(refs, prefix, NULL, fn, strlen(prefix), 0, cb_data);
+       struct refs_for_each_ref_options opts = {
+               .prefix = prefix,
+               .trim_prefix = strlen(prefix),
+       };
+       return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
 }
 
 int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
                             const char **exclude_patterns,
-                            refs_for_each_cb fn, void *cb_data)
+                            refs_for_each_cb cb, void *cb_data)
 {
-       return do_for_each_ref(refs, prefix, exclude_patterns, fn, 0, 0, cb_data);
+       struct refs_for_each_ref_options opts = {
+               .prefix = prefix,
+               .exclude_patterns = exclude_patterns,
+       };
+       return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
 }
 
-int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data)
+int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data)
 {
        const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
-       return do_for_each_ref(refs, git_replace_ref_base, NULL, fn,
-                              strlen(git_replace_ref_base),
-                              REFS_FOR_EACH_INCLUDE_BROKEN, cb_data);
+       struct refs_for_each_ref_options opts = {
+               .prefix = git_replace_ref_base,
+               .trim_prefix = strlen(git_replace_ref_base),
+               .flags = REFS_FOR_EACH_INCLUDE_BROKEN,
+       };
+       return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
 }
 
 int refs_for_each_namespaced_ref(struct ref_store *refs,
                                 const char **exclude_patterns,
-                                refs_for_each_cb fn, void *cb_data)
+                                refs_for_each_cb cb, void *cb_data)
 {
+       struct refs_for_each_ref_options opts = { 0 };
        struct strvec namespaced_exclude_patterns = STRVEC_INIT;
        struct strbuf prefix = STRBUF_INIT;
        int ret;
 
-       exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns,
-                                                          get_git_namespace(),
-                                                          &namespaced_exclude_patterns);
-
+       opts.exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns,
+                                                               get_git_namespace(),
+                                                               &namespaced_exclude_patterns);
        strbuf_addf(&prefix, "%srefs/", get_git_namespace());
-       ret = do_for_each_ref(refs, prefix.buf, exclude_patterns, fn, 0, 0, cb_data);
+       opts.prefix = prefix.buf;
+
+       ret = refs_for_each_ref_ext(refs, cb, cb_data, &opts);
 
        strvec_clear(&namespaced_exclude_patterns);
        strbuf_release(&prefix);
@@ -1926,10 +1940,13 @@ int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_d
 }
 
 int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix,
-                           refs_for_each_cb fn, void *cb_data)
+                           refs_for_each_cb cb, void *cb_data)
 {
-       return do_for_each_ref(refs, prefix, NULL, fn, 0,
-                              REFS_FOR_EACH_INCLUDE_BROKEN, cb_data);
+       struct refs_for_each_ref_options opts = {
+               .prefix = prefix,
+               .flags = REFS_FOR_EACH_INCLUDE_BROKEN,
+       };
+       return refs_for_each_ref_ext(refs, cb, cb_data, &opts);
 }
 
 static int qsort_strcmp(const void *va, const void *vb)
@@ -3187,6 +3204,9 @@ int repo_migrate_ref_storage_format(struct repository *repo,
                                    struct strbuf *errbuf)
 {
        struct ref_store *old_refs = NULL, *new_refs = NULL;
+       struct refs_for_each_ref_options for_each_ref_opts = {
+               .flags = REFS_FOR_EACH_INCLUDE_ROOT_REFS | REFS_FOR_EACH_INCLUDE_BROKEN,
+       };
        struct ref_transaction *transaction = NULL;
        struct strbuf new_gitdir = STRBUF_INIT;
        struct migration_data data = {
@@ -3270,7 +3290,7 @@ int repo_migrate_ref_storage_format(struct repository *repo,
        data.errbuf = errbuf;
 
        /*
-        * We need to use the internal `do_for_each_ref()` here so that we can
+        * We need to use `refs_for_each_ref_ext()` here so that we can
         * also include broken refs and symrefs. These would otherwise be
         * skipped silently.
         *
@@ -3280,9 +3300,7 @@ int repo_migrate_ref_storage_format(struct repository *repo,
         * allow for a central lock due to its design. It's thus on the user to
         * ensure that there are no concurrent writes.
         */
-       ret = do_for_each_ref(old_refs, "", NULL, migrate_one_ref, 0,
-                             REFS_FOR_EACH_INCLUDE_ROOT_REFS | REFS_FOR_EACH_INCLUDE_BROKEN,
-                             &data);
+       ret = refs_for_each_ref_ext(old_refs, migrate_one_ref, &data, &for_each_ref_opts);
        if (ret < 0)
                goto done;
 

diff --git a/refs.h b/refs.h
index 5190e98b2c6e1d3437fb2e6aa6ec359390d43df1..bb9c64a51c0c75933c9c26fd4f8b9e02d8b1765c 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -453,8 +453,37 @@ int refs_head_ref(struct ref_store *refs,
 int refs_head_ref_namespaced(struct ref_store *refs,
                             refs_for_each_cb fn, void *cb_data);
 
+
+struct refs_for_each_ref_options {
+       /* Only iterate over references that have this given prefix. */
+       const char *prefix;
+
+       /*
+        * Exclude any references that match any of these patterns on a
+        * best-effort basis. The caller needs to be prepared for the exclude
+        * patterns to be ignored.
+        *
+        * The array must be terminated with a NULL sentinel value.
+        */
+       const char **exclude_patterns;
+
+       /*
+        * The number of bytes to trim from the refname. Note that the trimmed
+        * bytes must not cause the reference to become empty. As such, this
+        * field should typically only be set when one uses a `prefix` ending
+        * in a slash.
+        */
+       size_t trim_prefix;
+
+       /* Flags that change which refs will be included. */
+       enum refs_for_each_flag flags;
+};
+
 int refs_for_each_ref(struct ref_store *refs,
                      refs_for_each_cb fn, void *cb_data);
+int refs_for_each_ref_ext(struct ref_store *refs,
+                         refs_for_each_cb cb, void *cb_data,
+                         const struct refs_for_each_ref_options *opts);
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
                         refs_for_each_cb fn, void *cb_data);
 int refs_for_each_tag_ref(struct ref_store *refs,