Re: [PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 12, 2026, 08:18 UTC
- Message-ID
- <aWSuObEsFaxi1NAf@pks.im>
- In-Reply-To
- <aWJUm-hrPquegbdf@ArchLinux>
On Sat, Jan 10, 2026 at 09:31:07PM +0800, shejialuo wrote:
Show 32 quoted lines
> On Fri, Jan 09, 2026 at 01:39:45PM +0100, Patrick Steinhardt wrote:
> > diff --git a/refs.c b/refs.c
> > index c3528862c6..a772d371cd 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,
> >
> > int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o,
> > struct fsck_ref_report *report,
> > - const char *refname UNUSED, const char *target)
> > + const char *refname, const char *target)
> > {
> > + const char *stripped_refname;
> > +
> > + parse_worktree_ref(refname, NULL, NULL, &stripped_refname);
> > +
> > + if (!strcmp(stripped_refname, "HEAD") &&
> > + !starts_with(target, "refs/heads/") &&
>
> We would first check whether the current ref is `HEAD`. And I am
> wondering whether we have some common APIs. And I find the similar logic
> in `reglog.c::is_head` like the following shows:
>
> static int is_head(const char *refname)
> {
> const char *stripped_refname;
> parse_worktree_ref(refname, NULL, NULL, &stripped_refname);
> return !strcmp(stripped_refname, "HEAD");
> }
>
> I think we might just extract this common logic to avoid introducing
> repetition.Hm. We could, but I'm a tiny bit worried about just calling it `is_head()`. It might be surprising to some callers that there isn't only one "HEAD", but that this would also recognize worktree HEADs. If somebody just goes like "I wanna know whether I've got HEAD" they might not think about that at all.
So given that the complexity is comparatively low I'd prefer to keep this as-is for now. On the other hand, if you've got some proposal for how to make this interface not confusing I'm very open to that :)
Thanks!
Patrick