Hi Junio,
On 6 Jun 2024, at 14:23, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>>
>>> 29 files changed, 64 insertions(+), 52 deletions(-)
>>
>> Wow, the blast radius of this thing is rather big. Among these
>> existing callers of refs_resolve_ref_unsafe(), how many of them
>> will benefit from being able to pass a non NULL parameter at the end
>> of the series, and more importantly, in the future to take advantage
>> of the new feature possibly with a separate series?
>> ...
>> That way, you do not have to touch those existing callers that will
>> never benefit from the new capability in the future. You won't risk
>> conflicting with in flight topics semantically, either.
>
> The same comment applies to [3/4], but I do not want to fix the Cc: header
> again, so I am replying to this message.
Yes, so for 3/4 I was exploring doing the same thing. However, each_repo_fn goes
pretty deep in the callstack and to provide an alternate set of functions that
use something like each_repo_referent_fn would still lead to a relatively large
blast radius, eg, something like:
diff --git a/ref-filter.c b/ref-filter.c
index f7fb0c7e0e..770d3715c2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2631,12 +2631,12 @@ static int filter_exclude_match(struct ref_filter *filter, const char *refname)
* pattern match, so the callback still has to match each ref individually.
*/
static int for_each_fullref_in_pattern(struct ref_filter *filter,
- each_ref_fn cb,
+ each_ref_with_referent_fn cb,
void *cb_data)
{
if (filter->kind & FILTER_REFS_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),
+ return refs_for_each_include_root_refs_with_referent(get_main_ref_store(the_repository),
cb, cb_data);
}
@@ -2646,7 +2646,7 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
* prefixes like "refs/heads/" etc. are stripped off,
* so we have to look at everything:
*/
- return refs_for_each_fullref_in(get_main_ref_store(the_repository),
+ return refs_for_each_fullref_in_with_referent(get_main_ref_store(the_repository),
"", NULL, cb, cb_data);
}
@@ -2656,17 +2656,17 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
* so just return everything and let the caller
* sort it out.
*/
- return refs_for_each_fullref_in(get_main_ref_store(the_repository),
+ return refs_for_each_fullref_in_with_referent(get_main_ref_store(the_repository),
"", NULL, cb, cb_data);
}
if (!filter->name_patterns[0]) {
/* no patterns; we have to look at everything */
- return refs_for_each_fullref_in(get_main_ref_store(the_repository),
+ return refs_for_each_fullref_in_with_referent(get_main_ref_store(the_repository),
"", filter->exclude.v, cb, cb_data);
}
- return refs_for_each_fullref_in_prefixes(get_main_ref_store(the_repository),
+ return refs_for_each_fullref_in_prefixes_with_referent(get_main_ref_store(the_repository),
NULL, filter->name_patterns,
filter->exclude.v,
cb, cb_data);
@@ -2781,7 +2781,7 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname)
return ref_kind_from_refname(refname);
}
-static struct ref_array_item *apply_ref_filter(const char *refname, const struct object_id *oid,
+static struct ref_array_item *apply_ref_filter(const char *refname, const char *referent, const struct object_id *oid,
int flag, struct ref_filter *filter)
{
struct ref_array_item *ref;
@@ -2850,6 +2850,7 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const struct
ref->commit = commit;
ref->flag = flag;
ref->kind = kind;
+ ref->symref = referent;
return ref;
}
@@ -2863,12 +2864,12 @@ struct ref_filter_cbdata {
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int filter_one(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int filter_one(const char *refname, const char *referent, const struct object_id *oid, int flag, void *cb_data)
{
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_array_item *ref;
- ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter);
+ ref = apply_ref_filter(refname, referent, oid, flag, ref_cbdata->filter);
if (ref)
ref_array_append(ref_cbdata->array, ref);
@@ -2898,13 +2899,13 @@ struct ref_filter_and_format_cbdata {
} internal;
};
-static int filter_and_format_one(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int filter_and_format_one(const char *refname, const char *referent, const struct object_id *oid, int flag, void *cb_data)
{
struct ref_filter_and_format_cbdata *ref_cbdata = cb_data;
struct ref_array_item *ref;
struct strbuf output = STRBUF_INIT, err = STRBUF_INIT;
- ref = apply_ref_filter(refname, oid, flag, ref_cbdata->filter);
+ ref = apply_ref_filter(refname, referent, oid, flag, ref_cbdata->filter);
if (!ref)
return 0;
@@ -3050,7 +3051,7 @@ void filter_ahead_behind(struct repository *r,
free(commits);
}
-static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref_fn fn, void *cb_data)
+static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref_with_referent_fn fn, void *cb_data)
{
int ret = 0;
@@ -3070,15 +3071,15 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
* of filter_ref_kind().
*/
if (filter->kind == FILTER_REFS_BRANCHES)
- ret = refs_for_each_fullref_in(get_main_ref_store(the_repository),
+ ret = refs_for_each_fullref_in_with_referent(get_main_ref_store(the_repository),
"refs/heads/", NULL,
fn, cb_data);
else if (filter->kind == FILTER_REFS_REMOTES)
- ret = refs_for_each_fullref_in(get_main_ref_store(the_repository),
+ ret = refs_for_each_fullref_in_with_referent(get_main_ref_store(the_repository),
"refs/remotes/", NULL,
fn, cb_data);
else if (filter->kind == FILTER_REFS_TAGS)
- ret = refs_for_each_fullref_in(get_main_ref_store(the_repository),
+ ret = refs_for_each_fullref_in_with_referent(get_main_ref_store(the_repository),
"refs/tags/", NULL, fn,
cb_data);
else if (filter->kind & FILTER_REFS_REGULAR)
@@ -3090,7 +3091,7 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
*/
if (!ret && !(filter->kind & FILTER_REFS_ROOT_REFS) &&
(filter->kind & FILTER_REFS_DETACHED_HEAD))
- refs_head_ref(get_main_ref_store(the_repository), fn,
+ refs_head_ref_referent(get_main_ref_store(the_repository), fn,
cb_data);
}
diff --git a/refs.c b/refs.c
index 6dcb0288cb..4366f35586 100644
--- a/refs.c
+++ b/refs.c
@@ -1529,6 +1529,19 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
return 0;
}
+int refs_head_ref_referent(struct ref_store *refs, each_ref_with_referent_fn fn, void *cb_data)
+{
+ struct object_id oid;
+ int flag;
+ const char *referent;
+
+ if (refs_resolve_ref_unsafe_with_referent(refs, "HEAD", referent, RESOLVE_REF_READING,
+ &oid, &flag))
+ return fn("HEAD", referent, &oid, flag, cb_data);
+
+ return 0;
+}
+
struct ref_iterator *refs_ref_iterator_begin(
struct ref_store *refs,
const char *prefix,
@@ -1560,6 +1573,22 @@ struct ref_iterator *refs_ref_iterator_begin(
return iter;
}
+static int do_for_each_ref_with_referent(struct ref_store *refs, const char *prefix,
+ const char **exclude_patterns,
+ each_ref_with_referent_fn fn, int trim,
+ enum do_for_each_ref_flags flags, void *cb_data)
+{
+ struct ref_iterator *iter;
+
+ if (!refs)
+ return 0;
+
+ iter = refs_ref_iterator_begin(refs, prefix, exclude_patterns, trim,
+ flags);
+
+ return do_for_each_ref_iterator_with_referent(iter, fn, cb_data);
+}
+
static int do_for_each_ref(struct ref_store *refs, const char *prefix,
const char **exclude_patterns,
each_ref_fn fn, int trim,
@@ -1594,6 +1623,13 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
return do_for_each_ref(refs, prefix, exclude_patterns, fn, 0, 0, cb_data);
}
+int refs_for_each_fullref_in_with_referent(struct ref_store *refs, const char *prefix,
+ const char **exclude_patterns,
+ each_ref_with_referent_fn fn, void *cb_data)
+{
+ return do_for_each_ref_with_referent(refs, prefix, exclude_patterns, fn, 0, 0, cb_data);
+}
+
int refs_for_each_replace_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
{
const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
@@ -1627,6 +1663,13 @@ int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn,
DO_FOR_EACH_INCLUDE_ROOT_REFS, cb_data);
}
+int refs_for_each_include_root_refs_with_referent(struct ref_store *refs, each_ref_with_referent_fn fn,
+ void *cb_data)
+{
+ return do_for_each_ref_with_referent(refs, "", NULL, fn, 0,
+ DO_FOR_EACH_INCLUDE_ROOT_REFS, cb_data);
+}
+
static int qsort_strcmp(const void *va, const void *vb)
{
const char *a = *(const char **)va;
@@ -1716,6 +1759,37 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store,
return ret;
}
+int refs_for_each_fullref_in_prefixes_with_referent(struct ref_store *ref_store,
+ const char *namespace,
+ const char **patterns,
+ const char **exclude_patterns,
+ each_ref_with_referent_fn fn, void *cb_data)
+{
+ struct string_list prefixes = STRING_LIST_INIT_DUP;
+ struct string_list_item *prefix;
+ struct strbuf buf = STRBUF_INIT;
+ int ret = 0, namespace_len;
+
+ find_longest_prefixes(&prefixes, patterns);
+
+ if (namespace)
+ strbuf_addstr(&buf, namespace);
+ namespace_len = buf.len;
+
+ for_each_string_list_item(prefix, &prefixes) {
+ strbuf_addstr(&buf, prefix->string);
+ ret = refs_for_each_fullref_in_with_referent(ref_store, buf.buf,
+ exclude_patterns, fn, cb_data);
+ if (ret)
+ break;
+ strbuf_setlen(&buf, namespace_len);
+ }
+
+ string_list_clear(&prefixes, 0);
+ strbuf_release(&buf);
+ return ret;
+}
+
static int refs_read_special_head(struct ref_store *ref_store,
const char *refname, struct object_id *oid,
struct strbuf *referent, unsigned int *type,
diff --git a/refs.h b/refs.h
index ba6d0e0753..d8387c6296 100644
--- a/refs.h
+++ b/refs.h
@@ -304,6 +304,9 @@ struct ref_transaction;
typedef int each_ref_fn(const char *refname,
const struct object_id *oid, int flags, void *cb_data);
+typedef int each_ref_with_referent_fn(const char *refname, const char *referent,
+ const struct object_id *oid, int flags, void *cb_data);
+
/*
* The following functions invoke the specified callback function for
* each reference indicated. If the function ever returns a nonzero
@@ -315,6 +318,8 @@ typedef int each_ref_fn(const char *refname,
*/
int refs_head_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
+int refs_head_ref_referent(struct ref_store *refs,
+ each_ref_with_referent_fn fn, void *cb_data);
int refs_for_each_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -351,6 +356,17 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *refs,
const char **exclude_patterns,
each_ref_fn fn, void *cb_data);
+int refs_for_each_fullref_in_prefixes_with_referent(struct ref_store *refs,
+ const char *namespace,
+ const char **patterns,
+ const char **exclude_patterns,
+ each_ref_with_referent_fn fn, void *cb_data);
+
+int refs_for_each_fullref_in_with_referent(struct ref_store *refs, const char *prefix,
+ const char **exclude_patterns,
+ each_ref_with_referent_fn fn, void *cb_data);
+
+
/* iterates all refs that match the specified glob pattern. */
int refs_for_each_glob_ref(struct ref_store *refs, each_ref_fn fn,
const char *pattern, void *cb_data);
@@ -377,6 +393,10 @@ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn,
void *cb_data);
+int refs_for_each_include_root_refs_with_referent(struct ref_store *refs, each_ref_with_referent_fn fn,
+ void *cb_data);
+
+
/*
* Normalizes partial refs to their fully qualified form.
* Will prepend <prefix> to the <pattern> if it doesn't start with 'refs/'.
diff --git a/refs/iterator.c b/refs/iterator.c
index 26acb6bd64..26c38ec6de 100644
--- a/refs/iterator.c
+++ b/refs/iterator.c
@@ -469,3 +469,30 @@ int do_for_each_ref_iterator(struct ref_iterator *iter,
return -1;
return retval;
}
+
+int do_for_each_ref_iterator_with_referent(struct ref_iterator *iter,
+ each_ref_with_referent_fn fn, void *cb_data)
+{
+ int retval = 0, ok;
+ struct ref_iterator *old_ref_iter = current_ref_iter;
+
+ current_ref_iter = iter;
+ while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
+ retval = fn(iter->refname, iter->referent, iter->oid, iter->flags, cb_data);
+ if (retval) {
+ /*
+ * If ref_iterator_abort() returns ITER_ERROR,
+ * we ignore that error in deference to the
+ * callback function's return value.
+ */
+ ref_iterator_abort(iter);
+ goto out;
+ }
+ }
+
+out:
+ current_ref_iter = old_ref_iter;
+ if (ok == ITER_ERROR)
+ return -1;
+ return retval;
+}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 0775451435..ebec407468 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -486,6 +486,9 @@ extern struct ref_iterator *current_ref_iter;
*/
int do_for_each_ref_iterator(struct ref_iterator *iter,
each_ref_fn fn, void *cb_data);
+int do_for_each_ref_iterator_with_referent(struct ref_iterator *iter,
+ each_ref_with_referent_fn fn, void *cb_data);
+
struct ref_store;
Which is a little bit unseemly in my view...so between the two options I would be
inclined to go with modifying each_repo_fn than create a whole subset set of
helpers. wdyt?
thanks
John
>
> Thanks.
