Re: [PATCH v2 1/4] wt-status: read HEAD and ORIG_HEAD via the refdb

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 13, 2023, 07:38 UTC
Message-ID: <ZXlfeWtDgr1GQFCL@tanuki>
In-Reply-To: <xmqq34w7os53.fsf@gitster.g>

On Tue, Dec 12, 2023 at 04:36:24PM -0800, Junio C Hamano wrote:

Show 18 quoted lines

> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> >> "via the refdb" -> "via the refs API" or something here and on the
> >> title, and possibly elsewhere in the proposed log messages and
> >> in-code comments in patches in this series, as I've never seen a
> >> word "refdb" used in the context of this project.
> >> 
> >> I agree it is bad manners to be intimate with the implementation
> >> details of the how files-backend stores HEAD and ORIG_HEAD.
> >
> > Hmm, I have never thought of the 'pseudo-refs' as being a part of
> > the 'reference database' at all. ;)
> 
> Me neither, but once you start thinking about getting rid of the
> need to use one-file-per-ref filesystem, being able to maintain all
> refs, including the pseudo refs, in one r/w store backend, becomes a
> very tempting goal.  From that point of view, I do not have problem
> with the idea to move _all_ pseudorefs to reftable.

Yes, we're in agreement.

> But I do have reservations on what Patrick, and the code he
> inherited from Han-Wen, calls "special refs" (which is not defined
> in the glossary at all), namely, refs.c:is_special_ref() and its
> callers.

I do not want to add "special refs" to the glossary because ultimately they should go away, with two exceptions: FETCH_HEAD and MERGE_HEAD. Once we're there we can of course discuss whether we want to explicitly point them out in the glossary and give them a special name.

> Neither am I very much sympathetic to the hardcoded list
> of "known" pseudorefs, refs.c:pseudorefs[].  I cannot quite see why
> we need anything more than

Show 20 quoted lines

>     any string that passes refs.c:is_pseudoref_syntax() is a
>     pseudoref, is per worktree, and ref backends can store them like
>     any other refs.  Many of them have specific meaning and uses
>     (e.g. HEAD is "the current branch").
> 
> Enumerating existing pseudorefs in files backend may need to
> opendir(".git") + readdir() filtered with is_pseudoref_syntax(),
> and a corresponding implementation for reftable backend may be much
> simpler (because there won't be "other cruft" stored there, unlike
> files backend that needs to worry about files that are not refs,
> like ".git/config" file.
> 
> > We seem to have pseudo-refs, special pseudo-refs and (recently)
> > ex-pseudo-refs!
> >
> > This patch (well series) changes the 'status' of some, *but not all*,
> > pseudo-refs; some graduate to full-blown refs stored as part of *a*
> > reference database (ie reftable).
> 
> Yeah, that leaves bad taste in my mouth, too.

I'm taking an iterative approach to things, which means that we're at times going to be in an in-between state. I want to avoid changing too many things at once and overwhelming potential reviewers. But I realize that I should've done a better job of explaining that this patch series is not the end goal, but rather a step towards that goal.

The patch series at hand merely records the status quo and rectifies any inconsistencies we have with accessing such "special" refs. The natural next step here would be to reduce the list of special refs (like e.g. we do in patch 4) so that in the end it will only contain those refs which really are special (FETCH_HEAD, MERGE_HEAD).

Please let me know in case you strongly disagree with my iterative approach, or whether the issue is rather that I didn't make myself sufficiently clear.

Patrick