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

Patrick Steinhardt
2024-05-02T08:08:03Z

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