Re: [PATCH 2/3] refs: do not label special refs as pseudo refs

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 4169 bytes --]

On Mon, Apr 29, 2024 at 04:12:37PM +0100, Phillip Wood wrote:
> Hi Patrick
> 
> On 29/04/2024 14:41, Patrick Steinhardt wrote:
> > diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> > index d71b199955..4275918fa0 100644
> > --- a/Documentation/glossary-content.txt
> > +++ b/Documentation/glossary-content.txt
> > @@ -497,20 +497,28 @@ exclude;;
> >   	unusual refs.
> >   [[def_pseudoref]]pseudoref::
> > -	Pseudorefs are a class of files under `$GIT_DIR` which behave
> > -	like refs for the purposes of rev-parse, but which are treated
> > -	specially by git.  Pseudorefs both have names that are all-caps,
> > -	and always start with a line consisting of a
> > -	<<def_SHA1,SHA-1>> followed by whitespace.  So, HEAD is not a
> > -	pseudoref, because it is sometimes a symbolic ref.  They might
> > -	optionally contain some additional data.  `MERGE_HEAD` and
> > -	`CHERRY_PICK_HEAD` are examples.  Unlike
> > -	<<def_per_worktree_ref,per-worktree refs>>, these files cannot
> > -	be symbolic refs, and never have reflogs.  They also cannot be
> > -	updated through the normal ref update machinery.  Instead,
> > -	they are updated by directly writing to the files.  However,
> > -	they can be read as if they were refs, so `git rev-parse
> > -	MERGE_HEAD` will work.
> > +	Pseudorefs are references that live in the root of the reference
> > +	hierarchy, outside of the usual "refs/" hierarchy. Pseudorefs have an
> > +	all-uppercase name and must end with a "_HEAD" suffix, for example
> > +	"`BISECT_HEAD`". Other than that, pseudorefs behave the exact same as
> > +	any other reference and can be both read and written via regular Git
> > +	tooling.
> 
> This changes the definition to allow pseudorefs to by symbolic refs. When
> is_pseudoref() was introduced Junio and I had a brief discussion about this
> restriction and he was not in favor of allowing pseudorefs to be symbolic
> refs [1].

So the reason why pseudorefs exist is that some refs behave like a ref
sometimes, but not always. And in my book that really only applies to
MERGE_HEAD and FETCH_HEAD, because those contain additional metadata
that makes them not-a-ref. And for those I very much see that they
should not ever be a symref.

But everyhing else living in the root of the ref hierarchy is not
special in any way, at least not in my opinion. We have never enforced
that those cannot be symrefs, and it makes our terminology needlessly
confusing.

I think I'm going to reroll this patch series and go down the nuclear
path that I've hinted at in the cover letter:

  - Pseudo refs can only be either FETCH_HEAD or MERGE_HEAD.

  - Refs starting with "refs/" are just plain normal refs.

  - Refs living in the root of the ref hierarchy need to conform to a
    set of strict rules, as Peff is starting to enforce in a separate
    patch series. These are just normal refs, as well, even though we
    may call them "root ref" in our tooling as they live in the root of
    the ref hierarchy.

I just don't think that the current state makes sense to anybody. It's
majorly confusing -- I've spent the last 8 months working in our refs
code almost exclusively and still forget what's what. How are our users
expected to understand this?

> Are there any practical implications of the changes in this patch for users
> running commands like "git log FETCH_HEAD" (I can't think of any off the top
> of my head but it would be good to have some reassurance on that point in
> the commit message)

Not really, no. We have never been doing a good job at enforcing the
difference between pseudo refs or normal refs anyway. Pseudo refs can be
symrefs just fine, and our tooling won't complain. The only exception
where I want us to become stricter is in how we enforce the syntax rules
for root refs (which is handled by Peff in a separate patch series), and
that we start to not treat FETCH_HEAD and MERGE_HEAD as proper refs.
They should still resolve when you ask git-rev-parse(1), but when you
iterate through refs they should not be surfaced as they _aren't_ refs.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]