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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

On Tue, Apr 30, 2024 at 12:09:57PM -0500, Justin Tobler wrote:
> On 24/04/30 02:26PM, Patrick Steinhardt wrote:
[snip]
> > 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;
> >  }
> 
> I'm not quite sure I understand why we are changing the behavior of
> `is_headref()` here. Do we no longer want to validate the ref exists if
> it is symbolic?

The implementation does not conform to the definition of a "HEAD" ref.
Even before this patch series, a "HEAD" ref could either be a symbolic
or a regular ref. So to answer the question of "Is this a HEAD ref?" you
only need to check whether the ref exists, not whether its target
exists.

> In a prior commit, `is_headref()` is commented to mention that we check
> whether the reference exists. Maybe that could use some additional
> clarification?

Which particular commit do you refer to? It's not part of this series,
is it?

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]