Re: [PATCH v2 07/10] refs: root refs can be symbolic refs

> Before this patch series, root refs except for "HEAD" and our special
> refs were classified as pseudorefs. Furthermore, our terminology
> clarified that pseudorefs must not be symbolic refs. This restriction
> is enforced in `is_root_ref()`, which explicitly checks that a supposed
> root ref resolves to an object ID without recursing.
> 
> This has been extremely confusing right from the start because (in old
> terminology) a ref name may sometimes be a pseudoref and sometimes not
> depending on whether it is a symbolic or regular ref. This behaviour
> does not seem reasonable at all and I very much doubt that it results in
> anything sane.
> 
> Furthermore, the behaviour is different to `is_headref()`, which only
> checks for the ref to exist. While that is in line with our glossary,
> this inconsistency only adds to the confusion.
> 
> Last but not least, the current behaviour can actually lead to a
> segfault when calling `is_root_ref()` with a reference that either does
> not exist or that is a symbolic ref because we never intialized `oid`.

> Let's loosen the restrictions in accordance to the new definition of
> root refs, which are simply plain refs that may as well be a symbolic
> ref. Consequently, we can just check for the ref to exist instead of
> requiring it to be a regular ref.
> 
> Add a test that verifies that this does not change user-visible
> behaviour. Namely, we still don't want to show broken refs to the user
> by default in git-for-each-ref(1). What this does allow though is for
> internal callers to surface dangling root refs when they pass in the
> `DO_FOR_EACH_INCLUDE_BROKEN` flag.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  refs.c                         | 50 ++++++++++++++++++++++++----------
>  t/t6302-for-each-ref-filter.sh | 17 ++++++++++++
>  2 files changed, 53 insertions(+), 14 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 5b89e83ad7..ca9844bc3e 100644
> --- a/refs.c
> +++ b/refs.c

>  int is_headref(struct ref_store *refs, const char *refname)
>  {
> -	if (!strcmp(refname, "HEAD"))
> -		return refs_ref_exists(refs, refname);
> +	struct strbuf referent = STRBUF_INIT;
> +	struct object_id oid = { 0 };
> +	int failure_errno, ret = 0;
> +	unsigned int flags;
>  
> -	return 0;
> +	/*
> +	 * Note that we cannot use `refs_ref_exists()` here because that also
> +	 * checks whether its target ref exists in case refname is a symbolic
> +	 * ref.
> +	 */
> +	if (!strcmp(refname, "HEAD")) {
> +		ret = !refs_read_raw_ref(refs, refname, &oid, &referent,
> +					 &flags, &failure_errno);
> +	}
> +
> +	strbuf_release(&referent);
> +	return ret;
>  }