Replace PackageGraphRoot.PackageDependency with PackageDependencyDescription (#2997)

* Replace PackageGraphRoot.PackageDependency with PackageDependencyDescription

* Add documentation comment for nothing type property

* Update CMakeLists.txt for PackageModel module

* Replace reference to FilteredDependencyDescription

* Add comment about how package identity is determined

Author: mattt

Sources/PackageGraph/GraphLoadingNode.swift

@@ -32,7 +32,7 @@ public struct GraphLoadingNode: Equatable, Hashable {
     }
 
     /// Returns the dependencies required by this node.
-    internal func requiredDependencies() -> [FilteredDependencyDescription] {
+    internal func requiredDependencies() -> [PackageDependencyDescription] {
         return manifest.dependenciesRequired(for: productFilter)
     }
 }

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -42,7 +42,7 @@ extension PackageGraph {
         let manifestMap = Dictionary(uniqueKeysWithValues: manifestMapSequence)
         let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
             node.requiredDependencies().compactMap({ dependency in
-                let url = config.mirroredURL(forURL: dependency.declaration.url)
+                let url = config.mirroredURL(forURL: dependency.url)
                 return manifestMap[PackageReference.computeIdentity(packageURL: url)].map { manifest in
                     GraphLoadingNode(manifest: manifest, productFilter: dependency.productFilter)
                 }
@@ -55,7 +55,7 @@ extension PackageGraph {
             manifestMap[PackageReference.computeIdentity(packageURL: $0.url)]
         }))
         let rootManifestNodes = root.manifests.map { GraphLoadingNode(manifest: $0, productFilter: .everything) }
-        let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageGraphRoot.PackageDependency) -> GraphLoadingNode? in
+        let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependencyDescription) -> GraphLoadingNode? in
             guard let manifest = manifestMap[PackageReference.computeIdentity(packageURL: dependency.url)] else { return nil }
             return GraphLoadingNode(manifest: manifest, productFilter: dependency.productFilter)
         }
@@ -223,22 +223,22 @@ private func createResolvedPackages(
         packageBuilder.dependencies = package.manifest.dependenciesRequired(for: packageBuilder.productFilter)
             .compactMap { dependency in
                 // Use the package name to lookup the dependency. The package name will be present in packages with tools version >= 5.2.
-                if let dependencyName = dependency.declaration.explicitName, let resolvedPackage = packageMapByName[dependencyName] {
+                if let dependencyName = dependency.explicitName, let resolvedPackage = packageMapByName[dependencyName] {
                     return resolvedPackage
                 }
 
                 // Otherwise, look it up by its identity.
-                let url = config.mirroredURL(forURL: dependency.declaration.url)
+                let url = config.mirroredURL(forURL: dependency.url)
                 let resolvedPackage = packageMapByIdentity[PackageReference.computeIdentity(packageURL: url)]
 
                 // We check that the explicit package dependency name matches the package name.
                 if let resolvedPackage = resolvedPackage,
-                    let explicitDependencyName = dependency.declaration.explicitName,
-                    resolvedPackage.package.name != dependency.declaration.explicitName
+                    let explicitDependencyName = dependency.explicitName,
+                    resolvedPackage.package.name != dependency.explicitName
                 {
                     let error = PackageGraphError.incorrectPackageDependencyName(
                         dependencyName: explicitDependencyName,
-                        dependencyURL: dependency.declaration.url,
+                        dependencyURL: dependency.url,
                         packageName: resolvedPackage.package.name)
                     let diagnosticLocation = PackageLocation.Local(name: package.name, packagePath: package.path)
                     diagnostics.emit(error, location: diagnosticLocation)

Sources/PackageGraph/PackageGraphRoot.swift

@@ -16,17 +16,14 @@ import SourceControl
 
 /// Represents the input to the package graph root.
 public struct PackageGraphRootInput {
-
-    public typealias PackageDependency = PackageGraphRoot.PackageDependency
-
     /// The list of root packages.
     public let packages: [AbsolutePath]
 
     /// Top level dependencies to the graph.
-    public let dependencies: [PackageDependency]
+    public let dependencies: [PackageDependencyDescription]
 
     /// Create a package graph root.
-    public init(packages: [AbsolutePath], dependencies: [PackageDependency] = []) {
+    public init(packages: [AbsolutePath], dependencies: [PackageDependencyDescription] = []) {
         self.packages = packages
         self.dependencies = dependencies
     }
@@ -35,67 +32,14 @@ public struct PackageGraphRootInput {
 /// Represents the inputs to the package graph.
 public struct PackageGraphRoot {
 
-    // FIXME: We can kill this now.
-    //
-    /// Represents a top level package dependencies.
-    public struct PackageDependency {
-
-        public typealias Requirement = PackageModel.PackageDependencyDescription.Requirement
-
-        // Location of this dependency.
-        //
-        // Opaque location object which will be included in any diagnostics
-        // related to this dependency.  Clients can use this identify where this
-        // dependency is declared.
-        public let location: String
-
-        /// The URL of the package.
-        public let url: String
-
-        /// The requirement of the package.
-        public let requirement: Requirement
-
-        /// The product filter to apply to the package.
-        public let productFilter: ProductFilter
-
-        /// Create the package reference object for the dependency.
-        public func createPackageRef(config: SwiftPMConfig) -> PackageReference {
-            let effectiveURL = config.mirroredURL(forURL: self.url)
-            return PackageReference(
-                identity: PackageReference.computeIdentity(packageURL: effectiveURL),
-                path: effectiveURL,
-                kind: requirement == .localPackage ? .local : .remote
-            )
-        }
-
-        public init(
-            url: String,
-            requirement: Requirement,
-            productFilter: ProductFilter,
-            location: String
-        ) {
-            // FIXME: SwiftPM can't handle file URLs with file:// scheme so we need to
-            // strip that. We need to design a URL data structure for SwiftPM.
-            let filePrefix = "file://"
-            if url.hasPrefix(filePrefix) {
-                self.url = AbsolutePath(String(url.dropFirst(filePrefix.count))).pathString
-            } else {
-                self.url = url
-            }
-            self.requirement = requirement
-            self.productFilter = productFilter
-            self.location = location
-        }
-    }
-
     /// The list of root manifests.
     public let manifests: [Manifest]
 
     /// The root package references.
     public let packageRefs: [PackageReference]
 
     /// The top level dependencies.
-    public let dependencies: [PackageDependency]
+    public let dependencies: [PackageDependencyDescription]
 
     /// Create a package graph root.
     public init(input: PackageGraphRootInput, manifests: [Manifest], explicitProduct: String? = nil) {
@@ -114,13 +58,8 @@ public struct PackageGraphRoot {
         // at which time the current special casing can be deprecated.
         var adjustedDependencies = input.dependencies
         if let product = explicitProduct {
-            for dependency in manifests.lazy
-                .map({ $0.dependenciesRequired(for: .everything) }).joined() {
-                    adjustedDependencies.append(PackageDependency(
-                        url: dependency.declaration.url,
-                        requirement: dependency.declaration.requirement,
-                        productFilter: .specific([product]),
-                        location: ""))
+            for dependency in manifests.lazy.map({ $0.dependenciesRequired(for: .everything) }).joined() {
+                adjustedDependencies.append(dependency.filtered(by: .specific([product])))
             }
         }
 

Sources/PackageGraph/PackageModel+Extensions.swift

@@ -14,9 +14,17 @@ import SourceControl
 extension PackageDependencyDescription {
     /// Create the package reference object for the dependency.
     public func createPackageRef(config: SwiftPMConfig) -> PackageReference {
-        let effectiveURL = config.mirroredURL(forURL: self.url)
+        let effectiveURL = config.mirroredURL(forURL: url)
+
+        // FIXME: The identity of a package dependency is currently based on
+        //        the explicit name provided in the package manifest, if provided,
+        //        falling back on a name computed from its effective URL.
+        //        We should instead use the declared URL of a package dependency as its identity,
+        //        as it will be necessary for supporting package registries.
+        let identity = explicitName?.lowercased() ?? PackageReference.computeIdentity(packageURL: effectiveURL)
+        
         return PackageReference(
-            identity: PackageReference.computeIdentity(packageURL: effectiveURL),
+            identity: identity,
             path: effectiveURL,
             kind: requirement == .localPackage ? .local : .remote
         )
@@ -28,8 +36,8 @@ extension Manifest {
     public func dependencyConstraints(productFilter: ProductFilter, config: SwiftPMConfig) -> [RepositoryPackageConstraint] {
         return dependenciesRequired(for: productFilter).map({
             return RepositoryPackageConstraint(
-                container: $0.declaration.createPackageRef(config: config),
-                requirement: $0.declaration.requirement.toConstraintRequirement(),
+                container: $0.createPackageRef(config: config),
+                requirement: $0.requirement.toConstraintRequirement(),
                 products: $0.productFilter)
         })
     }

Sources/PackageModel/CMakeLists.txt

@@ -12,7 +12,6 @@ add_library(PackageModel
   BuildSettings.swift
   Diagnostics.swift
   Manifest.swift
-  Manifest/FilteredDependencyDescription.swift
   Manifest/PackageConditionDescription.swift
   Manifest/PackageDependencyDescription.swift
   Manifest/PlatformDescription.swift
@@ -27,9 +26,9 @@ add_library(PackageModel
   Product.swift
   Resource.swift
   Sources.swift
-  Target.swift
   SupportedLanguageExtension.swift
   SwiftLanguageVersion.swift
+  Target.swift
   ToolsVersion.swift)
 target_link_libraries(PackageModel PUBLIC
   TSCBasic

Sources/PackageModel/Manifest.swift

@@ -86,7 +86,7 @@ public final class Manifest: ObjectIdentifierProtocol {
     private var _requiredTargets: [ProductFilter: [TargetDescription]]
 
     /// Dependencies required for building particular product filters.
-    private var _requiredDependencies: [ProductFilter: [FilteredDependencyDescription]]
+    private var _requiredDependencies: [ProductFilter: [PackageDependencyDescription]]
 
     public init(
         name: String,
@@ -154,7 +154,7 @@ public final class Manifest: ObjectIdentifierProtocol {
     }
 
     /// Returns the package dependencies required for a particular products filter.
-    public func dependenciesRequired(for productFilter: ProductFilter) -> [FilteredDependencyDescription] {
+    public func dependenciesRequired(for productFilter: ProductFilter) -> [PackageDependencyDescription] {
         // If we have already calcualted it, returned the cached value.
         if let dependencies = _requiredDependencies[productFilter] {
             return dependencies
@@ -194,7 +194,7 @@ public final class Manifest: ObjectIdentifierProtocol {
     public func dependenciesRequired(
         for targets: [TargetDescription],
         keepUnused: Bool = false
-    ) -> [FilteredDependencyDescription] {
+    ) -> [PackageDependencyDescription] {
 
         var registry: (known: [String: ProductFilter], unknown: Set<String>) = ([:], [])
         let availablePackages = Set(dependencies.lazy.map({ $0.name }))
@@ -211,23 +211,23 @@ public final class Manifest: ObjectIdentifierProtocol {
         let unknown = registry.unknown
         if !registry.unknown.isEmpty {
             for package in availablePackages {
-                associations[package, default: .specific([])].formUnion(.specific(unknown))
+                associations[package, default: .nothing].formUnion(.specific(unknown))
             }
         }
 
         return dependencies.compactMap { dependency in
             #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             if let filter = associations[dependency.name] {
-                return FilteredDependencyDescription(declaration: dependency, productFilter: filter)
+                return dependency.filtered(by: filter)
             } else if keepUnused {
                 // Register that while the dependency was kept, no products are needed.
-                return FilteredDependencyDescription(declaration: dependency, productFilter: .specific([]))
+                return dependency.filtered(by: .nothing)
             } else {
                 // Dependencies known to not have any relevant products are discarded.
                 return nil
             }
             #else
-            return FilteredDependencyDescription(declaration: dependency, productFilter: .everything)
+            return dependency.filtered(by: .everything)
             #endif
         }
     }

Sources/PackageModel/Manifest/FilteredDependencyDescription.swift

@@ -30,24 +30,53 @@ public struct PackageDependencyDescription: Equatable, Codable {
         }
     }
 
-    /// The name of the dependency explicitly defined in the manifest.
+    /// The name of the package dependency explicitly defined in the manifest, if any.
     public let explicitName: String?
 
-    /// The name of the dependency, either explicitly defined in the manifest, or deduced from the URL.
+    /// The name of the packagedependency,
+    /// either explicitly defined in the manifest,
+    /// or derived from the URL.
+    ///
+    /// - SeeAlso: `explicitName`
+    /// - SeeAlso: `url`
     public let name: String
 
-    /// The url of the dependency.
+    /// The url of the package dependency.
     public let url: String
 
     /// The dependency requirement.
     public let requirement: Requirement
 
-    /// Create a dependency.
-    public init(name: String?, url: String, requirement: Requirement) {
+    /// The products requested of the package dependency.
+    public let productFilter: ProductFilter
+
+    /// Create a package dependency.
+    public init(
+        name: String? = nil,
+        url: String,
+        requirement: Requirement,
+        productFilter: ProductFilter = .everything
+    ) {
+        // FIXME: SwiftPM can't handle file URLs with file:// scheme so we need to
+        // strip that. We need to design a URL data structure for SwiftPM.
+        let filePrefix = "file://"
+        let normalizedURL: String
+        if url.hasPrefix(filePrefix) {
+            normalizedURL = AbsolutePath(String(url.dropFirst(filePrefix.count))).pathString
+        } else {
+            normalizedURL = url
+        }
+
         self.explicitName = name
-        self.name = name ?? PackageReference.computeDefaultName(fromURL: url)
-        self.url = url
+        self.name = name ?? PackageReference.computeDefaultName(fromURL: normalizedURL)
+        self.url = normalizedURL
         self.requirement = requirement
+        self.productFilter = productFilter
+    }
+
+    /// Returns a new package dependency with the specified products.
+    public func filtered(by productFilter: ProductFilter) -> PackageDependencyDescription {
+        PackageDependencyDescription(name: explicitName, url: url, requirement: requirement, productFilter: productFilter)
     }
 }
 

Sources/PackageModel/Manifest/PackageDependencyDescription.swift

@@ -89,6 +89,9 @@ public enum ProductFilter: Equatable, Hashable {
     /// A set of specific products requested by one or more client packages.
     case specific(Set<String>)
 
+    /// No products, targets, or tests are requested.
+    public static var nothing: ProductFilter { .specific([]) }
+
     public func union(_ other: ProductFilter) -> ProductFilter {
         switch self {
         case .everything:

Sources/PackageModel/Product.swift

@@ -188,7 +188,7 @@ public final class TestWorkspace {
     }
 
     public struct PackageDependency {
-        public typealias Requirement = PackageGraphRoot.PackageDependency.Requirement
+        public typealias Requirement = PackageDependencyDescription.Requirement
 
         public let name: String
         public let requirement: Requirement
@@ -200,12 +200,11 @@ public final class TestWorkspace {
             self.products = products
         }
 
-        fileprivate func convert(_ packagesDir: AbsolutePath, url: String) -> PackageGraphRootInput.PackageDependency {
-            return PackageGraphRootInput.PackageDependency(
+        fileprivate func convert(_ packagesDir: AbsolutePath, url: String) -> PackageDependencyDescription {
+            return PackageDependencyDescription(
                 url: url,
                 requirement: requirement,
-                productFilter: products,
-                location: name
+                productFilter: products
             )
         }
     }
@@ -306,7 +305,7 @@ public final class TestWorkspace {
 
     public func checkPackageGraph(
         roots: [String] = [],
-        dependencies: [PackageGraphRootInput.PackageDependency] = [],
+        dependencies: [PackageDependencyDescription] = [],
         forceResolvedVersions: Bool = false,
         _ result: (PackageGraph, DiagnosticsEngine) -> ()
     ) {