From: shejialuo <shejialuo@gmail.com>
Date: Thu, 15 Jan 2026 12:52:59 GMT
Subject: Re: [PATCH 16/17] builtin/fsck: move generic HEAD check into `refs_fsck()`
Message-ID: <aWjjK8fmf8L7vlNi@ArchLinux>
In-Reply-To: <aWSuObEsFaxi1NAf@pks.im>

On Mon, Jan 12, 2026 at 09:18:01AM +0100, Patrick Steinhardt wrote:
> On Sat, Jan 10, 2026 at 09:31:07PM +0800, shejialuo wrote:
> > 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.
> 

Make sense.

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

Yeah, I cannot give some better idea, either. Let's keep this as-is :)

> Thanks!
> 
> Patrick