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

From: Patrick Steinhardt <ps@pks.im>
Date: May 2, 2024, 08:07 UTC
Message-ID: <ZjNJ3j2wSgui_cG8@tanuki>
In-Reply-To: <zmnute2cow2rbrv3cj5cq4roieyzssaxcnorxhorzyp3wfgllt@ubhsw6dpi5js>

On Tue, Apr 30, 2024 at 12:09:57PM -0500, Justin Tobler wrote:

> On 24/04/30 02:26PM, Patrick Steinhardt wrote:

[snip]

Show 32 quoted lines

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