remove metadata types from the in-memory branches #4714
Conversation
| import Unison.Util.List qualified as List | ||
| import Unison.Util.Relation qualified as R | ||
| import Unison.Util.Star3 (Star3) | ||
| import Unison.Util.Star3 qualified as Star3 | ||
|
|
||
| type Type = Reference | ||
| type Type = () -- dummy value, intermediate phase of removing metadata altogether |
There was a problem hiding this comment.
I'm a little confused, what's the purpose of keeping a dummy type here? Is this being used anywhere that we can't also just delete?
There was a problem hiding this comment.
I think we'd need to write a Star2 type and drop the Type index here. It would be better, but it just felt like busywork that was already slated to be thrown out.
There was a problem hiding this comment.
an hour later and no end in sight... it doesn't help that HLS lied about what's safe to delete, when it hadn't loaded all the packages yet to know.
There was a problem hiding this comment.
Alternative at https://github.com/unisonweb/unison/compare/topic/NeedTypeForBuiltinMetadata-more
I agree it is better, but in retrospect also agree with past self that it's hard to justify the time expenditure when it's going to be tossed anyway.
| @@ -530,9 +532,9 @@ branch2to1 branchCache lookupCT (V2.Branch.Branch v2terms v2types v2patches v2ch | |||
| let facts = Set.singleton ref | |||
| names = Relation.singleton ref name | |||
| types :: Relation.Relation ref V1.Metadata.Type = | |||
| Relation.insertManyRan ref (fmap mdref2to1 (Map.elems mdvals)) mempty | |||
| Relation.insertManyRan ref (fmap (const ()) (Set.toList mdvals)) mempty | |||
| vals :: Relation.Relation ref (V1.Metadata.Type, V1.Metadata.Value) = | |||
There was a problem hiding this comment.
Could this just be Relation.Relation ref V1.Metadata.Value now?
It was just the minimal patch to let #4416 stop crashing, which I want because I haven't been able to open my codebase for over three months 😓. The more comprehensive cleanup would be nice, and I did attempt that initially (i.e. eliminate |
Overview
This pretty hackily resolves #4416, where ucm would crash if a codebase included a term with a builtin associated as metadata, because the types of metadata are needed when loading a branch, but the types of builtins were not available when loading a branch, due to current package boundaries.
Since our long-term plan is to remove metadata altogether, with metadata-related commands and output already having been deleted in #4574, #4428, #4333, it seemed like a poor use of time to reorganize the packages to support this situation, and instead this PR removes "metadata type" from in-memory data structures.
Metadata type is not part of the hashing function of namespaces, so this should not affect anything but make the codebase a bit more skeleton-ish and weird.
Implementation notes
In
Unison.Codebase.Metadata, we replacewith
And then fix up the dependent spots.
I just realized it could have been a smaller diff except I also changed
U.Codebase.Branch.Typefromto
Test coverage
Relying on existing tests to make sure no behaviors have changed.