NFC: Remove .providedLibrary from PackageReference and its package container

xedin · xedin · commit ec680ec3c5c6 · 2024-04-26T13:45:59.000-07:00
Dependency resolution is switched to use `BoundVersion` to carry
provided library information, `.providedLibrary` is fully obsolete.
diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift
@@ -740,7 +740,7 @@ extension ResolvedPackage {
         switch self.underlying.manifest.packageKind {
         case .registry, .remoteSourceControl, .localSourceControl:
             return true
-        case .root, .fileSystem, .providedLibrary:
+        case .root, .fileSystem:
             return false
         }
     }
diff --git a/Sources/PackageGraph/CMakeLists.txt b/Sources/PackageGraph/CMakeLists.txt
@@ -20,7 +20,6 @@ add_library(PackageGraph
   PackageModel+Extensions.swift
   PackageRequirement.swift
   PinsStore.swift
-  ProvidedLibraryPackageContainer.swift
   Resolution/PubGrub/Assignment.swift
   Resolution/PubGrub/ContainerProvider.swift
   Resolution/PubGrub/DiagnosticReportBuilder.swift
diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift
@@ -362,8 +362,6 @@ extension PackageGraphError: CustomStringConvertible {
                     description += " (at '\(path)')"
                 case .remoteSourceControl(let url):
                     description += " (from '\(url)')"
-                case .providedLibrary(let url, let path):
-                    description += " (from \(url) at \(path))"
                 case .registry:
                     break
                 }
diff --git a/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift b/Sources/PackageGraph/ProvidedLibraryPackageContainer.swift
diff --git a/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift b/Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift
@@ -98,13 +98,6 @@ final class ContainerProvider {
                 let result = result.tryMap { container -> PubGrubPackageContainer in
                     let pubGrubContainer = PubGrubPackageContainer(underlying: container, pins: self.pins)
 
-                    // This container is not cached because it's intended to be transparent
-                    // and requested only when forming final assignments. Caching it would
-                    // mean that subsequent calls to `solve` would pick it up.
-                    if case .providedLibrary = package.kind {
-                        return pubGrubContainer
-                    }
-
                     // only cache positive results
                     self.containersCache[package] = pubGrubContainer
                     return pubGrubContainer
diff --git a/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift b/Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift
@@ -895,7 +895,7 @@ extension PackageRequirement {
 extension PackageReference {
     public func matchingPrebuiltLibrary(in availableLibraries: [ProvidedLibrary]) -> ProvidedLibrary? {
         switch self.kind {
-        case .fileSystem, .localSourceControl, .root, .providedLibrary:
+        case .fileSystem, .localSourceControl, .root:
             return nil // can never match a prebuilt library
         case .registry(let identity):
             if let registryIdentity = identity.registry {
diff --git a/Sources/PackageLoading/ManifestJSONParser.swift b/Sources/PackageLoading/ManifestJSONParser.swift
@@ -83,7 +83,7 @@ enum ManifestJSONParser {
         case .localSourceControl(let _packagePath):
             // we have a more accurate path than the virtual one
             packagePath = _packagePath
-        case .root(let _packagePath), .fileSystem(let _packagePath), .providedLibrary(_, let _packagePath):
+        case .root(let _packagePath), .fileSystem(let _packagePath):
             // we dont have a more accurate path, and they should be the same
             // asserting (debug only) to make sure refactoring is correct 11/2023
             assert(packagePath == _packagePath, "expecting package path '\(packagePath)' to be the same as '\(_packagePath)'")
diff --git a/Sources/PackageModel/IdentityResolver.swift b/Sources/PackageModel/IdentityResolver.swift
@@ -46,8 +46,6 @@ public struct DefaultIdentityResolver: IdentityResolver {
             return try self.resolveIdentity(for: url)
         case .registry(let identity):
             return identity
-        case .providedLibrary(let url, _):
-            return try self.resolveIdentity(for: url)
         }
     }
 
diff --git a/Sources/PackageModel/PackageReference.swift b/Sources/PackageModel/PackageReference.swift
@@ -34,9 +34,6 @@ public struct PackageReference {
         /// A package from  a registry.
         case registry(PackageIdentity)
 
-        /// A prebuilt library provided by a toolchain for a package identified by the given "origin" URL.
-        case providedLibrary(SourceControlURL, AbsolutePath)
-
         // FIXME: we should not need this once we migrate off URLs
         //@available(*, deprecated)
         public var locationString: String {
@@ -52,8 +49,6 @@ public struct PackageReference {
             case .registry(let identity):
                 // FIXME: this is a placeholder
                 return identity.description
-            case .providedLibrary(let url, _):
-                return url.absoluteString
             }
         }
 
@@ -75,8 +70,6 @@ public struct PackageReference {
                 return "remoteSourceControl \(url)"
             case .registry(let identity):
                 return "registry \(identity)"
-            case .providedLibrary(let url, let path):
-                return "provided library for \(url) @ \(path)"
             }
         }
 
@@ -131,8 +124,6 @@ public struct PackageReference {
         case .registry(let identity):
             // FIXME: this is a placeholder
             self.deprecatedName = name ?? identity.description
-        case .providedLibrary(let url, _):
-            self.deprecatedName = name ?? PackageIdentityParser.computeDefaultName(fromURL: url)
         }
     }
 
@@ -160,14 +151,6 @@ public struct PackageReference {
     public static func registry(identity: PackageIdentity) -> PackageReference {
         PackageReference(identity: identity, kind: .registry(identity))
     }
-
-    public static func providedLibrary(
-        identity: PackageIdentity,
-        origin: SourceControlURL,
-        path: AbsolutePath
-    ) -> PackageReference {
-        PackageReference(identity: identity, kind: .providedLibrary(origin, path))
-    }
 }
 
 extension PackageReference: Equatable {
@@ -187,11 +170,6 @@ extension PackageReference: Equatable {
         switch (self.kind, other.kind) {
         case (.remoteSourceControl(let lurl), .remoteSourceControl(let rurl)):
             return lurl.canonicalURL == rurl.canonicalURL
-        case (.remoteSourceControl(let lurl), .providedLibrary(let rurl, _)),
-             (.providedLibrary(let lurl, _), .remoteSourceControl(let rurl)):
-            return lurl.canonicalURL == rurl.canonicalURL
-        case (.providedLibrary(_, let lpath), .providedLibrary(_, let rpath)):
-            return lpath == rpath
         default:
             return true
         }
@@ -246,10 +224,6 @@ extension PackageReference.Kind: Encodable {
         case .registry:
             var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .registry)
             try unkeyedContainer.encode(self.isRoot)
-        case .providedLibrary(let url, let path):
-            var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .providedLibrary)
-            try unkeyedContainer.encode(url)
-            try unkeyedContainer.encode(path)
         }
     }
 }
diff --git a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift
@@ -240,8 +240,6 @@ internal struct PluginContextSerializer {
                 return .repository(url: url.absoluteString, displayVersion: String(describing: package.manifest.version), scmRevision: String(describing: package.manifest.revision))
             case .registry(let identity):
                 return .registry(identity: identity.description, displayVersion: String(describing: package.manifest.version))
-            case .providedLibrary(_, _):
-                throw InternalError("provided libraries are not supported in plugin context")
             }
         }
 
diff --git a/Sources/SPMTestSupport/MockManifestLoader.swift b/Sources/SPMTestSupport/MockManifestLoader.swift
@@ -109,9 +109,6 @@ extension ManifestLoader {
             packageIdentity = identity
             // FIXME: placeholder
             packageLocation = identity.description
-        case .providedLibrary(let url, let path):
-            packageIdentity = try identityResolver.resolveIdentity(for: url)
-            packageLocation = path.pathString
         }
         return try await self.load(
             manifestPath: manifestPath,
@@ -159,9 +156,6 @@ extension ManifestLoader {
             packageIdentity = identity
             // FIXME: placeholder
             packageLocation = identity.description
-        case .providedLibrary(let url, let path):
-            packageIdentity = try identityResolver.resolveIdentity(for: url)
-            packageLocation = path.pathString
         }
         return try await self.load(
             packagePath: packagePath,
diff --git a/Sources/SPMTestSupport/MockWorkspace.swift b/Sources/SPMTestSupport/MockWorkspace.swift
@@ -1035,8 +1035,6 @@ extension PackageReference.Kind {
             return "remoteSourceControl"
         case .registry:
             return "registry"
-        case .providedLibrary:
-            return "library"
         }
     }
 }
diff --git a/Sources/Workspace/Diagnostics.swift b/Sources/Workspace/Diagnostics.swift
@@ -183,7 +183,7 @@ extension Basics.Diagnostic {
             switch $0.kind {
             case .registry(let identity):
                 return "'\(identity.description)'"
-            case .remoteSourceControl(let url), .providedLibrary(let url, _):
+            case .remoteSourceControl(let url):
                 return "'\($0.identity)' from \(url)"
             case .localSourceControl(let path), .fileSystem(let path), .root(let path):
                 return "'\($0.identity)' at \(path)"
diff --git a/Sources/Workspace/Workspace+Dependencies.swift b/Sources/Workspace/Workspace+Dependencies.swift
@@ -33,7 +33,6 @@ import struct PackageGraph.ObservabilityDependencyResolverDelegate
 import struct PackageGraph.PackageContainerConstraint
 import struct PackageGraph.PackageGraphRoot
 import struct PackageGraph.PackageGraphRootInput
-import struct PackageGraph.ProvidedLibraryPackageContainer
 import class PackageGraph.PinsStore
 import struct PackageGraph.PubGrubDependencyResolver
 import struct PackageGraph.Term
diff --git a/Sources/Workspace/Workspace+PackageContainer.swift b/Sources/Workspace/Workspace+PackageContainer.swift
@@ -17,7 +17,6 @@ import enum PackageFingerprint.FingerprintCheckingMode
 import enum PackageGraph.ContainerUpdateStrategy
 import protocol PackageGraph.PackageContainer
 import protocol PackageGraph.PackageContainerProvider
-import struct PackageGraph.ProvidedLibraryPackageContainer
 import struct PackageModel.PackageReference
 
 // MARK: - Package container provider
@@ -94,14 +93,6 @@ extension Workspace: PackageContainerProvider {
                 queue.async {
                     completion(.success(container))
                 }
-
-            case .providedLibrary:
-                let container = try ProvidedLibraryPackageContainer(
-                    package: package,
-                    observabilityScope: observabilityScope)
-                queue.async {
-                    completion(.success(container))
-                }
             }
         } catch {
             queue.async {
diff --git a/Sources/Workspace/Workspace+State.swift b/Sources/Workspace/Workspace+State.swift
diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift
diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift
diff --git a/Tests/PackageLoadingTests/PDLoadingTests.swift b/Tests/PackageLoadingTests/PDLoadingTests.swift

Original file line number	Diff line number	Diff line change
`@@ -740,7 +740,7 @@ extension ResolvedPackage {`
`740`	`740`	`switch self.underlying.manifest.packageKind {`
`741`	`741`	`case .registry, .remoteSourceControl, .localSourceControl:`
`742`	`742`	`return true`
`743`		`- case .root, .fileSystem, .providedLibrary:`
	`743`	`+ case .root, .fileSystem:`
`744`	`744`	`return false`
`745`	`745`	`}`
`746`	`746`	`}`
Original file line number	Diff line number	Diff line change
`@@ -362,8 +362,6 @@ extension PackageGraphError: CustomStringConvertible {`
`362`	`362`	`description += " (at '\(path)')"`
`363`	`363`	`case .remoteSourceControl(let url):`
`364`	`364`	`description += " (from '\(url)')"`
`365`		`- case .providedLibrary(let url, let path):`
`366`		`- description += " (from \(url) at \(path))"`
`367`	`365`	`case .registry:`
`368`	`366`	`break`
`369`	`367`	`}`
Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,6 @@ public struct DefaultIdentityResolver: IdentityResolver {`
`46`	`46`	`return try self.resolveIdentity(for: url)`
`47`	`47`	`case .registry(let identity):`
`48`	`48`	`return identity`
`49`		`- case .providedLibrary(let url, _):`
`50`		`- return try self.resolveIdentity(for: url)`
`51`	`49`	`}`
`52`	`50`	`}`
`53`	`51`
Original file line number	Diff line number	Diff line change
`@@ -34,9 +34,6 @@ public struct PackageReference {`
`34`	`34`	`/// A package from a registry.`
`35`	`35`	`case registry(PackageIdentity)`
`36`	`36`
`37`		`- /// A prebuilt library provided by a toolchain for a package identified by the given "origin" URL.`
`38`		`- case providedLibrary(SourceControlURL, AbsolutePath)`
`39`		`-`
`40`	`37`	`// FIXME: we should not need this once we migrate off URLs`
`41`	`38`	`//@available(*, deprecated)`
`42`	`39`	`public var locationString: String {`
`@@ -52,8 +49,6 @@ public struct PackageReference {`
`52`	`49`	`case .registry(let identity):`
`53`	`50`	`// FIXME: this is a placeholder`
`54`	`51`	`return identity.description`
`55`		`- case .providedLibrary(let url, _):`
`56`		`- return url.absoluteString`
`57`	`52`	`}`
`58`	`53`	`}`
`59`	`54`
`@@ -75,8 +70,6 @@ public struct PackageReference {`
`75`	`70`	`return "remoteSourceControl \(url)"`
`76`	`71`	`case .registry(let identity):`
`77`	`72`	`return "registry \(identity)"`
`78`		`- case .providedLibrary(let url, let path):`
`79`		`- return "provided library for \(url) @ \(path)"`
`80`	`73`	`}`
`81`	`74`	`}`
`82`	`75`
`@@ -131,8 +124,6 @@ public struct PackageReference {`
`131`	`124`	`case .registry(let identity):`
`132`	`125`	`// FIXME: this is a placeholder`
`133`	`126`	`self.deprecatedName = name ?? identity.description`
`134`		`- case .providedLibrary(let url, _):`
`135`		`- self.deprecatedName = name ?? PackageIdentityParser.computeDefaultName(fromURL: url)`
`136`	`127`	`}`
`137`	`128`	`}`
`138`	`129`
`@@ -160,14 +151,6 @@ public struct PackageReference {`
`160`	`151`	`public static func registry(identity: PackageIdentity) -> PackageReference {`
`161`	`152`	`PackageReference(identity: identity, kind: .registry(identity))`
`162`	`153`	`}`
`163`		`-`
`164`		`- public static func providedLibrary(`
`165`		`- identity: PackageIdentity,`
`166`		`- origin: SourceControlURL,`
`167`		`- path: AbsolutePath`
`168`		`- ) -> PackageReference {`
`169`		`- PackageReference(identity: identity, kind: .providedLibrary(origin, path))`
`170`		`- }`
`171`	`154`	`}`
`172`	`155`
`173`	`156`	`extension PackageReference: Equatable {`
`@@ -187,11 +170,6 @@ extension PackageReference: Equatable {`
`187`	`170`	`switch (self.kind, other.kind) {`
`188`	`171`	`case (.remoteSourceControl(let lurl), .remoteSourceControl(let rurl)):`
`189`	`172`	`return lurl.canonicalURL == rurl.canonicalURL`
`190`		`- case (.remoteSourceControl(let lurl), .providedLibrary(let rurl, _)),`
`191`		`- (.providedLibrary(let lurl, _), .remoteSourceControl(let rurl)):`
`192`		`- return lurl.canonicalURL == rurl.canonicalURL`
`193`		`- case (.providedLibrary(_, let lpath), .providedLibrary(_, let rpath)):`
`194`		`- return lpath == rpath`
`195`	`173`	`default:`
`196`	`174`	`return true`
`197`	`175`	`}`
`@@ -246,10 +224,6 @@ extension PackageReference.Kind: Encodable {`
`246`	`224`	`case .registry:`
`247`	`225`	`var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .registry)`
`248`	`226`	`try unkeyedContainer.encode(self.isRoot)`
`249`		`- case .providedLibrary(let url, let path):`
`250`		`- var unkeyedContainer = container.nestedUnkeyedContainer(forKey: .providedLibrary)`
`251`		`- try unkeyedContainer.encode(url)`
`252`		`- try unkeyedContainer.encode(path)`
`253`	`227`	`}`
`254`	`228`	`}`
`255`	`229`	`}`
Original file line number	Diff line number	Diff line change
`@@ -240,8 +240,6 @@ internal struct PluginContextSerializer {`
`240`	`240`	`return .repository(url: url.absoluteString, displayVersion: String(describing: package.manifest.version), scmRevision: String(describing: package.manifest.revision))`
`241`	`241`	`case .registry(let identity):`
`242`	`242`	`return .registry(identity: identity.description, displayVersion: String(describing: package.manifest.version))`
`243`		`- case .providedLibrary(_, _):`
`244`		`- throw InternalError("provided libraries are not supported in plugin context")`
`245`	`243`	`}`
`246`	`244`	`}`
`247`	`245`
Original file line number	Diff line number	Diff line change
`@@ -1035,8 +1035,6 @@ extension PackageReference.Kind {`
`1035`	`1035`	`return "remoteSourceControl"`
`1036`	`1036`	`case .registry:`
`1037`	`1037`	`return "registry"`
`1038`		`- case .providedLibrary:`
`1039`		`- return "library"`
`1040`	`1038`	`}`
`1041`	`1039`	`}`
`1042`	`1040`	`}`