Hi Patrick
On 30/04/2024 08:30, Patrick Steinhardt wrote:
On Mon, Apr 29, 2024 at 04:12:37PM +0100, Phillip Wood wrote:
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 agree HEAD not being a pseudoref and having special refs as well as
pseudorefs refs is confusing. I do have some sympathy for the argument
that pseudorefs should not be symbolic refs though as AUTO_MERGE,
CHERRY_PICK_HEAD, ORIG_HEAD etc. are all pointers to a commit and it
would be a bug for them to be a symbolic ref. It is unfortunate that in
the move away from assessing those refs as files we lost the check that
they are not symbolic refs.
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.
That would certainly be simpler.
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?
The current state is confusing but arguably there is a logic to the
various distinctions - whether those distinctions are useful in practice
is open to debate though. I wonder how much users really care about
these distinctions and whether it affects their use of git. I was
unaware of the distinction between HEAD and pseudorefs until I reviewed
Karthik's for-each-ref series a couple of months ago and I don't think
that lack of knowledge had caused me any trouble when using git.
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.
That's good
Thanks
Phillip
