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

[Patrick Steinhardt <ps@pks.im>]

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

On Tue, Apr 30, 2024 at 06:23:10AM -0400, Jeff King wrote:
> On Tue, Apr 30, 2024 at 09:30:05AM +0200, Patrick Steinhardt wrote:
[snip]
> > > 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.
> 
> I actually would not even mind if they are surfaced when iterating with
> --include-root-refs. But then I am a little skeptical of the purpose of
> that feature in the first place. After all, the reason code shoves stuff
> into .git/FOO_HEAD is precisely because we don't want other stuff
> iterating over them, using them for reachability, and so on. That is why
> "--all" does not include them, for example.
> 
> But I did not follow the development of the feature, so maybe I am
> missing some cool use case.

The thing is that once we start to surface pseudorefs (in the sense of
these _really_ aren't refs) in ref-related tooling, users will want to
treat them as a ref, as well. And that's just bound to happen with
plumbing like `git for-each-ref`, where a user may rightfully expect
that all output here can be treated like a normal ref.

In fact though, I want to double down on restrictions regarding the
pseudorefs FETCH_HEAD and MERGE_HEAD. While it's fair enough that those
can be read like a ref, writing to them is a totally different thing. It
does not make any sense to try and write such refs, and our abstractions
aren't even prepared to write them correctly. They go through the ref
backend, and thus the "reftable" backend would write them into the
reftable stack instead of into the filesystem. Now you could argue that
this should be fixed, but I don't think it is reasonable to expect the
reftable backend to start writing loose refs for those pseudorefs.

So I'd really like to stick with the current explanation that we have in
the "special ref" glossary: pseudorefs must be written via the
filesystem and can't ever go through the ref backends.

Patrick

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