On Tue, Apr 30, 2024 at 10:59:36AM +0100, Phillip Wood wrote: > 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. While I agree that conceptually these should always be "regular" refs, I feel like that is higher-level logic that belongs into the respective subsystems that write those. I just don't see why the ref backend should care about the particular usecases that those higher-level subsystems have, and I can very much see that there might eventually be another subsystem that actually wants a specific ref to be a symref. No we could of course start to hard code all kinds of refs into the ref layer. But I think that this is the wrong way to go, and treating the ref store as just that, a generic store where you can store refs, without attaching specific meaning to any of the refs, is the proper way to go. > > 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. There is some logic, that's true enough. I just don't think that anybody understands the logic. Patrick > > > 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 >
Attachment:
signature.asc
Description: PGP signature
