Add PackageIdentity type (#3057)

* Replace PackageReference.PackageIdentity with top-level PackageIdentity type

* Change access level of PackageReference.computeDefaultName to internal

* Extract testPackageReference into new PackageIdentityTests file

* Allow package identity to be configurable via dependency injection

Rename PackageIdentityTests to LegacyPackageIdentityTests

* Revert change to access level of computeDefaultName, making it public

* Remove LosslessStringConvertible conformance

* Add documentation comment for provider type property

* Add initializer that takes an AbsolutePath parameter

* Remove commented @warn_unqualified_access attribute

* Add argument label to public PackageIdentity initializers

Distinguish between path and URL initializers at call sites

Author: mattt

Sources/PackageCollections/PackageCollections.swift

@@ -298,7 +298,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
     }
 
     func findPackage(
-        identifier: PackageReference.PackageIdentity,
+        identifier: PackageIdentity,
         profile: PackageCollectionsModel.Profile? = nil,
         callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void
     ) {

Sources/PackageCollections/Storage/PackageCollectionsStorage.swift

@@ -69,7 +69,7 @@ public protocol PackageCollectionsStorage {
     ///   - identifier: The package identifier
     ///   - collectionIdentifiers: Optional. The identifiers of the `PackageCollection`s
     ///   - callback: The closure to invoke when result becomes available
-    func findPackage(identifier: PackageReference.PackageIdentity,
+    func findPackage(identifier: PackageIdentity,
                      collectionIdentifiers: [PackageCollectionsModel.CollectionIdentifier]?,
                      callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void)
 
@@ -335,7 +335,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
     }
 
     // TODO: this is PoC for search, need a more performant version of this
-    func findPackage(identifier: PackageReference.PackageIdentity,
+    func findPackage(identifier: PackageIdentity,
                      collectionIdentifiers: [PackageCollectionsModel.CollectionIdentifier]?,
                      callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void) {
         self.list(identifiers: collectionIdentifiers) { result in

Sources/PackageCollections/Utility.swift

@@ -53,7 +53,7 @@ extension PackageReference {
     /// Initializes a `PackageReference` from `RepositorySpecifier`
     init(repository: RepositorySpecifier, kind: PackageReference.Kind = .remote) {
         self.init(
-            identity: PackageReference.computeIdentity(packageURL: repository.url),
+            identity: PackageIdentity(url: repository.url),
             path: repository.url,
             kind: kind
         )

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -38,12 +38,12 @@ extension PackageGraph {
         // the URL but that shouldn't be needed after <rdar://problem/33693433>
         // Ensure that identity and package name are the same once we have an
         // API to specify identity in the manifest file
-        let manifestMapSequence = (root.manifests + externalManifests).map({ (PackageReference.computeIdentity(packageURL: $0.url), $0) })
+        let manifestMapSequence = (root.manifests + externalManifests).map({ (PackageIdentity(url: $0.url), $0) })
         let manifestMap = Dictionary(uniqueKeysWithValues: manifestMapSequence)
         let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
             node.requiredDependencies().compactMap({ dependency in
                 let url = mirrors.effectiveURL(forURL: dependency.url)
-                return manifestMap[PackageReference.computeIdentity(packageURL: url)].map { manifest in
+                return manifestMap[PackageIdentity(url: url)].map { manifest in
                     GraphLoadingNode(manifest: manifest, productFilter: dependency.productFilter)
                 }
             })
@@ -52,11 +52,11 @@ extension PackageGraph {
         // Construct the root manifest and root dependencies set.
         let rootManifestSet = Set(root.manifests)
         let rootDependencies = Set(root.dependencies.compactMap({
-            manifestMap[PackageReference.computeIdentity(packageURL: $0.url)]
+            manifestMap[PackageIdentity(url: $0.url)]
         }))
         let rootManifestNodes = root.manifests.map { GraphLoadingNode(manifest: $0, productFilter: .everything) }
         let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependencyDescription) -> GraphLoadingNode? in
-            guard let manifest = manifestMap[PackageReference.computeIdentity(packageURL: dependency.url)] else { return nil }
+            guard let manifest = manifestMap[PackageIdentity(url: dependency.url)] else { return nil }
             return GraphLoadingNode(manifest: manifest, productFilter: dependency.productFilter)
         }
         let inputManifests = rootManifestNodes + rootDependencyNodes
@@ -209,8 +209,8 @@ private func createResolvedPackages(
     })
 
     // Create a map of package builders keyed by the package identity.
-    let packageMapByIdentity: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{
-        let identity = PackageReference.computeIdentity(packageURL: $0.package.manifest.url)
+    let packageMapByIdentity: [PackageIdentity: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{
+        let identity = PackageIdentity(url: $0.package.manifest.url)
         return (identity, $0)
     }
     let packageMapByName: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{ ($0.package.name, $0) }
@@ -229,7 +229,7 @@ private func createResolvedPackages(
 
                 // Otherwise, look it up by its identity.
                 let url = mirrors.effectiveURL(forURL: dependency.url)
-                let resolvedPackage = packageMapByIdentity[PackageReference.computeIdentity(packageURL: url)]
+                let resolvedPackage = packageMapByIdentity[PackageIdentity(url: url)]
 
                 // We check that the explicit package dependency name matches the package name.
                 if let resolvedPackage = resolvedPackage,
@@ -360,10 +360,10 @@ private func createResolvedPackages(
                     // dependency to share the same name. We don't check this in manifest loading for root-packages so
                     // we can provide a more detailed diagnostic here.
                     let referencedPackageURL = mirrors.effectiveURL(forURL: product.packageBuilder.package.manifest.url)
-                    let referencedPackageIdentity = PackageReference.computeIdentity(packageURL: referencedPackageURL)
+                    let referencedPackageIdentity = PackageIdentity(url: referencedPackageURL)
                     let packageDependency = packageBuilder.package.manifest.dependencies.first { package in
                         let packageURL = mirrors.effectiveURL(forURL: package.url)
-                        let packageIdentity = PackageReference.computeIdentity(packageURL: packageURL)
+                        let packageIdentity = PackageIdentity(url: packageURL)
                         return packageIdentity == referencedPackageIdentity
                     }!
 

Sources/PackageGraph/PackageGraphRoot.swift

@@ -48,7 +48,7 @@ public struct PackageGraphRoot {
     /// Create a package graph root.
     public init(input: PackageGraphRootInput, manifests: [Manifest], explicitProduct: String? = nil) {
         self.packageRefs = zip(input.packages, manifests).map { (path, manifest) in
-            let identity = PackageReference.computeIdentity(packageURL: manifest.url)
+            let identity = PackageIdentity(url: manifest.url)
             return PackageReference(identity: identity, path: path.pathString, kind: .root)
         }
         self.manifests = manifests

Sources/PackageGraph/PackageModel+Extensions.swift

@@ -23,7 +23,7 @@ extension PackageDependencyDescription {
         //        We should instead use the declared URL of a package dependency
         //        as its identity, as it will be needed for supporting package
         //        registries.
-        let identity = PackageReference.computeIdentity(packageURL: effectiveURL)
+        let identity = PackageIdentity(url: effectiveURL)
 
         return PackageReference(
             identity: identity,

Sources/PackageGraph/PinsStore.swift

@@ -14,7 +14,7 @@ import SourceControl
 import PackageModel
 
 public final class PinsStore {
-    public typealias PinsMap = [PackageReference.PackageIdentity: PinsStore.Pin]
+    public typealias PinsMap = [PackageIdentity: PinsStore.Pin]
 
     public struct Pin: Equatable {
         /// The package reference of the pinned dependency.
@@ -135,7 +135,7 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable {
     public init(json: JSON) throws {
         let name: String? = json.get("package")
         let url: String = try json.get("repositoryURL")
-        let identity = PackageReference.computeIdentity(packageURL: url)
+        let identity = PackageIdentity(url: url)
         let ref = PackageReference(identity: identity, path: url)
         self.packageRef = name.flatMap(ref.with(newName:)) ?? ref
         self.state = try json.get("state")

Sources/PackageGraph/Pubgrub/Incompatibility.swift

@@ -105,7 +105,7 @@ extension Incompatibility {
         case noAvailableVersion
 
         /// A version-based dependency contains unversioned-based dependency.
-        case versionBasedDependencyContainsUnversionedDependency(versionedDependency: String, unversionedDependency: String)
+        case versionBasedDependencyContainsUnversionedDependency(versionedDependency: PackageReference, unversionedDependency: PackageReference)
 
         /// The package's tools version is incompatible.
         case incompatibleToolsVersion

Sources/PackageGraph/Pubgrub/PubgrubDependencyResolver.swift

@@ -396,7 +396,7 @@ public final class PubgrubDependencyResolver {
         pinsMap: PinsStore.PinsMap = [:]
     ) throws -> [(container: PackageReference, binding: BoundVersion, products: ProductFilter)] {
         let root = DependencyResolutionNode.root(package: PackageReference(
-            identity: "<synthesized-root>",
+            identity: PackageIdentity(url: "<synthesized-root>"),
             path: "<synthesized-root-path>",
             name: nil,
             kind: .root
@@ -895,7 +895,7 @@ private final class DiagnosticReportBuilder {
         case .conflict:
             break
         case .versionBasedDependencyContainsUnversionedDependency(let versionedDependency, let unversionedDependency):
-            return "package \(versionedDependency) is required using a version-based requirement and it depends on unversion package \(unversionedDependency)"
+            return "package \(versionedDependency.identity) is required using a version-based requirement and it depends on unversion package \(unversionedDependency.identity)"
         case .incompatibleToolsVersion:
             let term = incompatibility.terms.first!
             return "\(description(for: term, normalizeRange: true)) contains incompatible tools version"
@@ -979,7 +979,7 @@ private final class DiagnosticReportBuilder {
 
     // FIXME: This is duplicated and wrong.
     private func isFailure(_ incompatibility: Incompatibility) -> Bool {
-        return incompatibility.terms.count == 1 && incompatibility.terms.first?.node.package.identity == "<synthesized-root>"
+        return incompatibility.terms.count == 1 && incompatibility.terms.first?.node.package.identity == PackageIdentity(url: "<synthesized-root>")
     }
 
     private func description(for term: Term, normalizeRange: Bool = false) -> String {
@@ -1180,8 +1180,8 @@ private final class PubGrubPackageContainer {
             // Version-based packages are not allowed to contain unversioned dependencies.
             guard case .versionSet = dep.requirement else {
                 let cause: Incompatibility.Cause = .versionBasedDependencyContainsUnversionedDependency(
-                    versionedDependency: package.identity,
-                    unversionedDependency: dep.identifier.identity)
+                    versionedDependency: package,
+                    unversionedDependency: dep.identifier)
                 return [Incompatibility(Term(node, .exact(version)), root: root, cause: cause)]
             }
 

Sources/PackageLoading/ManifestLoader.swift

@@ -268,7 +268,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         }
 
         // Get the JSON string for the manifest.
-        let identity = PackageReference.computeIdentity(packageURL: baseURL)
+        let identity = PackageIdentity(url: baseURL)
         let jsonString = try loadJSONString(
             path: inputPath,
             toolsVersion: toolsVersion,
@@ -388,7 +388,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         diagnostics: DiagnosticsEngine?
     ) throws {
         let dependenciesByIdentity = Dictionary(grouping: manifest.dependencies, by: { dependency in
-            PackageReference.computeIdentity(packageURL: dependency.url)
+            PackageIdentity(url: dependency.url)
         })
 
         let duplicateDependencyIdentities = dependenciesByIdentity
@@ -476,7 +476,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     private func loadJSONString(
         path inputPath: AbsolutePath,
         toolsVersion: ToolsVersion,
-        packageIdentity: String,
+        packageIdentity: PackageIdentity,
         fs: FileSystem? = nil,
         diagnostics: DiagnosticsEngine? = nil
     ) throws -> String {
@@ -560,7 +560,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     }
 
     fileprivate struct ManifestCacheKey {
-        let packageIdentity: String
+        let packageIdentity: PackageIdentity
         let pathOrContents: ManifestPathOrContents
         let toolsVersion: ToolsVersion
         let env: [String: String]
@@ -621,7 +621,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
 
     /// Parse the manifest at the given path to JSON.
     fileprivate func parse(
-        packageIdentity: String,
+        packageIdentity: PackageIdentity,
         pathOrContents: ManifestPathOrContents,
         toolsVersion: ToolsVersion
     ) -> ManifestParseResult {
@@ -706,7 +706,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
             // Add the arguments for emitting serialized diagnostics, if requested.
             if serializedDiagnostics, cacheDir != nil {
                 let diaDir = cacheDir.appending(component: "ManifestLoading")
-                let diagnosticFile = diaDir.appending(component: packageIdentity + ".dia")
+                let diagnosticFile = diaDir.appending(component: "\(packageIdentity).dia")
                 try localFileSystem.createDirectory(diaDir, recursive: true)
                 cmd += ["-Xfrontend", "-serialize-diagnostics-path", "-Xfrontend", diagnosticFile.pathString]
                 manifestParseResult.diagnosticFile = diagnosticFile
@@ -939,7 +939,7 @@ extension TSCBasic.Diagnostic.Message {
         .error("invalid type for binary product '\(productName)'; products referencing only binary targets must have a type of 'library'")
     }
 
-    static func duplicateDependency(dependencyIdentity: String) -> Self {
+    static func duplicateDependency(dependencyIdentity: PackageIdentity) -> Self {
         .error("duplicate dependency '\(dependencyIdentity)'")
     }
 

Sources/PackageModel/CMakeLists.txt

@@ -22,6 +22,7 @@ add_library(PackageModel
   ManifestSourceGeneration.swift
   ModuleMapType.swift
   Package.swift
+  PackageIdentity.swift
   PackageReference.swift
   Platform.swift
   Product.swift