[1/4] refs: add referent parameter to refs_resolve_ref_unsafe

Commit Message

John Cai June 6, 2024, 5:26 p.m. UTC

From: John Cai <[e-mail]>

refs_resolve_ref_unsafe retrieves the referent, the unresolved value of
a reference. Add a parameter to allow refs_resolve_ref_unsafe to pass up
the value of referent to the caller so it can save this value in ref
iterators for more efficient access.

Signed-off-by: John Cai <[e-mail]>
---
 add-interactive.c           |  6 +++---
 blame.c                     |  4 ++--
 builtin/bisect.c            |  2 +-
 builtin/blame.c             |  2 +-
 builtin/branch.c            |  2 +-
 builtin/fsck.c              |  2 +-
 builtin/log.c               |  1 +
 builtin/rebase.c            |  6 +++---
 builtin/receive-pack.c      |  2 +-
 builtin/remote.c            |  2 +-
 builtin/stash.c             |  2 +-
 builtin/submodule--helper.c |  4 ++--
 builtin/symbolic-ref.c      |  2 +-
 config.c                    |  2 +-
 http-backend.c              |  1 +
 log-tree.c                  |  2 +-
 ls-refs.c                   |  3 ++-
 refs.c                      | 20 ++++++++++++--------
 refs.h                      |  1 +
 refs/files-backend.c        | 16 ++++++++--------
 refs/reftable-backend.c     |  6 +++---
 remote.c                    |  6 +++---
 revision.c                  |  2 +-
 sequencer.c                 |  9 +++++----
 t/helper/test-ref-store.c   |  2 +-
 transport-helper.c          |  1 +
 transport.c                 |  2 +-
 upload-pack.c               |  2 +-
 worktree.c                  |  4 +++-
 29 files changed, 64 insertions(+), 52 deletions(-)

Comments

Junio C Hamano June 6, 2024, 6:21 p.m. UTC | #1

ADMINISTRIVIA.  Check the address you place on the CC: line.  What
we can see for this message at

https://lore.kernel.org/git/011c[e-mail]/raw

looks like this.

    Cc: "Phillip Wood [ ]" <[e-mail]>,
        Kristoffer Haugsbakk <[[e-mail]]>,
        "Jeff King [ ]" <[e-mail]>,
        "Patrick Steinhardt [ ]" <[e-mail]>,
        "=?UTF-8?Q?Jean-No=C3=ABl?= Avila [ ]" <[e-mail]>,
        John Cai <[e-mail]>,
        John Cai <[e-mail]>

I fixed them manually, but it wasn't pleasant.  I think we saw a
similar breakage earlier coming via GGG, but I do not recall the
details of how to cause such breakages (iow, what to avoid repeating
this).

Anyway.

"John Cai via GitGitGadget" <[e-mail]> 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?

I am assuming that this will benefit only a selected few and the
callers that would want to take advantage of the new feature will
remain low.  Have you considered renaming refs_resolve_ref_unsafe()
to a new name (say, refs_resolve_ref_unsafe_with_referent()) and
implement the new feature (which is only triggered when the new
parameter gets a non NULL value), make refs_resolve_ref_unsafe() a
very thin wrapper that passes NULL to the new thing?

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.

Or will they also benefit from the new feature in the future?

Offhand, I do not know how a caller like this ...

> diff --git a/add-interactive.c b/add-interactive.c
> index b5d6cd689a1..041d30cf2b3 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -533,7 +533,7 @@ static int get_modified_files(struct repository *r,
>  {
>  	struct object_id head_oid;
>  	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> -						  "HEAD", RESOLVE_REF_READING,
> +						  "HEAD", NULL, RESOLVE_REF_READING,
>  						  &head_oid, NULL);
>  	struct collection_status s = { 0 };
>  	int i;

... would be helped.

Thanks.

Junio C Hamano June 6, 2024, 6:23 p.m. UTC | #2

Junio C Hamano <[e-mail]> writes:

> "John Cai via GitGitGadget" <[e-mail]> 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.

Thanks.

John Cai June 6, 2024, 9:02 p.m. UTC | #3

Hi Junio,

On 6 Jun 2024, at 14:21, Junio C Hamano wrote:

> ADMINISTRIVIA.  Check the address you place on the CC: line.  What
> we can see for this message at
>
> https://lore.kernel.org/git/011c[e-mail]/raw
>
> looks like this.
>
>     Cc: "Phillip Wood [ ]" <[e-mail]>,
>         Kristoffer Haugsbakk <[[e-mail]]>,
>         "Jeff King [ ]" <[e-mail]>,
>         "Patrick Steinhardt [ ]" <[e-mail]>,
>         "=?UTF-8?Q?Jean-No=C3=ABl?= Avila [ ]" <[e-mail]>,
>         John Cai <[e-mail]>,
>         John Cai <[e-mail]>
>
> I fixed them manually, but it wasn't pleasant.  I think we saw a
> similar breakage earlier coming via GGG, but I do not recall the
> details of how to cause such breakages (iow, what to avoid repeating
> this).

oof, apologies. Didn't notice that. I'll be more mindful about the cc line.

>
> Anyway.
>
> "John Cai via GitGitGadget" <[e-mail]> 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?
>
> I am assuming that this will benefit only a selected few and the
> callers that would want to take advantage of the new feature will
> remain low.  Have you considered renaming refs_resolve_ref_unsafe()
> to a new name (say, refs_resolve_ref_unsafe_with_referent()) and
> implement the new feature (which is only triggered when the new
> parameter gets a non NULL value), make refs_resolve_ref_unsafe() a
> very thin wrapper that passes NULL to the new thing?
>
> 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.
>
> Or will they also benefit from the new feature in the future?
>
> Offhand, I do not know how a caller like this ...
>
>> diff --git a/add-interactive.c b/add-interactive.c
>> index b5d6cd689a1..041d30cf2b3 100644
>> --- a/add-interactive.c
>> +++ b/add-interactive.c
>> @@ -533,7 +533,7 @@ static int get_modified_files(struct repository *r,
>>  {
>>  	struct object_id head_oid;
>>  	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
>> -						  "HEAD", RESOLVE_REF_READING,
>> +						  "HEAD", NULL, RESOLVE_REF_READING,
>>  						  &head_oid, NULL);
>>  	struct collection_status s = { 0 };
>>  	int i;
>
> ... would be helped.

Good point. I was sensing the same thing as I made all the callsite changes so
this feedback makes it clear what we should do.

>
> Thanks.

Junio C Hamano June 6, 2024, 10:44 p.m. UTC | #4

John Cai <[e-mail]> writes:

> On 6 Jun 2024, at 14:21, Junio C Hamano wrote:
>
>> ADMINISTRIVIA.  Check the address you place on the CC: line.  What
>> we can see for this message at
>> ...
>> I fixed them manually, but it wasn't pleasant.  I think we saw a
>> similar breakage earlier coming via GGG, but I do not recall the
>> details of how to cause such breakages (iow, what to avoid repeating
>> this).
>
> oof, apologies. Didn't notice that. I'll be more mindful about the cc line.

I found the previous occurrences of the same problem:

  https://lore.kernel.org/git/xmqqjzm3qumx.fsf@gitster.g/
  https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/

The last message in the thread

  https://lore.kernel.org/git/CAMbn=[e-mail]/

says that the original user of GGG found what was wrong in the way
the user was using GGG to send and fixed it, but unfortunately we
didn't hear exactly *what* the breakage was and *how* it was fixed.

Aryan, do you remember what the problem was and more importantly
what the fix was?

Thanks.

John Cai June 7, 2024, 1:43 p.m. UTC | #5

Hi Junio,

On 6 Jun 2024, at 14:23, Junio C Hamano wrote:

> Junio C Hamano <[e-mail]> writes:
>
>> "John Cai via GitGitGadget" <[e-mail]> 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.

Junio C Hamano June 7, 2024, 3:21 p.m. UTC | #6

John Cai <[e-mail]> writes:

> 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:

Hmph.  You only care about teaching referent to calls to
refs_for_each_ref_in() and friends in apply_ref_filter()
and nowhere else.  So for example, to preserve the current
calling pattern for this pair of caller/callback (and you have
dozens more exactly like this one you touch in [3/4]):

	static int register_ref(const char *refname,
                                const struct object_id *oid,
				int flags,
				void *cb_data);

	static int read_bisect_refs(void)
	{
		return refs_for_each_ref_in(get_main_ref_store(the_repository),
					    "refs/bisect", register_ref, NULL);
	}

you could arrange your _new_ API the way you want, e.g.,

	typedef int each_ref_with_referent_fn(const char *refname,
	                                      const char *referent,
                                              const struct object_id *oid,
					      int flags,
					      void *cb_data);

	int refs_for_each_ref_with_referent_in(struct ref_store *refs,
					       const char *prefix,
                                               each_ref_with_referent_fn fn,
					       void *cb_data);

to help the single caller, i.e., apply_ref_filter(), and rebuild the
existing API on top as a thin wrapper, e.g.

	/* This cannot change without butchering existing callers */
	typedef int each_ref_fn(const char *refname,
                                const struct object_id *oid,
				int flags,
				void *cb_data);

	/* Hence we introduce an adapter */
	struct each_ref_fn_adapter_cbdata {
		each_ref_fn user_each_ref_fn;
		void *user_cb_data;
	};

	/* This is designed to work as an each_ref_with_referent_fn */
	static int each_ref_adapter_fn(const char *refname,
				       const char *referent,
                                       const struct object_id *oid,
				       int flags,
				       void *cb_data)
	{
		struct each_ref_fn_adapter_cbdata *adapter_cbdata = cbdata;

		/* the callers have no need for referent */
                return adapter_cbdata->user_each_ref_fn(refname, oid, flags,
					                adapter_cbdata->user_cbdata);
	}

	/*
         * The function signature must stay the same to help existing,
         * callers, but the implementation is now a thin wrapper.
	 */
	int refs_for_each_ref_in(struct ref_store *refs,
				 const char *prefix,
                                 each_ref_fn fn,
				 void *cb_data)
        {
		struct each_ref_fn_adapter_cbdata adapter_cbdata = {
                	.user_each_ref_fn = fn,
			.user_cb_data = cb_data,
		};
		return refs_for_each_ref_with_referetnt_in(refs, prefix,
							   each_ref_adapter_fn,
                                                           &adapter_cbdata);
	}

no?

You'd need to pass through the new parameter "referent" through the
code paths that implement refs_for_each_ref_in() and friends to
update them to refs_for_each_ref_with_referent_in() and friends no
matter what, but there are limited number of the top-level
refs_for_each_ref_in() and friends that are unaware of the
"referent", and each of them would need the above ~20 lines (couting
the comment) adapter function that all can share the single
each_ref_adapter_fn() callback function.

Or am I missing some other intricacy in the existing for-each-* API?

Patrick Steinhardt June 10, 2024, 7:29 a.m. UTC | #7

On Thu, Jun 06, 2024 at 11:23:18AM -0700, Junio C Hamano wrote:
> Junio C Hamano <[e-mail]> writes:
> 
> > "John Cai via GitGitGadget" <[e-mail]> 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.

I wonder whether we can future-proof the code a bit more by introducing
a struct that contains everything we want to pass to the callback
function. That wouldn't fix the blast radius of this patch, but would
mean that we can easily add additional fielids to that struct in the
future without having to adapt any callers at all anymore.

Something like:

    struct ref_data {
        const char *refname;
        const struct object_id *oid;
        const char *referent;
        int flags;
    };

This would also allow us to get rid of this awful `peel_iterated_oid()`
function that relies on global state in many places, as we can put the
peeled OID into that structure, as well.

Patrick

Junio C Hamano June 10, 2024, 6:09 p.m. UTC | #8

Patrick Steinhardt <[e-mail]> writes:

> I wonder whether we can future-proof the code a bit more by introducing
> a struct that contains everything we want to pass to the callback
> function.

That would hopefully make a change with a large blast a one-time
event.  But at the same time, it may end up making it too opaque and
hard to verify if all the API functions are using/updating/verifying
all the members of the struct as they should.  Compared to that,
unused parameters are easier to verify mechanically by compilers.

> This would also allow us to get rid of this awful `peel_iterated_oid()`
> function that relies on global state in many places, as we can put the
> peeled OID into that structure, as well.

Yes, such a benefit may justify a one-time "affect many callers"
event.  Or the underlying for_each_*() friend of functions can be
updated to use a single struct and then the current "only selected
parameters that are used ar passed" API can be made into a set of
thin wrappers around it, and then callers can be converted one step
at a time, in a multi-step series, perhaps.

Jeff King June 11, 2024, 8:50 a.m. UTC | #9

On Thu, Jun 06, 2024 at 05:26:37PM +0000, John Cai via GitGitGadget wrote:

> From: John Cai <[e-mail]>
> 
> refs_resolve_ref_unsafe retrieves the referent, the unresolved value of
> a reference. Add a parameter to allow refs_resolve_ref_unsafe to pass up
> the value of referent to the caller so it can save this value in ref
> iterators for more efficient access.

This commit message left me with a lot of questions.

For one, it wasn't immediately obvious to me what a "referent" is. ;) I
think an example could help. If I understand, you mean that if you have
a situation like:

  - refs/heads/one is a symref pointing to refs/heads/two
  - refs/heads/two is a regular ref

and we resolve "one", then "two" is the referent? And the caller might
want to know that?

But I think we already pass that out as the return value from
refs_resolve_ref_unsafe(). That is how something like "rev-parse
--symbolic-full-name" works now.

But there are some subtleties. In a chain of symbolic refs (say, "two"
is a symbolic ref to "three"), we return only the final name ("three").
And you might want to know about "two".

You can pass RESOLVE_REF_NO_RECURSE to inhibit this, and get back just
"two". You can see that now with "git symbolic-ref --no-recurse". The
downside is that we never look at the referent at all, so you get only
the symref value (and no information about the actual oid, or if the
referent even exists). You would still get an oid for any non-symrefs
you examine.

So reading between the lines, you have a caller in mind which wants to
know the immediate referent in addition to the final recursive oid?

Looking at the rest of your series, I guess that caller is the one in
loose_fill_ref_dir_regular_file(), so that it can get passed to the
for-each-ref callback. But why is it right thing for it to record and
pass along the immediate referent there, and not the final one? For that
matter, would a caller ever want to see the whole chain of
one/two/three?

> @@ -1761,6 +1761,7 @@ int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,
>  
>  const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>  				    const char *refname,
> +				    const char *referent,
>  				    int resolve_flags,
>  				    struct object_id *oid,
>  				    int *flags)

Unless I am misunderstanding the purpose of your patch completely, this
"referent" is meant to be an out-parameter, right? In which case,
shouldn't it be "const char **referent"?

As the code is now:

> @@ -1822,6 +1823,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>  		}
>  
>  		*flags |= read_flags;
> +		if (referent && (read_flags & REF_ISSYMREF) &&
> +		    sb_refname.len > 0)
> +			referent = sb_refname.buf;
>  
>  		if (!(read_flags & REF_ISSYMREF)) {
>  			if (*flags & REF_BAD_NAME) {

...we'd assign the local "referent" pointer to our refname buf, but
the caller would never see that. Plus doing so would not help you
anyway, since sb_refname will be used again as we recurse. So at best,
you end up with the final name in the chain anyway. Or at worst,
sb_refname gets reallocated and "referent" is left as a dangling
pointer.

-Peff

Kristoffer Haugsbakk June 28, 2024, 3:30 p.m. UTC | #10

On Fri, Jun 7, 2024, at 00:44, Junio C Hamano wrote:
> I found the previous occurrences of the same problem:
>
>   https://lore.kernel.org/git/xmqqjzm3qumx.fsf@gitster.g/
>   https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/
>
> The last message in the thread
>
>
> https://lore.kernel.org/git/CAMbn=[e-mail]/
>
> says that the original user of GGG found what was wrong in the way
> the user was using GGG to send and fixed it, but unfortunately we
> didn't hear exactly *what* the breakage was and *how* it was fixed.
>
> Aryan, do you remember what the problem was and more importantly
> what the fix was?
>
> Thanks.

Yes, Aryan didn’t explain what the issue was. But Linus Arver (+CC) in
that first thread/link figured out what was happening in his case:[1]

> I realize now that it's because I copy/pasted the "Cc: ..." lines in the PR
> description from
> https://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such
> that when I pasted those in for the PR description for this series at
> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it
> carried over the email addresses as Markdown-formatted hyperlinks.
> Currently it reads
>
>    Cc: Christian Couder [[e-mail]](mailto:[e-mail])
>    Cc: Junio C Hamano [[e-mail]](mailto:[e-mail])
>    Cc: Emily Shaffer [[e-mail]](mailto:[e-mail])
>    cc: Josh Steadmon [[e-mail]](mailto:[e-mail])
>    cc: Randall S. Becker [[e-mail]](mailto:[e-mail])
>    cc: Christian Couder [[e-mail]](mailto:[e-mail])
>    cc: "Kristoffer Haugsbakk" [[e-mail]](mailto:[e-mail])
>    cc: "Kristoffer Haugsbakk" <[e-mail]>
>
> when I click on "edit", where the last line must be from your manual
> fix which GGG picked up. I've cleaned up the PR description manually
> now, and for this message I'm also attempting to clean up those square
> brackets.

When I read that I assumed that Aryan had made the same mistake.

Junio C Hamano June 28, 2024, 7:47 p.m. UTC | #11

"Kristoffer Haugsbakk" <[e-mail]> writes:

>> ...
>> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it
>> carried over the email addresses as Markdown-formatted hyperlinks.
>> Currently it reads
>>
>>    Cc: Christian Couder [[e-mail]](mailto:[e-mail])
>>...
>>    cc: "Kristoffer Haugsbakk" <[e-mail]>
>>
>> when I click on "edit", where the last line must be from your manual
>> fix which GGG picked up. I've cleaned up the PR description manually
>> now, and for this message I'm also attempting to clean up those square
>> brackets.
>
> When I read that I assumed that Aryan had made the same mistake.

Ah, I see.

I wonder if there are things we can do to help reduce the pain in
future users of GGG here.

 - Is there some documentation update we can make on our end,
   perhaps in Documentation/MyFirstContribution (SubmittingPatches
   does not talk about GGG other than refering readers to
   MyFirstContribution)?

 - Or the welcome message GGG adds to the pull-request that begins
   with "Thanks for taking the time to contribute" can mention
   something about this?

 - Or possibly the handling of Cc: addresses done by GGG can be
   tweaked not to cause this?

Anything else?

Thanks.

Linus Arver June 30, 2024, 10:12 a.m. UTC | #12

Junio C Hamano <[e-mail]> writes:

> "Kristoffer Haugsbakk" <[e-mail]> writes:
>
>>> ...
>>> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it
>>> carried over the email addresses as Markdown-formatted hyperlinks.
>>> Currently it reads
>>>
>>>    Cc: Christian Couder [[e-mail]](mailto:[e-mail])
>>>...
>>>    cc: "Kristoffer Haugsbakk" <[e-mail]>
>>>
>>> when I click on "edit", where the last line must be from your manual
>>> fix which GGG picked up. I've cleaned up the PR description manually
>>> now, and for this message I'm also attempting to clean up those square
>>> brackets.
>>
>> When I read that I assumed that Aryan had made the same mistake.
>
> Ah, I see.
>
> I wonder if there are things we can do to help reduce the pain in
> future users of GGG here.
>
>  [...]
>
>  - Or the welcome message GGG adds to the pull-request that begins
>    with "Thanks for taking the time to contribute" can mention
>    something about this?

I've created https://github.com/gitgitgadget/gitgitgadget/pull/1644 to
address this for now.
>
>  - Or possibly the handling of Cc: addresses done by GGG can be
>    tweaked not to cause this?

I wanted to do this first but was deterred by my lack of familiarity
with TypeScript.

Junio C Hamano June 30, 2024, 6:19 p.m. UTC | #13

Linus Arver <[e-mail]> writes:

>>  - Or the welcome message GGG adds to the pull-request that begins
>>    with "Thanks for taking the time to contribute" can mention
>>    something about this?
>
> I've created https://github.com/gitgitgadget/gitgitgadget/pull/1644 to
> address this for now.

Thanks.

John Cai July 30, 2024, 2:38 p.m. UTC | #14

Hey Peff,

On 11 Jun 2024, at 4:50, Jeff King wrote:

> On Thu, Jun 06, 2024 at 05:26:37PM +0000, John Cai via GitGitGadget wrote:
>
>> From: John Cai <[e-mail]>
>>
>> refs_resolve_ref_unsafe retrieves the referent, the unresolved value of
>> a reference. Add a parameter to allow refs_resolve_ref_unsafe to pass up
>> the value of referent to the caller so it can save this value in ref
>> iterators for more efficient access.
>
> This commit message left me with a lot of questions.
>
> For one, it wasn't immediately obvious to me what a "referent" is. ;) I
> think an example could help. If I understand, you mean that if you have
> a situation like:
>
>   - refs/heads/one is a symref pointing to refs/heads/two
>   - refs/heads/two is a regular ref
>
> and we resolve "one", then "two" is the referent? And the caller might
> want to know that?
>
> But I think we already pass that out as the return value from
> refs_resolve_ref_unsafe(). That is how something like "rev-parse
> --symbolic-full-name" works now.

Yes, exactly. I think you're right that it'd be preferable to just use the
output of refs_resolve_ref_unsafe() to get the value of the referent.
>
> But there are some subtleties. In a chain of symbolic refs (say, "two"
> is a symbolic ref to "three"), we return only the final name ("three").
> And you might want to know about "two".
>
> You can pass RESOLVE_REF_NO_RECURSE to inhibit this, and get back just
> "two". You can see that now with "git symbolic-ref --no-recurse". The
> downside is that we never look at the referent at all, so you get only
> the symref value (and no information about the actual oid, or if the
> referent even exists). You would still get an oid for any non-symrefs
> you examine.
>
> So reading between the lines, you have a caller in mind which wants to
> know the immediate referent in addition to the final recursive oid?

The goal is to keep track of the value that %(symref) would need in the
iterator so that a separate call doesn't need to be made.

>
> Looking at the rest of your series, I guess that caller is the one in
> loose_fill_ref_dir_regular_file(), so that it can get passed to the
> for-each-ref callback. But why is it right thing for it to record and
> pass along the immediate referent there, and not the final one? For that
> matter, would a caller ever want to see the whole chain of
> one/two/three?

Right, the final referent is the right one to pass down.

>
>> @@ -1761,6 +1761,7 @@ int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,
>>
>>  const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>>  				    const char *refname,
>> +				    const char *referent,
>>  				    int resolve_flags,
>>  				    struct object_id *oid,
>>  				    int *flags)
>
> Unless I am misunderstanding the purpose of your patch completely, this
> "referent" is meant to be an out-parameter, right? In which case,
> shouldn't it be "const char **referent"?
>
> As the code is now:
>
>> @@ -1822,6 +1823,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>>  		}
>>
>>  		*flags |= read_flags;
>> +		if (referent && (read_flags & REF_ISSYMREF) &&
>> +		    sb_refname.len > 0)
>> +			referent = sb_refname.buf;
>>
>>  		if (!(read_flags & REF_ISSYMREF)) {
>>  			if (*flags & REF_BAD_NAME) {
>
> ...we'd assign the local "referent" pointer to our refname buf, but
> the caller would never see that. Plus doing so would not help you
> anyway, since sb_refname will be used again as we recurse. So at best,
> you end up with the final name in the chain anyway. Or at worst,
> sb_refname gets reallocated and "referent" is left as a dangling
> pointer.

Going to include changes to remove the out-parameter which will simplify things
quite a bit.

>
> -Peff

thanks for the review!
John

Patch

diff --git a/add-interactive.c b/add-interactive.c
index b5d6cd689a1..041d30cf2b3 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -533,7 +533,7 @@  static int get_modified_files(struct repository *r,
 {
 	struct object_id head_oid;
 	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						  "HEAD", RESOLVE_REF_READING,
+						  "HEAD", NULL, RESOLVE_REF_READING,
 						  &head_oid, NULL);
 	struct collection_status s = { 0 };
 	int i;
@@ -763,7 +763,7 @@  static int run_revert(struct add_i_state *s, const struct pathspec *ps,
 
 	struct object_id oid;
 	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						  "HEAD", RESOLVE_REF_READING,
+						  "HEAD", NULL, RESOLVE_REF_READING,
 						  &oid,
 						  NULL);
 	struct lock_file index_lock;
@@ -994,7 +994,7 @@  static int run_diff(struct add_i_state *s, const struct pathspec *ps,
 
 	struct object_id oid;
 	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						  "HEAD", RESOLVE_REF_READING,
+						  "HEAD", NULL, RESOLVE_REF_READING,
 						  &oid,
 						  NULL);
 	if (get_modified_files(s->r, INDEX_ONLY, files, ps, NULL, NULL) < 0)
diff --git a/blame.c b/blame.c
index 33586b97772..9e5d0cd788f 100644
--- a/blame.c
+++ b/blame.c
@@ -2700,7 +2700,7 @@  static struct commit *dwim_reverse_initial(struct rev_info *revs,
 		return NULL;
 
 	/* Do we have HEAD? */
-	if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", RESOLVE_REF_READING, &head_oid, NULL))
+	if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, RESOLVE_REF_READING, &head_oid, NULL))
 		return NULL;
 	head_commit = lookup_commit_reference_gently(revs->repo,
 						     &head_oid, 1);
@@ -2803,7 +2803,7 @@  void setup_scoreboard(struct blame_scoreboard *sb,
 		if (sb->final) {
 			parent_oid = &sb->final->object.oid;
 		} else {
-			if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", RESOLVE_REF_READING, &head_oid, NULL))
+			if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, RESOLVE_REF_READING, &head_oid, NULL))
 				die("no such ref: HEAD");
 			parent_oid = &head_oid;
 		}
diff --git a/builtin/bisect.c b/builtin/bisect.c
index a58432b9d90..76ce5f0e0df 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -783,7 +783,7 @@  static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
 	 * Verify HEAD
 	 */
 	head = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-				       "HEAD", 0, &head_oid, &flags);
+				       "HEAD", NULL, 0, &head_oid, &flags);
 	if (!head)
 		if (repo_get_oid(the_repository, "HEAD", &head_oid))
 			return error(_("bad HEAD - I need a HEAD"));
diff --git a/builtin/blame.c b/builtin/blame.c
index fadba1a5304..1504a2ed99d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1092,7 +1092,7 @@  int cmd_blame(int argc, const char **argv, const char *prefix)
 		struct commit *head_commit;
 		struct object_id head_oid;
 
-		if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", RESOLVE_REF_READING,
+		if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, RESOLVE_REF_READING,
 					     &head_oid, NULL) ||
 		    !(head_commit = lookup_commit_reference_gently(revs.repo,
 							     &head_oid, 1)))
diff --git a/builtin/branch.c b/builtin/branch.c
index 48cac74f97f..dd871d44f2d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -503,7 +503,7 @@  static void print_current_branch_name(void)
 {
 	int flags;
 	const char *refname = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						      "HEAD", 0, NULL, &flags);
+						      "HEAD", NULL, 0, NULL, &flags);
 	const char *shortname;
 	if (!refname)
 		die(_("could not resolve HEAD"));
diff --git a/builtin/fsck.c b/builtin/fsck.c
index d13a226c2ed..2c0ac6653ca 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -716,7 +716,7 @@  static int fsck_head_link(const char *head_ref_name,
 		fprintf_ln(stderr, _("Checking %s link"), head_ref_name);
 
 	*head_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						  head_ref_name, 0, head_oid,
+						  head_ref_name, NULL, 0, head_oid,
 						  NULL);
 	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
diff --git a/builtin/log.c b/builtin/log.c
index c8ce0c0d88a..337f367e974 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2228,6 +2228,7 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			const char *ref, *v;
 			ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
 						      "HEAD",
+						      NULL,
 						      RESOLVE_REF_READING,
 						      NULL, NULL);
 			if (ref && skip_prefix(ref, "refs/heads/", &v))
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0466d9414af..fd14b1e4505 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1641,7 +1641,7 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 	} else if (argc == 0) {
 		/* Do not need to switch branches, we are already on it. */
 		options.head_name =
-			xstrdup_or_null(refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", 0, NULL,
+			xstrdup_or_null(refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, 0, NULL,
 								&flags));
 		if (!options.head_name)
 			die(_("No such ref: %s"), "HEAD");
@@ -1736,7 +1736,7 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 			if (!(options.flags & REBASE_NO_QUIET))
 				; /* be quiet */
 			else if (!strcmp(branch_name, "HEAD") &&
-				 refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", 0, NULL, &flag))
+				 refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, 0, NULL, &flag))
 				puts(_("HEAD is up to date."));
 			else
 				printf(_("Current branch %s is up to date.\n"),
@@ -1746,7 +1746,7 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 		} else if (!(options.flags & REBASE_NO_QUIET))
 			; /* be quiet */
 		else if (!strcmp(branch_name, "HEAD") &&
-			 refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", 0, NULL, &flag))
+			 refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, 0, NULL, &flag))
 			puts(_("HEAD is up to date, rebase forced."));
 		else
 			printf(_("Current branch %s is up to date, rebase "
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index be8969a84a8..d9e2c4bbe39 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1695,7 +1695,7 @@  static void check_aliased_update(struct command *cmd, struct string_list *list)
 
 	strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name);
 	dst_name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					   buf.buf, 0, NULL, &flag);
+					   buf.buf, NULL, 0, NULL, &flag);
 	check_aliased_update_internal(cmd, list, dst_name, flag);
 	strbuf_release(&buf);
 }
diff --git a/builtin/remote.c b/builtin/remote.c
index 447ef1d3c92..039d1d6c55a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -599,7 +599,7 @@  static int read_remote_branches(const char *refname,
 	if (starts_with(refname, buf.buf)) {
 		item = string_list_append(rename->remote_branches, refname);
 		symref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						 refname, RESOLVE_REF_READING,
+						 refname, NULL, RESOLVE_REF_READING,
 						 NULL, &flag);
 		if (symref && (flag & REF_ISSYMREF)) {
 			item->util = xstrdup(symref);
diff --git a/builtin/stash.c b/builtin/stash.c
index 7859bc0866a..b733492cead 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1386,7 +1386,7 @@  static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 	}
 
 	branch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					     "HEAD", 0, NULL, &flags);
+					     "HEAD", NULL, 0, NULL, &flags);
 	if (flags & REF_ISSYMREF)
 		skip_prefix(branch_ref, "refs/heads/", &branch_name);
 	head_short_sha1 = repo_find_unique_abbrev(the_repository,
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 897f19868e8..e3e1f08a58c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -44,7 +44,7 @@  static int repo_get_default_remote(struct repository *repo, char **default_remot
 	char *dest = NULL;
 	struct strbuf sb = STRBUF_INIT;
 	struct ref_store *store = get_main_ref_store(repo);
-	const char *refname = refs_resolve_ref_unsafe(store, "HEAD", 0, NULL,
+	const char *refname = refs_resolve_ref_unsafe(store, "HEAD", NULL, 0, NULL,
 						      NULL);
 
 	if (!refname)
@@ -2459,7 +2459,7 @@  static int remote_submodule_branch(const char *path, const char **branch)
 
 	if (!strcmp(*branch, ".")) {
 		const char *refname = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-							      "HEAD", 0, NULL,
+							      "HEAD", NULL, 0, NULL,
 							      NULL);
 
 		if (!refname)
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 81abdd170fe..96fa311b075 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -19,7 +19,7 @@  static int check_symref(const char *HEAD, int quiet, int shorten, int recurse, i
 
 	resolve_flags = (recurse ? 0 : RESOLVE_REF_NO_RECURSE);
 	refname = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					  HEAD, resolve_flags, NULL, &flag);
+					  HEAD, NULL, resolve_flags, NULL, &flag);
 
 	if (!refname)
 		die("No such ref: %s", HEAD);
diff --git a/config.c b/config.c
index 14461312b33..3cc1dab0fb9 100644
--- a/config.c
+++ b/config.c
@@ -304,7 +304,7 @@  static int include_by_branch(const char *cond, size_t cond_len)
 	struct strbuf pattern = STRBUF_INIT;
 	const char *refname = !the_repository->gitdir ?
 		NULL : refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					       "HEAD", 0, NULL, &flags);
+					       "HEAD", NULL, 0, NULL, &flags);
 	const char *shortname;
 
 	if (!refname || !(flags & REF_ISSYMREF)	||
diff --git a/http-backend.c b/http-backend.c
index 5b65287ac90..20c3ff8fa95 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -574,6 +574,7 @@  static int show_head_ref(const char *refname, const struct object_id *oid,
 	if (flag & REF_ISSYMREF) {
 		const char *target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
 							     refname,
+							     NULL,
 							     RESOLVE_REF_READING,
 							     NULL, NULL);
 
diff --git a/log-tree.c b/log-tree.c
index 41416de4e3f..da06c6e982f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -280,7 +280,7 @@  static const struct name_decoration *current_pointed_by_HEAD(const struct name_d
 
 	/* Now resolve and find the matching current branch */
 	branch_name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					      "HEAD", 0, NULL, &rru_flags);
+					      "HEAD", NULL, 0, NULL, &rru_flags);
 	if (!branch_name || !(rru_flags & REF_ISSYMREF))
 		return NULL;
 
diff --git a/ls-refs.c b/ls-refs.c
index 398afe4ce39..3d047bbc64f 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -97,6 +97,7 @@  static int send_ref(const char *refname, const struct object_id *oid,
 		struct object_id unused;
 		const char *symref_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
 								    refname,
+								    NULL,
 								    0,
 								    &unused,
 								    &flag);
@@ -128,7 +129,7 @@  static void send_possibly_unborn_head(struct ls_refs_data *data)
 	int oid_is_null;
 
 	strbuf_addf(&namespaced, "%sHEAD", get_git_namespace());
-	if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), namespaced.buf, 0, &oid, &flag))
+	if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), namespaced.buf, NULL, 0, &oid, &flag))
 		return; /* bad ref */
 	oid_is_null = is_null_oid(&oid);
 	if (!oid_is_null ||
diff --git a/refs.c b/refs.c
index 31032588e0e..2c592a9ae29 100644
--- a/refs.c
+++ b/refs.c
@@ -378,7 +378,7 @@  char *refs_resolve_refdup(struct ref_store *refs,
 {
 	const char *result;
 
-	result = refs_resolve_ref_unsafe(refs, refname, resolve_flags,
+	result = refs_resolve_ref_unsafe(refs, refname, NULL, resolve_flags,
 					 oid, flags);
 	return xstrdup_or_null(result);
 }
@@ -394,7 +394,7 @@  struct for_each_ref_filter {
 int refs_read_ref_full(struct ref_store *refs, const char *refname,
 		       int resolve_flags, struct object_id *oid, int *flags)
 {
-	if (refs_resolve_ref_unsafe(refs, refname, resolve_flags,
+	if (refs_resolve_ref_unsafe(refs, refname, NULL, resolve_flags,
 				    oid, flags))
 		return 0;
 	return -1;
@@ -407,7 +407,7 @@  int refs_read_ref(struct ref_store *refs, const char *refname, struct object_id
 
 int refs_ref_exists(struct ref_store *refs, const char *refname)
 {
-	return !!refs_resolve_ref_unsafe(refs, refname, RESOLVE_REF_READING,
+	return !!refs_resolve_ref_unsafe(refs, refname, NULL, RESOLVE_REF_READING,
 					 NULL, NULL);
 }
 
@@ -442,7 +442,7 @@  static int warn_if_dangling_symref(const char *refname,
 	if (!(flags & REF_ISSYMREF))
 		return 0;
 
-	resolves_to = refs_resolve_ref_unsafe(d->refs, refname, 0, NULL, NULL);
+	resolves_to = refs_resolve_ref_unsafe(d->refs, refname, NULL, 0, NULL, NULL);
 	if (!resolves_to
 	    || (d->refname
 		? strcmp(resolves_to, d->refname)
@@ -716,7 +716,7 @@  int expand_ref(struct repository *repo, const char *str, int len,
 		this_result = refs_found ? &oid_from_ref : oid;
 		strbuf_reset(&fullref);
 		strbuf_addf(&fullref, *p, len, str);
-		r = refs_resolve_ref_unsafe(refs, fullref.buf,
+		r = refs_resolve_ref_unsafe(refs, fullref.buf, NULL,
 					    RESOLVE_REF_READING,
 					    this_result, &flag);
 		if (r) {
@@ -750,7 +750,7 @@  int repo_dwim_log(struct repository *r, const char *str, int len,
 
 		strbuf_reset(&path);
 		strbuf_addf(&path, *p, len, str);
-		ref = refs_resolve_ref_unsafe(refs, path.buf,
+		ref = refs_resolve_ref_unsafe(refs, path.buf, NULL,
 					      RESOLVE_REF_READING,
 					      oid ? &hash : NULL, NULL);
 		if (!ref)
@@ -1522,7 +1522,7 @@  int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 	struct object_id oid;
 	int flag;
 
-	if (refs_resolve_ref_unsafe(refs, "HEAD", RESOLVE_REF_READING,
+	if (refs_resolve_ref_unsafe(refs, "HEAD", NULL, RESOLVE_REF_READING,
 				    &oid, &flag))
 		return fn("HEAD", &oid, flag, cb_data);
 
@@ -1761,6 +1761,7 @@  int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,
 
 const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 				    const char *refname,
+				    const char *referent,
 				    int resolve_flags,
 				    struct object_id *oid,
 				    int *flags)
@@ -1822,6 +1823,9 @@  const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 		}
 
 		*flags |= read_flags;
+		if (referent && (read_flags & REF_ISSYMREF) &&
+		    sb_refname.len > 0)
+			referent = sb_refname.buf;
 
 		if (!(read_flags & REF_ISSYMREF)) {
 			if (*flags & REF_BAD_NAME) {
@@ -1865,7 +1869,7 @@  int repo_resolve_gitlink_ref(struct repository *r,
 	if (!refs)
 		return -1;
 
-	if (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||
+	if (!refs_resolve_ref_unsafe(refs, refname, NULL, 0, oid, &flags) ||
 	    is_null_oid(oid))
 		return -1;
 	return 0;
diff --git a/refs.h b/refs.h
index fe7f0db35e6..ea4a3217658 100644
--- a/refs.h
+++ b/refs.h
@@ -68,6 +68,7 @@  const char *ref_storage_format_to_name(unsigned int ref_storage_format);
 
 const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 				    const char *refname,
+				    const char *referent,
 				    int resolve_flags,
 				    struct object_id *oid,
 				    int *flags);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 324c59b096c..bf2ffe062ea 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -244,7 +244,7 @@  static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
 	struct object_id oid;
 	int flag;
 
-	if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_READING,
+	if (!refs_resolve_ref_unsafe(&refs->base, refname, NULL, RESOLVE_REF_READING,
 				     &oid, &flag)) {
 		oidclr(&oid);
 		flag |= REF_ISBROKEN;
@@ -1118,7 +1118,7 @@  static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
 		goto error_return;
 	}
 
-	if (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,
+	if (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, NULL, 0,
 				     &lock->old_oid, NULL))
 		oidclr(&lock->old_oid);
 	goto out;
@@ -1455,7 +1455,7 @@  static int files_copy_or_rename_ref(struct ref_store *ref_store,
 		goto out;
 	}
 
-	if (!refs_resolve_ref_unsafe(&refs->base, oldrefname,
+	if (!refs_resolve_ref_unsafe(&refs->base, oldrefname, NULL,
 				     RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 				     &orig_oid, &flag)) {
 		ret = error("refname %s not found", oldrefname);
@@ -1501,7 +1501,7 @@  static int files_copy_or_rename_ref(struct ref_store *ref_store,
 	 * the safety anyway; we want to delete the reference whatever
 	 * its current value.
 	 */
-	if (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname,
+	if (!copy && refs_resolve_ref_unsafe(&refs->base, newrefname, NULL,
 					     RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 					     NULL, NULL) &&
 	    refs_delete_ref(&refs->base, NULL, newrefname,
@@ -1875,7 +1875,7 @@  static int commit_ref_update(struct files_ref_store *refs,
 		int head_flag;
 		const char *head_ref;
 
-		head_ref = refs_resolve_ref_unsafe(&refs->base, "HEAD",
+		head_ref = refs_resolve_ref_unsafe(&refs->base, "HEAD", NULL,
 						   RESOLVE_REF_READING,
 						   NULL, &head_flag);
 		if (head_ref && (head_flag & REF_ISSYMREF) &&
@@ -2464,7 +2464,7 @@  static int lock_ref_for_update(struct files_ref_store *refs,
 			 * to record and possibly check old_oid:
 			 */
 			if (!refs_resolve_ref_unsafe(&refs->base,
-						     referent.buf, 0,
+						     referent.buf, NULL, 0,
 						     &lock->old_oid, NULL)) {
 				if (update->flags & REF_HAVE_OLD) {
 					strbuf_addf(err, "cannot lock ref '%s': "
@@ -2823,7 +2823,7 @@  static int parse_and_write_reflog(struct files_ref_store *refs,
 		 * We want to get the resolved OID for the target, to ensure
 		 * that the correct value is added to the reflog.
 		 */
-		if (!refs_resolve_ref_unsafe(&refs->base, update->new_target,
+		if (!refs_resolve_ref_unsafe(&refs->base, update->new_target, NULL,
 					     RESOLVE_REF_READING,
 					     &update->new_oid, NULL)) {
 			/*
@@ -3229,7 +3229,7 @@  static int files_reflog_expire(struct ref_store *ref_store,
 			int type;
 			const char *ref;
 
-			ref = refs_resolve_ref_unsafe(&refs->base, refname,
+			ref = refs_resolve_ref_unsafe(&refs->base, refname, NULL,
 						      RESOLVE_REF_NO_RECURSE,
 						      NULL, &type);
 			update = !!(ref && !(type & REF_ISSYMREF));
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 438b5c478b4..9e03582e7da 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -438,7 +438,7 @@  static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
 			oidread(&iter->oid, iter->ref.value.val2.value);
 			break;
 		case REFTABLE_REF_SYMREF:
-			if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->ref.refname,
+			if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->ref.refname, NULL,
 						     RESOLVE_REF_READING, &iter->oid, &flags))
 				oidclr(&iter->oid);
 			break;
@@ -926,7 +926,7 @@  static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 			 * so it is safe to call `refs_resolve_ref_unsafe()`
 			 * here without causing races.
 			 */
-			const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, 0,
+			const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, NULL, 0,
 								       &current_oid, NULL);
 
 			if (u->flags & REF_NO_DEREF) {
@@ -1148,7 +1148,7 @@  static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 			int create_reflog = 1;
 
 			if (u->new_target) {
-				if (!refs_resolve_ref_unsafe(&arg->refs->base, u->new_target,
+				if (!refs_resolve_ref_unsafe(&arg->refs->base, u->new_target, NULL,
 							     RESOLVE_REF_READING, &u->new_oid, NULL)) {
 					/*
 					 * TODO: currently we skip creating reflogs for dangling
diff --git a/remote.c b/remote.c
index 5898da2bb5c..c9718ee0ca1 100644
--- a/remote.c
+++ b/remote.c
@@ -520,7 +520,7 @@  static void read_config(struct repository *repo, int early)
 	repo->remote_state->current_branch = NULL;
 	if (startup_info->have_repository && !early) {
 		const char *head_ref = refs_resolve_ref_unsafe(
-			get_main_ref_store(repo), "HEAD", 0, NULL, &flag);
+			get_main_ref_store(repo), "HEAD", NULL, 0, NULL, &flag);
 		if (head_ref && (flag & REF_ISSYMREF) &&
 		    skip_prefix(head_ref, "refs/heads/", &head_ref)) {
 			repo->remote_state->current_branch = make_branch(
@@ -1201,7 +1201,7 @@  static char *guess_ref(const char *name, struct ref *peer)
 	struct strbuf buf = STRBUF_INIT;
 
 	const char *r = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						peer->name,
+						peer->name, NULL,
 						RESOLVE_REF_READING,
 						NULL, NULL);
 	if (!r)
@@ -1321,7 +1321,7 @@  static int match_explicit(struct ref *src, struct ref *dst,
 		int flag;
 
 		dst_value = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						    matched_src->name,
+						    matched_src->name, NULL,
 						    RESOLVE_REF_READING,
 						    NULL, &flag);
 		if (!dst_value ||
diff --git a/revision.c b/revision.c
index 7ddf0f151a3..6aca4f42303 100644
--- a/revision.c
+++ b/revision.c
@@ -2920,7 +2920,7 @@  static void NORETURN diagnose_missing_default(const char *def)
 	const char *refname;
 
 	refname = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					  def, 0, NULL, &flags);
+					  def, NULL, 0, NULL, &flags);
 	if (!refname || !(flags & REF_ISSYMREF) || (flags & REF_ISBROKEN))
 		die(_("your current branch appears to be broken"));
 
diff --git a/sequencer.c b/sequencer.c
index aa2a2398357..cf7a2a9a112 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -839,10 +839,10 @@  static int is_index_unchanged(struct repository *r)
 	struct index_state *istate = r->index;
 	const char *head_name;
 
-	if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
+	if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), "HEAD", NULL, RESOLVE_REF_READING, &head_oid, NULL)) {
 		/* Check to see if this is an unborn branch */
 		head_name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						    "HEAD",
+						    "HEAD", NULL,
 						    RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 						    &head_oid, NULL);
 		if (!head_name ||
@@ -1480,7 +1480,7 @@  void print_commit_summary(struct repository *r,
 	diff_setup_done(&rev.diffopt);
 
 	refs = get_main_ref_store(r);
-	head = refs_resolve_ref_unsafe(refs, "HEAD", 0, NULL, NULL);
+	head = refs_resolve_ref_unsafe(refs, "HEAD", NULL, 0, NULL, NULL);
 	if (!head)
 		die(_("unable to resolve HEAD after creating commit"));
 	if (!strcmp(head, "HEAD"))
@@ -4715,7 +4715,7 @@  static int apply_save_autostash_ref(struct repository *r, const char *refname,
 	if (!refs_ref_exists(get_main_ref_store(r), refname))
 		return 0;
 
-	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,
+	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname, NULL,
 				     RESOLVE_REF_READING, &stash_oid, &flag))
 		return -1;
 	if (flag & REF_ISSYMREF)
@@ -6213,6 +6213,7 @@  static int add_decorations_to_list(const struct commit *commit,
 	const struct name_decoration *decoration = get_name_decoration(&commit->object);
 	const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
 						       "HEAD",
+						       NULL,
 						       RESOLVE_REF_READING,
 						       NULL,
 						       NULL);
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index c9efd74c2b5..ef1d6acbfae 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -184,7 +184,7 @@  static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
 	int flags;
 	const char *ref;
 
-	ref = refs_resolve_ref_unsafe(refs, refname, resolve_flags,
+	ref = refs_resolve_ref_unsafe(refs, refname, NULL, resolve_flags,
 				      &oid, &flags);
 	printf("%s %s 0x%x\n", oid_to_hex(&oid), ref ? ref : "(null)", flags);
 	return ref ? 0 : 1;
diff --git a/transport-helper.c b/transport-helper.c
index 780fcaf5292..27d85748b60 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1110,6 +1110,7 @@  static int push_refs_with_export(struct transport *transport,
 					/* Follow symbolic refs (mainly for HEAD). */
 					name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
 								       ref->peer_ref->name,
+								       NULL,
 								       RESOLVE_REF_READING,
 								       &oid,
 								       &flag);
diff --git a/transport.c b/transport.c
index 0ad04b77fd2..8818ba75e7a 100644
--- a/transport.c
+++ b/transport.c
@@ -101,7 +101,7 @@  static void set_upstreams(struct transport *transport, struct ref *refs,
 		localname = ref->peer_ref->name;
 		remotename = ref->name;
 		tmp = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-					      localname, RESOLVE_REF_READING,
+					      localname, NULL, RESOLVE_REF_READING,
 					      NULL, &flag);
 		if (tmp && flag & REF_ISSYMREF &&
 			starts_with(tmp, "refs/heads/"))
diff --git a/upload-pack.c b/upload-pack.c
index bbfb04c8bd8..4fddc081237 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1291,7 +1291,7 @@  static int find_symref(const char *refname,
 	if ((flag & REF_ISSYMREF) == 0)
 		return 0;
 	symref_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
-						refname, 0, NULL, &flag);
+						refname, NULL, 0, NULL, &flag);
 	if (!symref_target || (flag & REF_ISSYMREF) == 0)
 		die("'%s' is a symref but it is not?", refname);
 	item = string_list_append(cb_data, strip_namespace(refname));
diff --git a/worktree.c b/worktree.c
index 12eadacc618..ed89bab481d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -42,6 +42,7 @@  static void add_head_info(struct worktree *wt)
 
 	target = refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
 					 "HEAD",
+					 NULL,
 					 0,
 					 &wt->head_oid, &flags);
 	if (!target)
@@ -448,7 +449,7 @@  int is_shared_symref(const struct worktree *wt, const char *symref,
 	}
 
 	refs = get_worktree_ref_store(wt);
-	symref_target = refs_resolve_ref_unsafe(refs, symref, 0,
+	symref_target = refs_resolve_ref_unsafe(refs, symref, NULL, 0,
 						NULL, &flags);
 	if ((flags & REF_ISSYMREF) &&
 	    symref_target && !strcmp(symref_target, target))
@@ -549,6 +550,7 @@  int other_head_refs(each_ref_fn fn, void *cb_data)
 		strbuf_worktree_ref(wt, &refname, "HEAD");
 		if (refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
 					    refname.buf,
+					    NULL,
 					    RESOLVE_REF_READING,
 					    &oid, &flag))
 			ret = fn(refname.buf, &oid, flag, cb_data);