From 7430175b04dc0080c8e94b51f87f6d6cc637e42e Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 22 Nov 2023 16:18:08 +0000 Subject: [PATCH 01/24] Filter out duplicate package manifest conditions Multiple places in our codebase pass around and store `[any PackageConditionProtocol]` arrays of existentials with `FIXME` notes that those should be sets and not arrays. Adding `Hashable` requirement to `PackageConditionProtocol` doesn't fix the issue, since existentials of this protocol still wouldn't conform to `Hashable`. A simple alternative is to create `enum PackageCondition` with cases for each condition type. We only have two of those types and they're always declared in the same package. There's no use case for externally defined types for this protocol. That allows us to convert uses of `[any PackageConditionProtocol]` to `Set`. Existing protocol kept around until clients of SwiftPM can migrate to the new `PackageCondition` enum. --- .../PackageGraph/PackageGraph+Loading.swift | 9 ++- .../Resolution/ResolvedTarget.swift | 8 +-- Sources/PackageLoading/PackageBuilder.swift | 14 ++--- Sources/PackageModel/BuildSettings.swift | 19 ++++-- .../PackageConditionDescription.swift | 63 ++++++++++++++++++- Sources/PackageModel/Target/Target.swift | 6 +- .../Plugins/PluginInvocation.swift | 2 +- Sources/XCBuildSupport/PIF.swift | 3 +- Sources/XCBuildSupport/PIFBuilder.swift | 12 ++-- .../PackageBuilderTests.swift | 2 +- 10 files changed, 105 insertions(+), 33 deletions(-) diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index c70c128216d..d0015d6e385 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -869,10 +869,10 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { enum Dependency { /// Dependency to another target, with conditions. - case target(_ target: ResolvedTargetBuilder, conditions: [PackageConditionProtocol]) + case target(_ target: ResolvedTargetBuilder, conditions: Set) /// Dependency to a product, with conditions. - case product(_ product: ResolvedProductBuilder, conditions: [PackageConditionProtocol]) + case product(_ product: ResolvedProductBuilder, conditions: Set) } /// The target reference. @@ -918,7 +918,10 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { try self.target.validateDependency(target: targetBuilder.target) return .target(try targetBuilder.construct(), conditions: conditions) case .product(let productBuilder, let conditions): - try self.target.validateDependency(product: productBuilder.product, productPackage: productBuilder.packageBuilder.package.identity) + try self.target.validateDependency( + product: productBuilder.product, + productPackage: productBuilder.packageBuilder.package.identity + ) let product = try productBuilder.construct() if !productBuilder.packageBuilder.isAllowedToVendUnsafeProducts { try self.diagnoseInvalidUseOfUnsafeFlags(product) diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 67824c73d97..2753d32ca47 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -19,10 +19,10 @@ public final class ResolvedTarget { /// Represents dependency of a resolved target. public enum Dependency { /// Direct dependency of the target. This target is in the same package and should be statically linked. - case target(_ target: ResolvedTarget, conditions: [PackageConditionProtocol]) + case target(_ target: ResolvedTarget, conditions: Set) /// The target depends on this product. - case product(_ product: ResolvedProduct, conditions: [PackageConditionProtocol]) + case product(_ product: ResolvedProduct, conditions: Set) public var target: ResolvedTarget? { switch self { @@ -38,7 +38,7 @@ public final class ResolvedTarget { } } - public var conditions: [PackageConditionProtocol] { + public var conditions: Set { switch self { case .target(_, let conditions): return conditions case .product(_, let conditions): return conditions @@ -141,7 +141,7 @@ public final class ResolvedTarget { /// The list of platforms that are supported by this target. public let platforms: SupportedPlatforms - /// Create a target instance. + /// Create a resolved target instance. public init( target: Target, dependencies: [Dependency], diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index 8f3aec97666..9289adb6a93 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -697,7 +697,7 @@ public final class PackageBuilder { // The created targets mapped to their name. var targets = [String: Target]() - // If a direcotry is empty, we don't create a target object for them. + // If a directory is empty, we don't create a target object for them. var emptyModules = Set() // Start iterating the potential targets. @@ -710,7 +710,7 @@ public final class PackageBuilder { // Get the dependencies of this target. let dependencies: [Target.Dependency] = try manifestTarget.map { - try $0.dependencies.compactMap { dependency in + try $0.dependencies.compactMap { dependency -> Target.Dependency? in switch dependency { case .target(let name, let condition): // We don't create an object for targets which have no sources. @@ -1136,7 +1136,7 @@ public final class PackageBuilder { // Create an assignment for this setting. var assignment = BuildSettings.Assignment() assignment.values = values - assignment.conditions = self.buildConditions(from: setting.condition) + assignment.uniqueConditions = self.buildConditions(from: setting.condition) // Finally, add the assignment to the assignment table. table.add(assignment, for: decl) @@ -1145,12 +1145,12 @@ public final class PackageBuilder { return table } - func buildConditions(from condition: PackageConditionDescription?) -> [PackageConditionProtocol] { - var conditions: [PackageConditionProtocol] = [] + func buildConditions(from condition: PackageConditionDescription?) -> Set { + var conditions: Set = [] if let config = condition?.config.flatMap({ BuildConfiguration(rawValue: $0) }) { let condition = ConfigurationCondition(configuration: config) - conditions.append(condition) + conditions.insert(.configuration(condition)) } if let platforms = condition?.platformNames.map({ @@ -1163,7 +1163,7 @@ public final class PackageBuilder { !platforms.isEmpty { let condition = PlatformsCondition(platforms: platforms) - conditions.append(condition) + conditions.insert(.platforms(condition)) } return conditions diff --git a/Sources/PackageModel/BuildSettings.swift b/Sources/PackageModel/BuildSettings.swift index 64c56531d55..079ee6067fa 100644 --- a/Sources/PackageModel/BuildSettings.swift +++ b/Sources/PackageModel/BuildSettings.swift @@ -43,8 +43,8 @@ public enum BuildSettings { /// The assignment value. public var values: [String] - // FIXME: This should be a set but we need Equatable existential (or AnyEquatable) for that. /// The condition associated with this assignment. + @available(*, deprecated, renamed: "uniqueConditions") public var conditions: [PackageConditionProtocol] { get { return _conditions.map{ $0.condition } @@ -52,6 +52,16 @@ public enum BuildSettings { set { _conditions = newValue.map{ PackageConditionWrapper($0) } } + } + + /// A set of unique conditions associated with this assignment. + public var uniqueConditions: Set { + get { + return Set(_conditions.map(\.underlying)) + } + set { + _conditions = newValue.map { PackageConditionWrapper($0) } + } } private var _conditions: [PackageConditionWrapper] @@ -64,7 +74,7 @@ public enum BuildSettings { /// Build setting assignment table which maps a build setting to a list of assignments. public struct AssignmentTable: Codable { - public private(set) var assignments: [Declaration: [Assignment]] + public private(set) var assignments: [Declaration: Set] public init() { assignments = [:] @@ -72,8 +82,7 @@ public enum BuildSettings { /// Add the given assignment to the table. mutating public func add(_ assignment: Assignment, for decl: Declaration) { - // FIXME: We should check for duplicate assignments. - assignments[decl, default: []].append(assignment) + assignments[decl, default: []].insert(assignment) } } @@ -102,7 +111,7 @@ public enum BuildSettings { // Add values from each assignment if it satisfies the build environment. let values = assignments .lazy - .filter { $0.conditions.allSatisfy { $0.satisfies(self.environment) } } + .filter { $0.uniqueConditions.allSatisfy { $0.satisfies(self.environment) } } .flatMap { $0.values } return Array(values) diff --git a/Sources/PackageModel/Manifest/PackageConditionDescription.swift b/Sources/PackageModel/Manifest/PackageConditionDescription.swift index 7e1700443b6..45f2de6dadd 100644 --- a/Sources/PackageModel/Manifest/PackageConditionDescription.swift +++ b/Sources/PackageModel/Manifest/PackageConditionDescription.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2020 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information @@ -27,11 +27,12 @@ public protocol PackageConditionProtocol: Codable { func satisfies(_ environment: BuildEnvironment) -> Bool } -/// Wrapper for package condition so it can be conformed to Codable. +/// Wrapper for package condition so it can conform to Codable. struct PackageConditionWrapper: Codable, Equatable, Hashable { var platform: PlatformsCondition? var config: ConfigurationCondition? + @available(*, deprecated, renamed: "underlying") var condition: PackageConditionProtocol { if let platform { return platform @@ -42,6 +43,17 @@ struct PackageConditionWrapper: Codable, Equatable, Hashable { } } + var underlying: PackageCondition { + if let platform { + return .platforms(platform) + } else if let config { + return .configuration(config) + } else { + fatalError("unreachable") + } + } + + @available(*, deprecated, message: "Use .init(_ condition: PackageCondition) instead") init(_ condition: PackageConditionProtocol) { switch condition { case let platform as PlatformsCondition: @@ -52,6 +64,53 @@ struct PackageConditionWrapper: Codable, Equatable, Hashable { fatalError("unknown condition \(condition)") } } + + init(_ condition: PackageCondition) { + switch condition { + case let .platforms(platforms): + self.platform = platforms + case let .configuration(configuration): + self.config = configuration + } + } +} + +public enum PackageCondition: Hashable { + case platforms(PlatformsCondition) + case configuration(ConfigurationCondition) + + public func satisfies(_ environment: BuildEnvironment) -> Bool { + switch self { + case .configuration(let configuration): + return configuration.satisfies(environment) + case .platforms(let platforms): + return platforms.satisfies(environment) + } + } + + public var platformsCondition: PlatformsCondition? { + guard case let .platforms(platformsCondition) = self else { + return nil + } + + return platformsCondition + } + + public var configurationCondition: ConfigurationCondition? { + guard case let .configuration(configurationCondition) = self else { + return nil + } + + return configurationCondition + } + + public init(platforms: [Platform]) { + self = .platforms(.init(platforms: platforms)) + } + + public init(configuration: BuildConfiguration) { + self = .configuration(.init(configuration: configuration)) + } } /// Platforms condition implies that an assignment is valid on these platforms. diff --git a/Sources/PackageModel/Target/Target.swift b/Sources/PackageModel/Target/Target.swift index 8464ea896d0..e3bdb533826 100644 --- a/Sources/PackageModel/Target/Target.swift +++ b/Sources/PackageModel/Target/Target.swift @@ -79,10 +79,10 @@ public class Target: PolymorphicCodableProtocol { /// A target dependency to a target or product. public enum Dependency { /// A dependency referencing another target, with conditions. - case target(_ target: Target, conditions: [PackageConditionProtocol]) + case target(_ target: Target, conditions: Set) /// A dependency referencing a product, with conditions. - case product(_ product: ProductReference, conditions: [PackageConditionProtocol]) + case product(_ product: ProductReference, conditions: Set) /// The target if the dependency is a target dependency. public var target: Target? { @@ -103,7 +103,7 @@ public class Target: PolymorphicCodableProtocol { } /// The dependency conditions. - public var conditions: [PackageConditionProtocol] { + public var conditions: Set { switch self { case .target(_, let conditions): return conditions diff --git a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift index 2b0829a36ae..eb2c67e0dde 100644 --- a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift +++ b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift @@ -584,7 +584,7 @@ public extension PluginTarget { } fileprivate extension Target.Dependency { - var conditions: [PackageConditionProtocol] { + var conditions: Set { switch self { case .target(_, let conditions): return conditions case .product(_, let conditions): return conditions diff --git a/Sources/XCBuildSupport/PIF.swift b/Sources/XCBuildSupport/PIF.swift index d4fc74d8897..47b4112b914 100644 --- a/Sources/XCBuildSupport/PIF.swift +++ b/Sources/XCBuildSupport/PIF.swift @@ -1002,7 +1002,8 @@ public enum PIF { } public var conditions: [String] { - let filters = [PlatformsCondition(platforms: [packageModelPlatform])].toPlatformFilters().map { (filter: PIF.PlatformFilter) -> String in + let filters = Set([.init(platforms: [packageModelPlatform])]) + .toPlatformFilters().map { (filter: PIF.PlatformFilter) -> String in if filter.environment.isEmpty { return filter.platform } else { diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index 6dddf351b21..bc9a28c7d5e 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -781,7 +781,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { private func addDependency( to target: ResolvedTarget, in pifTarget: PIFTargetBuilder, - conditions: [PackageConditionProtocol], + conditions: Set, linkProduct: Bool ) { // Only add the binary target as a library when we want to link against the product. @@ -803,7 +803,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { private func addDependency( to product: ResolvedProduct, in pifTarget: PIFTargetBuilder, - conditions: [PackageConditionProtocol], + conditions: Set, linkProduct: Bool ) { pifTarget.addDependency( @@ -1498,7 +1498,7 @@ private extension BuildSettings.AssignmentTable { private extension BuildSettings.Assignment { var configurations: [BuildConfiguration] { - if let configurationCondition = conditions.lazy.compactMap({ $0 as? ConfigurationCondition }).first { + if let configurationCondition = uniqueConditions.lazy.compactMap(\.configurationCondition).first { return [configurationCondition.configuration] } else { return BuildConfiguration.allCases @@ -1506,7 +1506,7 @@ private extension BuildSettings.Assignment { } var pifPlatforms: [PIF.BuildSettings.Platform]? { - if let platformsCondition = conditions.lazy.compactMap({ $0 as? PlatformsCondition }).first { + if let platformsCondition = uniqueConditions.lazy.compactMap(\.platformsCondition).first { return platformsCondition.platforms.compactMap { PIF.BuildSettings.Platform(rawValue: $0.name) } } else { return nil @@ -1537,10 +1537,10 @@ public struct DelayedImmutable { } } -extension Array where Element == PackageConditionProtocol { +extension Set { func toPlatformFilters() -> [PIF.PlatformFilter] { var result: [PIF.PlatformFilter] = [] - let platformConditions = self.compactMap{ $0 as? PlatformsCondition }.flatMap{ $0.platforms } + let platformConditions = self.compactMap(\.platformsCondition).flatMap { $0.platforms } for condition in platformConditions { switch condition { diff --git a/Tests/PackageLoadingTests/PackageBuilderTests.swift b/Tests/PackageLoadingTests/PackageBuilderTests.swift index c615f9f28ad..c2f7d0d9ddc 100644 --- a/Tests/PackageLoadingTests/PackageBuilderTests.swift +++ b/Tests/PackageLoadingTests/PackageBuilderTests.swift @@ -2990,7 +2990,7 @@ class PackageBuilderTests: XCTestCase { var assignment = BuildSettings.Assignment() assignment.values = ["YOLO"] - assignment.conditions = [PlatformsCondition(platforms: [PackageModel.Platform.custom(name: "bestOS", oldestSupportedVersion: .unknown)])] + assignment.uniqueConditions = Set([.init(platforms: [PackageModel.Platform.custom(name: "bestOS", oldestSupportedVersion: .unknown)])]) var settings = BuildSettings.AssignmentTable() settings.add(assignment, for: .SWIFT_ACTIVE_COMPILATION_CONDITIONS) From 6fd1606b1688af8ef35a476d9263eb98730f506d Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 22 Nov 2023 16:20:27 +0000 Subject: [PATCH 02/24] Add doc comment to the new type --- Sources/PackageModel/Manifest/PackageConditionDescription.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/PackageModel/Manifest/PackageConditionDescription.swift b/Sources/PackageModel/Manifest/PackageConditionDescription.swift index 45f2de6dadd..a8fef073cba 100644 --- a/Sources/PackageModel/Manifest/PackageConditionDescription.swift +++ b/Sources/PackageModel/Manifest/PackageConditionDescription.swift @@ -75,6 +75,8 @@ struct PackageConditionWrapper: Codable, Equatable, Hashable { } } +/// One of possible conditions used in package manifests to restrict targets from being built for certain platforms or +/// build configurations. public enum PackageCondition: Hashable { case platforms(PlatformsCondition) case configuration(ConfigurationCondition) From 456959ed3ffe8854f2235b10b428d1d1fa66c101 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 22 Nov 2023 16:40:16 +0000 Subject: [PATCH 03/24] Fix tests regression --- Sources/PackageModel/BuildSettings.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/PackageModel/BuildSettings.swift b/Sources/PackageModel/BuildSettings.swift index 079ee6067fa..d493b0f0b5c 100644 --- a/Sources/PackageModel/BuildSettings.swift +++ b/Sources/PackageModel/BuildSettings.swift @@ -74,7 +74,7 @@ public enum BuildSettings { /// Build setting assignment table which maps a build setting to a list of assignments. public struct AssignmentTable: Codable { - public private(set) var assignments: [Declaration: Set] + public private(set) var assignments: [Declaration: [Assignment]] public init() { assignments = [:] @@ -82,7 +82,7 @@ public enum BuildSettings { /// Add the given assignment to the table. mutating public func add(_ assignment: Assignment, for decl: Declaration) { - assignments[decl, default: []].insert(assignment) + assignments[decl, default: []].append(assignment) } } From 420f01ac39e5de93cdc55d50e6435c9bd21fe3ae Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 22 Nov 2023 16:41:26 +0000 Subject: [PATCH 04/24] Remove custom `ResolvedTarget.Dependency` conformance --- .../Resolution/ResolvedTarget.swift | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 2753d32ca47..706576c99e4 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -17,7 +17,7 @@ import func TSCBasic.topologicalSort /// Represents a fully resolved target. All the dependencies for the target are resolved. public final class ResolvedTarget { /// Represents dependency of a resolved target. - public enum Dependency { + public enum Dependency: Hashable { /// Direct dependency of the target. This target is in the same package and should be statically linked. case target(_ target: ResolvedTarget, conditions: Set) @@ -171,30 +171,6 @@ extension ResolvedTarget: CustomStringConvertible { } } -extension ResolvedTarget.Dependency: Equatable { - public static func == (lhs: ResolvedTarget.Dependency, rhs: ResolvedTarget.Dependency) -> Bool { - switch (lhs, rhs) { - case (.target(let lhsTarget, _), .target(let rhsTarget, _)): - return lhsTarget == rhsTarget - case (.product(let lhsProduct, _), .product(let rhsProduct, _)): - return lhsProduct == rhsProduct - case (.product, .target), (.target, .product): - return false - } - } -} - -extension ResolvedTarget.Dependency: Hashable { - public func hash(into hasher: inout Hasher) { - switch self { - case .target(let target, _): - hasher.combine(target) - case .product(let product, _): - hasher.combine(product) - } - } -} - extension ResolvedTarget.Dependency: CustomStringConvertible { public var description: String { var str = " Date: Sat, 2 Dec 2023 16:16:05 +0000 Subject: [PATCH 05/24] Roll back to arrays and reduce the diff --- Sources/PackageGraph/PackageGraph+Loading.swift | 4 ++-- .../Resolution/ResolvedTarget.swift | 6 +++--- Sources/PackageLoading/PackageBuilder.swift | 10 +++++----- Sources/PackageModel/BuildSettings.swift | 17 +++-------------- Sources/PackageModel/Target/Target.swift | 6 +++--- .../SPMBuildCore/Plugins/PluginInvocation.swift | 2 +- Sources/XCBuildSupport/PIF.swift | 2 +- Sources/XCBuildSupport/PIFBuilder.swift | 10 +++++----- .../PackageBuilderTests.swift | 2 +- 9 files changed, 24 insertions(+), 35 deletions(-) diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index d0015d6e385..5fe6b21ec30 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -869,10 +869,10 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { enum Dependency { /// Dependency to another target, with conditions. - case target(_ target: ResolvedTargetBuilder, conditions: Set) + case target(_ target: ResolvedTargetBuilder, conditions: [PackageCondition]) /// Dependency to a product, with conditions. - case product(_ product: ResolvedProductBuilder, conditions: Set) + case product(_ product: ResolvedProductBuilder, conditions: [PackageCondition]) } /// The target reference. diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 706576c99e4..0011bcff501 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -19,10 +19,10 @@ public final class ResolvedTarget { /// Represents dependency of a resolved target. public enum Dependency: Hashable { /// Direct dependency of the target. This target is in the same package and should be statically linked. - case target(_ target: ResolvedTarget, conditions: Set) + case target(_ target: ResolvedTarget, conditions: [PackageCondition]) /// The target depends on this product. - case product(_ product: ResolvedProduct, conditions: Set) + case product(_ product: ResolvedProduct, conditions: [PackageCondition]) public var target: ResolvedTarget? { switch self { @@ -38,7 +38,7 @@ public final class ResolvedTarget { } } - public var conditions: Set { + public var conditions: [PackageCondition] { switch self { case .target(_, let conditions): return conditions case .product(_, let conditions): return conditions diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index 9289adb6a93..26b8729a846 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -1136,7 +1136,7 @@ public final class PackageBuilder { // Create an assignment for this setting. var assignment = BuildSettings.Assignment() assignment.values = values - assignment.uniqueConditions = self.buildConditions(from: setting.condition) + assignment.conditions = self.buildConditions(from: setting.condition) // Finally, add the assignment to the assignment table. table.add(assignment, for: decl) @@ -1145,12 +1145,12 @@ public final class PackageBuilder { return table } - func buildConditions(from condition: PackageConditionDescription?) -> Set { - var conditions: Set = [] + func buildConditions(from condition: PackageConditionDescription?) -> [PackageCondition] { + var conditions = [PackageCondition]() if let config = condition?.config.flatMap({ BuildConfiguration(rawValue: $0) }) { let condition = ConfigurationCondition(configuration: config) - conditions.insert(.configuration(condition)) + conditions.append(.configuration(condition)) } if let platforms = condition?.platformNames.map({ @@ -1163,7 +1163,7 @@ public final class PackageBuilder { !platforms.isEmpty { let condition = PlatformsCondition(platforms: platforms) - conditions.insert(.platforms(condition)) + conditions.append(.platforms(condition)) } return conditions diff --git a/Sources/PackageModel/BuildSettings.swift b/Sources/PackageModel/BuildSettings.swift index d493b0f0b5c..d951cc92788 100644 --- a/Sources/PackageModel/BuildSettings.swift +++ b/Sources/PackageModel/BuildSettings.swift @@ -44,20 +44,9 @@ public enum BuildSettings { public var values: [String] /// The condition associated with this assignment. - @available(*, deprecated, renamed: "uniqueConditions") - public var conditions: [PackageConditionProtocol] { + public var conditions: [PackageCondition] { get { - return _conditions.map{ $0.condition } - } - set { - _conditions = newValue.map{ PackageConditionWrapper($0) } - } - } - - /// A set of unique conditions associated with this assignment. - public var uniqueConditions: Set { - get { - return Set(_conditions.map(\.underlying)) + return _conditions.map { $0.underlying } } set { _conditions = newValue.map { PackageConditionWrapper($0) } @@ -111,7 +100,7 @@ public enum BuildSettings { // Add values from each assignment if it satisfies the build environment. let values = assignments .lazy - .filter { $0.uniqueConditions.allSatisfy { $0.satisfies(self.environment) } } + .filter { $0.conditions.allSatisfy { $0.satisfies(self.environment) } } .flatMap { $0.values } return Array(values) diff --git a/Sources/PackageModel/Target/Target.swift b/Sources/PackageModel/Target/Target.swift index e3bdb533826..faf1bb5cda0 100644 --- a/Sources/PackageModel/Target/Target.swift +++ b/Sources/PackageModel/Target/Target.swift @@ -79,10 +79,10 @@ public class Target: PolymorphicCodableProtocol { /// A target dependency to a target or product. public enum Dependency { /// A dependency referencing another target, with conditions. - case target(_ target: Target, conditions: Set) + case target(_ target: Target, conditions: [PackageCondition]) /// A dependency referencing a product, with conditions. - case product(_ product: ProductReference, conditions: Set) + case product(_ product: ProductReference, conditions: [PackageCondition]) /// The target if the dependency is a target dependency. public var target: Target? { @@ -103,7 +103,7 @@ public class Target: PolymorphicCodableProtocol { } /// The dependency conditions. - public var conditions: Set { + public var conditions: [PackageCondition] { switch self { case .target(_, let conditions): return conditions diff --git a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift index eb2c67e0dde..2a8544acd31 100644 --- a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift +++ b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift @@ -584,7 +584,7 @@ public extension PluginTarget { } fileprivate extension Target.Dependency { - var conditions: Set { + var conditions: [PackageCondition] { switch self { case .target(_, let conditions): return conditions case .product(_, let conditions): return conditions diff --git a/Sources/XCBuildSupport/PIF.swift b/Sources/XCBuildSupport/PIF.swift index 47b4112b914..69df9409de3 100644 --- a/Sources/XCBuildSupport/PIF.swift +++ b/Sources/XCBuildSupport/PIF.swift @@ -1002,7 +1002,7 @@ public enum PIF { } public var conditions: [String] { - let filters = Set([.init(platforms: [packageModelPlatform])]) + let filters = [.init(platforms: [packageModelPlatform])] .toPlatformFilters().map { (filter: PIF.PlatformFilter) -> String in if filter.environment.isEmpty { return filter.platform diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index bc9a28c7d5e..fe8e14f7671 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -781,7 +781,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { private func addDependency( to target: ResolvedTarget, in pifTarget: PIFTargetBuilder, - conditions: Set, + conditions: [PackageCondition], linkProduct: Bool ) { // Only add the binary target as a library when we want to link against the product. @@ -803,7 +803,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { private func addDependency( to product: ResolvedProduct, in pifTarget: PIFTargetBuilder, - conditions: Set, + conditions: [PackageCondition], linkProduct: Bool ) { pifTarget.addDependency( @@ -1498,7 +1498,7 @@ private extension BuildSettings.AssignmentTable { private extension BuildSettings.Assignment { var configurations: [BuildConfiguration] { - if let configurationCondition = uniqueConditions.lazy.compactMap(\.configurationCondition).first { + if let configurationCondition = conditions.lazy.compactMap(\.configurationCondition).first { return [configurationCondition.configuration] } else { return BuildConfiguration.allCases @@ -1506,7 +1506,7 @@ private extension BuildSettings.Assignment { } var pifPlatforms: [PIF.BuildSettings.Platform]? { - if let platformsCondition = uniqueConditions.lazy.compactMap(\.platformsCondition).first { + if let platformsCondition = conditions.lazy.compactMap(\.platformsCondition).first { return platformsCondition.platforms.compactMap { PIF.BuildSettings.Platform(rawValue: $0.name) } } else { return nil @@ -1537,7 +1537,7 @@ public struct DelayedImmutable { } } -extension Set { +extension [PackageCondition] { func toPlatformFilters() -> [PIF.PlatformFilter] { var result: [PIF.PlatformFilter] = [] let platformConditions = self.compactMap(\.platformsCondition).flatMap { $0.platforms } diff --git a/Tests/PackageLoadingTests/PackageBuilderTests.swift b/Tests/PackageLoadingTests/PackageBuilderTests.swift index c2f7d0d9ddc..26268ba2463 100644 --- a/Tests/PackageLoadingTests/PackageBuilderTests.swift +++ b/Tests/PackageLoadingTests/PackageBuilderTests.swift @@ -2990,7 +2990,7 @@ class PackageBuilderTests: XCTestCase { var assignment = BuildSettings.Assignment() assignment.values = ["YOLO"] - assignment.uniqueConditions = Set([.init(platforms: [PackageModel.Platform.custom(name: "bestOS", oldestSupportedVersion: .unknown)])]) + assignment.conditions = [.init(platforms: [PackageModel.Platform.custom(name: "bestOS", oldestSupportedVersion: .unknown)])] var settings = BuildSettings.AssignmentTable() settings.add(assignment, for: .SWIFT_ACTIVE_COMPILATION_CONDITIONS) From 4bd4bb47e7082493d41c297ed4e3365e8ff1e08d Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 16:20:13 +0000 Subject: [PATCH 06/24] Update comments and reduce the diff --- Sources/PackageGraph/Resolution/ResolvedTarget.swift | 2 +- Sources/PackageLoading/PackageBuilder.swift | 2 +- Sources/PackageModel/BuildSettings.swift | 4 +++- Sources/SPMBuildCore/Plugins/PluginInvocation.swift | 2 +- Sources/XCBuildSupport/PIF.swift | 2 +- Sources/XCBuildSupport/PIFBuilder.swift | 2 +- Tests/PackageLoadingTests/PackageBuilderTests.swift | 5 ++--- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 0011bcff501..315a7116454 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2020 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index 26b8729a846..2beb3c6d76c 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2021 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information diff --git a/Sources/PackageModel/BuildSettings.swift b/Sources/PackageModel/BuildSettings.swift index d951cc92788..14951dce9be 100644 --- a/Sources/PackageModel/BuildSettings.swift +++ b/Sources/PackageModel/BuildSettings.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2018 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information @@ -43,6 +43,7 @@ public enum BuildSettings { /// The assignment value. public var values: [String] + // FIXME: This should use `Set` but we need to investigate potential build failures on Linux caused by using it. /// The condition associated with this assignment. public var conditions: [PackageCondition] { get { @@ -71,6 +72,7 @@ public enum BuildSettings { /// Add the given assignment to the table. mutating public func add(_ assignment: Assignment, for decl: Declaration) { + // FIXME: We should check for duplicate assignments. assignments[decl, default: []].append(assignment) } } diff --git a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift index 2a8544acd31..e7414507105 100644 --- a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift +++ b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2021-2022 Apple Inc. and the Swift project authors +// Copyright (c) 2021-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information diff --git a/Sources/XCBuildSupport/PIF.swift b/Sources/XCBuildSupport/PIF.swift index 69df9409de3..168ff3389ed 100644 --- a/Sources/XCBuildSupport/PIF.swift +++ b/Sources/XCBuildSupport/PIF.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2020 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index fe8e14f7671..ab6d29e78e8 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2021 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information diff --git a/Tests/PackageLoadingTests/PackageBuilderTests.swift b/Tests/PackageLoadingTests/PackageBuilderTests.swift index 26268ba2463..6535b317d62 100644 --- a/Tests/PackageLoadingTests/PackageBuilderTests.swift +++ b/Tests/PackageLoadingTests/PackageBuilderTests.swift @@ -2,7 +2,7 @@ // // This source file is part of the Swift open source project // -// Copyright (c) 2014-2021 Apple Inc. and the Swift project authors +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See http://swift.org/LICENSE.txt for license information @@ -19,8 +19,7 @@ import XCTest import class TSCBasic.InMemoryFileSystem /// Tests for the handling of source layout conventions. -class PackageBuilderTests: XCTestCase { - +final class PackageBuilderTests: XCTestCase { func testDotFilesAreIgnored() throws { let fs = InMemoryFileSystem(emptyFiles: "/Sources/foo/.Bar.swift", From d6c0c8ab4d2f1ab465216ab00371d79527a7036b Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 16:22:39 +0000 Subject: [PATCH 07/24] Reduce the diff --- Sources/XCBuildSupport/PIF.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/XCBuildSupport/PIF.swift b/Sources/XCBuildSupport/PIF.swift index 168ff3389ed..2029bddca49 100644 --- a/Sources/XCBuildSupport/PIF.swift +++ b/Sources/XCBuildSupport/PIF.swift @@ -1002,8 +1002,7 @@ public enum PIF { } public var conditions: [String] { - let filters = [.init(platforms: [packageModelPlatform])] - .toPlatformFilters().map { (filter: PIF.PlatformFilter) -> String in + let filters = [PackageCondition(platforms: [packageModelPlatform])].toPlatformFilters().map { filter in if filter.environment.isEmpty { return filter.platform } else { From f3004a7a4929cc9fa5628706e7e066decbe4b8ae Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 16:24:19 +0000 Subject: [PATCH 08/24] Reduce the diff --- Tests/PackageLoadingTests/PackageBuilderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/PackageLoadingTests/PackageBuilderTests.swift b/Tests/PackageLoadingTests/PackageBuilderTests.swift index 6535b317d62..6d530c4cf62 100644 --- a/Tests/PackageLoadingTests/PackageBuilderTests.swift +++ b/Tests/PackageLoadingTests/PackageBuilderTests.swift @@ -2989,7 +2989,7 @@ final class PackageBuilderTests: XCTestCase { var assignment = BuildSettings.Assignment() assignment.values = ["YOLO"] - assignment.conditions = [.init(platforms: [PackageModel.Platform.custom(name: "bestOS", oldestSupportedVersion: .unknown)])] + assignment.conditions = [PackageCondition(platforms: [.custom(name: "bestOS", oldestSupportedVersion: .unknown)])] var settings = BuildSettings.AssignmentTable() settings.add(assignment, for: .SWIFT_ACTIVE_COMPILATION_CONDITIONS) From a6760d93f307110827682e5472ecdafc1c0d3075 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 19:22:45 +0000 Subject: [PATCH 09/24] Reduce the diff to track the regression --- .../Resolution/ResolvedTarget.swift | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 315a7116454..e6c478f1763 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -17,7 +17,7 @@ import func TSCBasic.topologicalSort /// Represents a fully resolved target. All the dependencies for the target are resolved. public final class ResolvedTarget { /// Represents dependency of a resolved target. - public enum Dependency: Hashable { + public enum Dependency { /// Direct dependency of the target. This target is in the same package and should be statically linked. case target(_ target: ResolvedTarget, conditions: [PackageCondition]) @@ -171,6 +171,30 @@ extension ResolvedTarget: CustomStringConvertible { } } +extension ResolvedTarget.Dependency: Equatable { + public static func == (lhs: ResolvedTarget.Dependency, rhs: ResolvedTarget.Dependency) -> Bool { + switch (lhs, rhs) { + case (.target(let lhsTarget, _), .target(let rhsTarget, _)): + return lhsTarget == rhsTarget + case (.product(let lhsProduct, _), .product(let rhsProduct, _)): + return lhsProduct == rhsProduct + case (.product, .target), (.target, .product): + return false + } + } +} + +extension ResolvedTarget.Dependency: Hashable { + public func hash(into hasher: inout Hasher) { + switch self { + case .target(let target, _): + hasher.combine(target) + case .product(let product, _): + hasher.combine(product) + } + } +} + extension ResolvedTarget.Dependency: CustomStringConvertible { public var description: String { var str = " Date: Sun, 3 Dec 2023 00:03:05 +0000 Subject: [PATCH 10/24] [NFC] Remove `SupportedPlatforms`, add `PlatformVersionProvider` Depends on https://github.com/apple/swift-package-manager/pull/7117, unblocks https://github.com/apple/swift-package-manager/pull/7160. The `PackageModel` module has no business of defining supported platforms computation for resolved targets, products, and packages. This belongs to `PackageGraph`, but was previously leaking out into `PackageModel` by passing the `derivedXCTestPlatformProvider` closure around. That prevented us from converting marking `SupportedPlatforms` as `Hashable` and marking any of `Resolved*` types as value types and also `Hashable`. In the future, when we'll also need to make them `Sendable`, this could become problematic, passing so much state captured in a closure, mostly by accident. --- .../ProductBuildDescription.swift | 2 +- Sources/Build/BuildPlan/BuildPlan+Test.swift | 9 +- Sources/Build/BuildPlan/BuildPlan.swift | 6 +- Sources/PackageGraph/CMakeLists.txt | 1 + .../PackageGraph/PackageGraph+Loading.swift | 70 ++++++----- .../Resolution/PlatformVersionProvider.swift | 109 ++++++++++++++++++ .../Resolution/ResolvedPackage.swift | 20 +++- .../Resolution/ResolvedProduct.swift | 54 ++++----- .../Resolution/ResolvedTarget.swift | 18 ++- Sources/PackageModel/Platform.swift | 55 --------- .../SPMTestSupport/PackageGraphTester.swift | 12 +- Sources/XCBuildSupport/PIFBuilder.swift | 34 +++--- Tests/PackageGraphTests/TargetTests.swift | 3 +- 13 files changed, 244 insertions(+), 149 deletions(-) create mode 100644 Sources/PackageGraph/Resolution/PlatformVersionProvider.swift diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index bd63fa749c4..a779f548070 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -284,7 +284,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription // When deploying to macOS prior to macOS 12, add an rpath to the // back-deployed concurrency libraries. if useStdlibRpath, self.buildParameters.targetTriple.isMacOSX { - let macOSSupportedPlatform = self.package.platforms.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest) + let macOSSupportedPlatform = self.package.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest) if macOSSupportedPlatform.version.major < 12 { let backDeployedStdlib = try buildParameters.toolchain.macosSwiftStdlib .parentDirectory diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index 0def8c0f985..70c0faee9e5 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -87,7 +87,8 @@ extension BuildPlan { target: discoveryTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) }, defaultLocalization: testProduct.defaultLocalization, - platforms: testProduct.platforms + supportedPlatforms: testProduct.supportedPlatforms, + platformVersionProvider: testProduct.platformVersionProvider ) let discoveryTargetBuildDescription = try SwiftTargetBuildDescription( package: package, @@ -120,7 +121,8 @@ extension BuildPlan { target: entryPointTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) } + [.target(discoveryResolvedTarget, conditions: [])], defaultLocalization: testProduct.defaultLocalization, - platforms: testProduct.platforms + supportedPlatforms: testProduct.supportedPlatforms, + platformVersionProvider: testProduct.platformVersionProvider ) return try SwiftTargetBuildDescription( package: package, @@ -149,7 +151,8 @@ extension BuildPlan { target: entryPointTarget, dependencies: entryPointResolvedTarget.dependencies + [.target(discoveryTargets.resolved, conditions: [])], defaultLocalization: testProduct.defaultLocalization, - platforms: testProduct.platforms + supportedPlatforms: testProduct.supportedPlatforms, + platformVersionProvider: testProduct.platformVersionProvider ) let entryPointTargetBuildDescription = try SwiftTargetBuildDescription( package: package, diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index fadea472769..a076bddb7b8 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -133,7 +133,7 @@ extension BuildParameters { // Compute the triple string for Darwin platform using the platform version. if targetTriple.isDarwin() { let platform = buildEnvironment.platform - let supportedPlatform = target.platforms.getDerived(for: platform, usingXCTest: target.type == .test) + let supportedPlatform = target.getDerived(for: platform, usingXCTest: target.type == .test) args += [targetTriple.tripleString(forPlatformVersion: supportedPlatform.version.versionString)] } else { args += [targetTriple.tripleString] @@ -409,8 +409,8 @@ public class BuildPlan: SPMBuildCore.BuildPlan { ) throws { // Supported platforms are defined at the package level. // This will need to become a bit complicated once we have target-level or product-level platform support. - let productPlatform = product.platforms.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest) - let targetPlatform = target.platforms.getDerived(for: .macOS, usingXCTest: target.type == .test) + let productPlatform = product.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest) + let targetPlatform = target.getDerived(for: .macOS, usingXCTest: target.type == .test) // Check if the version requirement is satisfied. // diff --git a/Sources/PackageGraph/CMakeLists.txt b/Sources/PackageGraph/CMakeLists.txt index 9d70abc6546..e7cbdad4619 100644 --- a/Sources/PackageGraph/CMakeLists.txt +++ b/Sources/PackageGraph/CMakeLists.txt @@ -31,6 +31,7 @@ add_library(PackageGraph Resolution/DependencyResolverBinding.swift Resolution/DependencyResolverDelegate.swift Resolution/DependencyResolverError.swift + Resolution/PlatformVersionProvider.swift Resolution/ResolvedPackage.swift Resolution/ResolvedProduct.swift Resolution/ResolvedTarget.swift diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index 5fe6b21ec30..5b81ba146f2 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -153,13 +153,9 @@ extension PackageGraph { rootManifests: root.manifests, unsafeAllowedPackages: unsafeAllowedPackages, platformRegistry: customPlatformsRegistry ?? .default, - derivedXCTestPlatformProvider: { declared in - if let customXCTestMinimumDeploymentTargets { - return customXCTestMinimumDeploymentTargets[declared] - } else { - return MinimumDeploymentTarget.default.computeXCTestMinimumDeploymentTarget(for: declared) - } - }, + platformVersionProvider: .init( + implementation: .customXCTestMinimumDeploymentTargets(customXCTestMinimumDeploymentTargets) + ), fileSystem: fileSystem, observabilityScope: observabilityScope ) @@ -240,7 +236,7 @@ private func createResolvedPackages( rootManifests: [PackageIdentity: Manifest], unsafeAllowedPackages: Set, platformRegistry: PlatformRegistry, - derivedXCTestPlatformProvider: @escaping (_ declared: PackageModel.Platform) -> PlatformVersion?, + platformVersionProvider: PlatformVersionProvider, fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws -> [ResolvedPackage] { @@ -257,7 +253,8 @@ private func createResolvedPackages( package, productFilter: node.productFilter, isAllowedToVendUnsafeProducts: isAllowedToVendUnsafeProducts, - allowedToOverride: allowedToOverride + allowedToOverride: allowedToOverride, + platformVersionProvider: platformVersionProvider ) } @@ -361,14 +358,19 @@ private func createResolvedPackages( packageBuilder.defaultLocalization = package.manifest.defaultLocalization - packageBuilder.platforms = computePlatforms( + packageBuilder.supportedPlatforms = computePlatforms( package: package, - platformRegistry: platformRegistry, - derivedXCTestPlatformProvider: derivedXCTestPlatformProvider + platformRegistry: platformRegistry ) // Create target builders for each target in the package. - let targetBuilders = package.targets.map{ ResolvedTargetBuilder(target: $0, observabilityScope: packageObservabilityScope) } + let targetBuilders = package.targets.map { + ResolvedTargetBuilder( + target: $0, + observabilityScope: packageObservabilityScope, + platformVersionProvider: platformVersionProvider + ) + } packageBuilder.targets = targetBuilders // Establish dependencies between the targets. A target can only depend on another target present in the same package. @@ -386,7 +388,7 @@ private func createResolvedPackages( } } targetBuilder.defaultLocalization = packageBuilder.defaultLocalization - targetBuilder.platforms = packageBuilder.platforms + targetBuilder.supportedPlatforms = packageBuilder.supportedPlatforms } // Create product builders for each product in the package. A product can only contain a target present in the same package. @@ -743,10 +745,8 @@ private class DuplicateProductsChecker { private func computePlatforms( package: Package, - platformRegistry: PlatformRegistry, - derivedXCTestPlatformProvider: @escaping (_ declared: PackageModel.Platform) -> PlatformVersion? -) -> SupportedPlatforms { - + platformRegistry: PlatformRegistry +) -> [SupportedPlatform] { // the supported platforms as declared in the manifest let declaredPlatforms: [SupportedPlatform] = package.manifest.platforms.map { platform in let declaredPlatform = platformRegistry.platformByName[platform.platformName] @@ -758,10 +758,7 @@ private func computePlatforms( ) } - return SupportedPlatforms( - declared: declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }), - derivedXCTestPlatformProvider: derivedXCTestPlatformProvider - ) + return declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }) } // Track and override module aliases specified for targets in a package graph @@ -888,11 +885,14 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { var defaultLocalization: String? = nil /// The platforms supported by this package. - var platforms: SupportedPlatforms = .init(declared: [], derivedXCTestPlatformProvider: .none) + var supportedPlatforms: [SupportedPlatform] = [] + + let platformVersionProvider: PlatformVersionProvider init( target: Target, - observabilityScope: ObservabilityScope + observabilityScope: ObservabilityScope, + platformVersionProvider: PlatformVersionProvider ) { self.target = target self.diagnosticsEmitter = observabilityScope.makeDiagnosticsEmitter() { @@ -900,6 +900,7 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { metadata.targetName = target.name return metadata } + self.platformVersionProvider = platformVersionProvider } func diagnoseInvalidUseOfUnsafeFlags(_ product: ResolvedProduct) throws { @@ -934,7 +935,8 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { target: self.target, dependencies: dependencies, defaultLocalization: self.defaultLocalization, - platforms: self.platforms + supportedPlatforms: self.supportedPlatforms, + platformVersionProvider: self.platformVersionProvider ) } } @@ -983,27 +985,37 @@ private final class ResolvedPackageBuilder: ResolvedBuilder { var defaultLocalization: String? = nil /// The platforms supported by this package. - var platforms: SupportedPlatforms = .init(declared: [], derivedXCTestPlatformProvider: .none) + var supportedPlatforms: [SupportedPlatform] = [] /// If the given package's source is a registry release, this provides additional metadata and signature information. var registryMetadata: RegistryReleaseMetadata? - init(_ package: Package, productFilter: ProductFilter, isAllowedToVendUnsafeProducts: Bool, allowedToOverride: Bool) { + let platformVersionProvider: PlatformVersionProvider + + init( + _ package: Package, + productFilter: ProductFilter, + isAllowedToVendUnsafeProducts: Bool, + allowedToOverride: Bool, + platformVersionProvider: PlatformVersionProvider + ) { self.package = package self.productFilter = productFilter self.isAllowedToVendUnsafeProducts = isAllowedToVendUnsafeProducts self.allowedToOverride = allowedToOverride + self.platformVersionProvider = platformVersionProvider } override func constructImpl() throws -> ResolvedPackage { return ResolvedPackage( package: self.package, defaultLocalization: self.defaultLocalization, - platforms: self.platforms, + supportedPlatforms: self.supportedPlatforms, dependencies: try self.dependencies.map{ try $0.construct() }, targets: try self.targets.map{ try $0.construct() }, products: try self.products.map{ try $0.construct() }, - registryMetadata: self.registryMetadata + registryMetadata: self.registryMetadata, + platformVersionProvider: self.platformVersionProvider ) } } diff --git a/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift b/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift new file mode 100644 index 00000000000..eac22178a1d --- /dev/null +++ b/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift @@ -0,0 +1,109 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import struct PackageModel.MinimumDeploymentTarget +import struct PackageModel.Platform +import struct PackageModel.PlatformVersion +import struct PackageModel.SupportedPlatform + +/// Merging two sets of supported platforms, preferring the max constraint +func merge(into partial: inout [SupportedPlatform], platforms: [SupportedPlatform]) { + for platformSupport in platforms { + if let existing = partial.firstIndex(where: { $0.platform == platformSupport.platform }) { + if partial[existing].version < platformSupport.version { + partial.remove(at: existing) + partial.append(platformSupport) + } + } else { + partial.append(platformSupport) + } + } +} + +public struct PlatformVersionProvider: Hashable { + public enum Implementation: Hashable { + case mergingFromTargets([ResolvedTarget]) + case customXCTestMinimumDeploymentTargets([PackageModel.Platform: PlatformVersion]?) + case empty + } + + private let implementation: Implementation + + public init(implementation: Implementation) { + self.implementation = implementation + } + + func derivedXCTestPlatformProvider(_ declared: PackageModel.Platform) -> PlatformVersion? { + switch implementation { + case .mergingFromTargets(let targets): + let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in + merge(into: &partial, platforms: [item.getDerived(for: declared, usingXCTest: item.type == .test)]) + } + return platforms.first!.version + + case .customXCTestMinimumDeploymentTargets(let customXCTestMinimumDeploymentTargets): + if let customXCTestMinimumDeploymentTargets { + return customXCTestMinimumDeploymentTargets[declared] + } else { + return MinimumDeploymentTarget.default.computeXCTestMinimumDeploymentTarget(for: declared) + } + + case .empty: + return nil + } + } + + /// Returns the supported platform instance for the given platform. + func getDerived(declared: [SupportedPlatform], for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + // derived platform based on known minimum deployment target logic + if let declaredPlatform = declared.first(where: { $0.platform == platform }) { + var version = declaredPlatform.version + + if usingXCTest, let xcTestMinimumDeploymentTarget = self.derivedXCTestPlatformProvider(platform), version < xcTestMinimumDeploymentTarget { + version = xcTestMinimumDeploymentTarget + } + + // If the declared version is smaller than the oldest supported one, we raise the derived version to that. + if version < platform.oldestSupportedVersion { + version = platform.oldestSupportedVersion + } + + return SupportedPlatform( + platform: declaredPlatform.platform, + version: version, + options: declaredPlatform.options + ) + } else { + let minimumSupportedVersion: PlatformVersion + if usingXCTest, let xcTestMinimumDeploymentTarget = self.derivedXCTestPlatformProvider(platform), xcTestMinimumDeploymentTarget > platform.oldestSupportedVersion { + minimumSupportedVersion = xcTestMinimumDeploymentTarget + } else { + minimumSupportedVersion = platform.oldestSupportedVersion + } + + let oldestSupportedVersion: PlatformVersion + if platform == .macCatalyst { + let iOS = self.getDerived(declared: declared, for: .iOS, usingXCTest: usingXCTest) + // If there was no deployment target specified for Mac Catalyst, fall back to the iOS deployment target. + oldestSupportedVersion = max(minimumSupportedVersion, iOS.version) + } else { + oldestSupportedVersion = minimumSupportedVersion + } + + return SupportedPlatform( + platform: platform, + version: oldestSupportedVersion, + options: [] + ) + } + } +} diff --git a/Sources/PackageGraph/Resolution/ResolvedPackage.swift b/Sources/PackageGraph/Resolution/ResolvedPackage.swift index fe7939c499f..72916172972 100644 --- a/Sources/PackageGraph/Resolution/ResolvedPackage.swift +++ b/Sources/PackageGraph/Resolution/ResolvedPackage.swift @@ -46,27 +46,39 @@ public final class ResolvedPackage { public let defaultLocalization: String? /// The list of platforms that are supported by this target. - public let platforms: SupportedPlatforms + public let supportedPlatforms: [SupportedPlatform] /// If the given package's source is a registry release, this provides additional metadata and signature information. public let registryMetadata: RegistryReleaseMetadata? + private let platformVersionProvider: PlatformVersionProvider + public init( package: Package, defaultLocalization: String?, - platforms: SupportedPlatforms, + supportedPlatforms: [SupportedPlatform], dependencies: [ResolvedPackage], targets: [ResolvedTarget], products: [ResolvedProduct], - registryMetadata: RegistryReleaseMetadata? + registryMetadata: RegistryReleaseMetadata?, + platformVersionProvider: PlatformVersionProvider ) { self.underlyingPackage = package self.defaultLocalization = defaultLocalization - self.platforms = platforms + self.supportedPlatforms = supportedPlatforms self.dependencies = dependencies self.targets = targets self.products = products self.registryMetadata = registryMetadata + self.platformVersionProvider = platformVersionProvider + } + + public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + self.platformVersionProvider.getDerived( + declared: self.supportedPlatforms, + for: platform, + usingXCTest: usingXCTest + ) } } diff --git a/Sources/PackageGraph/Resolution/ResolvedProduct.swift b/Sources/PackageGraph/Resolution/ResolvedProduct.swift index e27b13883e3..e677c5cc1a1 100644 --- a/Sources/PackageGraph/Resolution/ResolvedProduct.swift +++ b/Sources/PackageGraph/Resolution/ResolvedProduct.swift @@ -37,7 +37,9 @@ public final class ResolvedProduct { public let defaultLocalization: String? /// The list of platforms that are supported by this product. - public let platforms: SupportedPlatforms + public let supportedPlatforms: [SupportedPlatform] + + public let platformVersionProvider: PlatformVersionProvider /// The main executable target of product. /// @@ -64,10 +66,11 @@ public final class ResolvedProduct { let defaultLocalization = self.targets.first?.defaultLocalization self.defaultLocalization = defaultLocalization - let platforms = Self.computePlatforms(targets: targets) - self.platforms = platforms + let (platforms, platformVersionProvider) = Self.computePlatforms(targets: targets) + self.supportedPlatforms = platforms + self.platformVersionProvider = platformVersionProvider - self.testEntryPointTarget = underlyingProduct.testEntryPointPath.map { testEntryPointPath in + self.testEntryPointTarget = product.testEntryPointPath.map { testEntryPointPath in // Create an executable resolved target with the entry point file, adding product's targets as dependencies. let dependencies: [Target.Dependency] = product.targets.map { .target($0, conditions: []) } let swiftTarget = SwiftTarget(name: product.name, @@ -78,7 +81,8 @@ public final class ResolvedProduct { target: swiftTarget, dependencies: targets.map { .target($0, conditions: []) }, defaultLocalization: defaultLocalization ?? .none, // safe since this is a derived product - platforms: platforms + supportedPlatforms: platforms, + platformVersionProvider: platformVersionProvider ) } } @@ -100,33 +104,23 @@ public final class ResolvedProduct { let recursiveDependencies = try targets.lazy.flatMap { try $0.recursiveTargetDependencies() } return Array(Set(targets).union(recursiveDependencies)) } - - private static func computePlatforms(targets: [ResolvedTarget]) -> SupportedPlatforms { - // merging two sets of supported platforms, preferring the max constraint - func merge(into partial: inout [SupportedPlatform], platforms: [SupportedPlatform]) { - for platformSupport in platforms { - if let existing = partial.firstIndex(where: { $0.platform == platformSupport.platform }) { - if partial[existing].version < platformSupport.version { - partial.remove(at: existing) - partial.append(platformSupport) - } - } else { - partial.append(platformSupport) - } - } + private static func computePlatforms(targets: [ResolvedTarget]) -> ([SupportedPlatform], PlatformVersionProvider) { + let declaredPlatforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in + merge(into: &partial, platforms: item.supportedPlatforms) } - let declared = targets.reduce(into: [SupportedPlatform]()) { partial, item in - merge(into: &partial, platforms: item.platforms.declared) - } - - return SupportedPlatforms( - declared: declared.sorted(by: { $0.platform.name < $1.platform.name })) { declared in - let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in - merge(into: &partial, platforms: [item.platforms.getDerived(for: declared, usingXCTest: item.type == .test)]) - } - return platforms.first!.version - } + return ( + declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }), + PlatformVersionProvider(implementation: .mergingFromTargets(targets)) + ) + } + + public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + self.platformVersionProvider.getDerived( + declared: self.supportedPlatforms, + for: platform, + usingXCTest: usingXCTest + ) } } diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index e6c478f1763..d67e6335729 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -139,19 +139,31 @@ public final class ResolvedTarget { public let defaultLocalization: String? /// The list of platforms that are supported by this target. - public let platforms: SupportedPlatforms + public let supportedPlatforms: [SupportedPlatform] + + private let platformVersionProvider: PlatformVersionProvider /// Create a resolved target instance. public init( target: Target, dependencies: [Dependency], defaultLocalization: String?, - platforms: SupportedPlatforms + supportedPlatforms: [SupportedPlatform], + platformVersionProvider: PlatformVersionProvider ) { self.underlyingTarget = target self.dependencies = dependencies self.defaultLocalization = defaultLocalization - self.platforms = platforms + self.supportedPlatforms = supportedPlatforms + self.platformVersionProvider = platformVersionProvider + } + + public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + self.platformVersionProvider.getDerived( + declared: self.supportedPlatforms, + for: platform, + usingXCTest: usingXCTest + ) } } diff --git a/Sources/PackageModel/Platform.swift b/Sources/PackageModel/Platform.swift index b74d94b93f9..8cce1a3dc8b 100644 --- a/Sources/PackageModel/Platform.swift +++ b/Sources/PackageModel/Platform.swift @@ -52,61 +52,6 @@ public struct Platform: Equatable, Hashable, Codable { } -public struct SupportedPlatforms { - public let declared: [SupportedPlatform] - private let derivedXCTestPlatformProvider: ((Platform) -> PlatformVersion?)? - - public init(declared: [SupportedPlatform], derivedXCTestPlatformProvider: ((_ declared: Platform) -> PlatformVersion?)?) { - self.declared = declared - self.derivedXCTestPlatformProvider = derivedXCTestPlatformProvider - } - - /// Returns the supported platform instance for the given platform. - public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { - // derived platform based on known minimum deployment target logic - if let declaredPlatform = self.declared.first(where: { $0.platform == platform }) { - var version = declaredPlatform.version - - if usingXCTest, let xcTestMinimumDeploymentTarget = derivedXCTestPlatformProvider?(platform), version < xcTestMinimumDeploymentTarget { - version = xcTestMinimumDeploymentTarget - } - - // If the declared version is smaller than the oldest supported one, we raise the derived version to that. - if version < platform.oldestSupportedVersion { - version = platform.oldestSupportedVersion - } - - return SupportedPlatform( - platform: declaredPlatform.platform, - version: version, - options: declaredPlatform.options - ) - } else { - let minimumSupportedVersion: PlatformVersion - if usingXCTest, let xcTestMinimumDeploymentTarget = derivedXCTestPlatformProvider?(platform), xcTestMinimumDeploymentTarget > platform.oldestSupportedVersion { - minimumSupportedVersion = xcTestMinimumDeploymentTarget - } else { - minimumSupportedVersion = platform.oldestSupportedVersion - } - - let oldestSupportedVersion: PlatformVersion - if platform == .macCatalyst { - let iOS = getDerived(for: .iOS, usingXCTest: usingXCTest) - // If there was no deployment target specified for Mac Catalyst, fall back to the iOS deployment target. - oldestSupportedVersion = max(minimumSupportedVersion, iOS.version) - } else { - oldestSupportedVersion = minimumSupportedVersion - } - - return SupportedPlatform( - platform: platform, - version: oldestSupportedVersion, - options: [] - ) - } - } -} - /// Represents a platform supported by a target. public struct SupportedPlatform: Equatable, Codable { /// The platform. diff --git a/Sources/SPMTestSupport/PackageGraphTester.swift b/Sources/SPMTestSupport/PackageGraphTester.swift index 981f7fc91e0..5af4a88eb99 100644 --- a/Sources/SPMTestSupport/PackageGraphTester.swift +++ b/Sources/SPMTestSupport/PackageGraphTester.swift @@ -174,7 +174,7 @@ public final class ResolvedTargetResult { } public func checkDeclaredPlatforms(_ platforms: [String: String], file: StaticString = #file, line: UInt = #line) { - let targetPlatforms = Dictionary(uniqueKeysWithValues: target.platforms.declared.map({ ($0.platform.name, $0.version.versionString) })) + let targetPlatforms = Dictionary(uniqueKeysWithValues: target.supportedPlatforms.map({ ($0.platform.name, $0.version.versionString) })) XCTAssertEqual(platforms, targetPlatforms, file: file, line: line) } @@ -182,7 +182,7 @@ public final class ResolvedTargetResult { let derived = platforms.map { let platform = PlatformRegistry.default.platformByName[$0.key] ?? PackageModel.Platform .custom(name: $0.key, oldestSupportedVersion: $0.value) - return self.target.platforms.getDerived(for: platform, usingXCTest: self.target.type == .test) + return self.target.getDerived(for: platform, usingXCTest: self.target.type == .test) } let targetPlatforms = Dictionary( uniqueKeysWithValues: derived @@ -192,7 +192,7 @@ public final class ResolvedTargetResult { } public func checkDerivedPlatformOptions(_ platform: PackageModel.Platform, options: [String], file: StaticString = #file, line: UInt = #line) { - let platform = target.platforms.getDerived(for: platform, usingXCTest: target.type == .test) + let platform = target.getDerived(for: platform, usingXCTest: target.type == .test) XCTAssertEqual(platform.options, options, file: file, line: line) } } @@ -233,21 +233,21 @@ public final class ResolvedProductResult { } public func checkDeclaredPlatforms(_ platforms: [String: String], file: StaticString = #file, line: UInt = #line) { - let targetPlatforms = Dictionary(uniqueKeysWithValues: product.platforms.declared.map({ ($0.platform.name, $0.version.versionString) })) + let targetPlatforms = Dictionary(uniqueKeysWithValues: product.supportedPlatforms.map({ ($0.platform.name, $0.version.versionString) })) XCTAssertEqual(platforms, targetPlatforms, file: file, line: line) } public func checkDerivedPlatforms(_ platforms: [String: String], file: StaticString = #file, line: UInt = #line) { let derived = platforms.map { let platform = PlatformRegistry.default.platformByName[$0.key] ?? PackageModel.Platform.custom(name: $0.key, oldestSupportedVersion: $0.value) - return product.platforms.getDerived(for: platform, usingXCTest: product.isLinkingXCTest) + return product.getDerived(for: platform, usingXCTest: product.isLinkingXCTest) } let targetPlatforms = Dictionary(uniqueKeysWithValues: derived.map({ ($0.platform.name, $0.version.versionString) })) XCTAssertEqual(platforms, targetPlatforms, file: file, line: line) } public func checkDerivedPlatformOptions(_ platform: PackageModel.Platform, options: [String], file: StaticString = #file, line: UInt = #line) { - let platform = product.platforms.getDerived(for: platform, usingXCTest: product.isLinkingXCTest) + let platform = product.getDerived(for: platform, usingXCTest: product.isLinkingXCTest) XCTAssertEqual(platform.options, options, file: file, line: line) } } diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index ab6d29e78e8..b984499c105 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -271,13 +271,13 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { settings[.SDKROOT] = "auto" settings[.SDK_VARIANT] = "auto" settings[.SKIP_INSTALL] = "YES" - settings[.MACOSX_DEPLOYMENT_TARGET] = package.platforms.deploymentTarget(for: .macOS) - settings[.IPHONEOS_DEPLOYMENT_TARGET] = package.platforms.deploymentTarget(for: .iOS) - settings[.IPHONEOS_DEPLOYMENT_TARGET, for: .macCatalyst] = package.platforms.deploymentTarget(for: .macCatalyst) - settings[.TVOS_DEPLOYMENT_TARGET] = package.platforms.deploymentTarget(for: .tvOS) - settings[.WATCHOS_DEPLOYMENT_TARGET] = package.platforms.deploymentTarget(for: .watchOS) - settings[.XROS_DEPLOYMENT_TARGET] = package.platforms.deploymentTarget(for: .visionOS) - settings[.DRIVERKIT_DEPLOYMENT_TARGET] = package.platforms.deploymentTarget(for: .driverKit) + settings[.MACOSX_DEPLOYMENT_TARGET] = package.deploymentTarget(for: .macOS) + settings[.IPHONEOS_DEPLOYMENT_TARGET] = package.deploymentTarget(for: .iOS) + settings[.IPHONEOS_DEPLOYMENT_TARGET, for: .macCatalyst] = package.deploymentTarget(for: .macCatalyst) + settings[.TVOS_DEPLOYMENT_TARGET] = package.deploymentTarget(for: .tvOS) + settings[.WATCHOS_DEPLOYMENT_TARGET] = package.deploymentTarget(for: .watchOS) + settings[.XROS_DEPLOYMENT_TARGET] = package.deploymentTarget(for: .visionOS) + settings[.DRIVERKIT_DEPLOYMENT_TARGET] = package.deploymentTarget(for: .driverKit) settings[.DYLIB_INSTALL_NAME_BASE] = "@rpath" settings[.USE_HEADERMAP] = "NO" settings[.SWIFT_ACTIVE_COMPILATION_CONDITIONS] = ["$(inherited)", "SWIFT_PACKAGE"] @@ -301,7 +301,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { PlatformRegistry.default.knownPlatforms.forEach { guard let platform = PIF.BuildSettings.Platform.from(platform: $0) else { return } - let supportedPlatform = package.platforms.getDerived(for: $0, usingXCTest: false) + let supportedPlatform = package.getDerived(for: $0, usingXCTest: false) if !supportedPlatform.options.isEmpty { settings[.SPECIALIZATION_SDK_OPTIONS, for: platform] = supportedPlatform.options } @@ -430,11 +430,11 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { // Tests can have a custom deployment target based on the minimum supported by XCTest. if mainTarget.underlyingTarget.type == .test { - settings[.MACOSX_DEPLOYMENT_TARGET] = mainTarget.platforms.deploymentTarget(for: .macOS, usingXCTest: true) - settings[.IPHONEOS_DEPLOYMENT_TARGET] = mainTarget.platforms.deploymentTarget(for: .iOS, usingXCTest: true) - settings[.TVOS_DEPLOYMENT_TARGET] = mainTarget.platforms.deploymentTarget(for: .tvOS, usingXCTest: true) - settings[.WATCHOS_DEPLOYMENT_TARGET] = mainTarget.platforms.deploymentTarget(for: .watchOS, usingXCTest: true) - settings[.XROS_DEPLOYMENT_TARGET] = mainTarget.platforms.deploymentTarget(for: .visionOS, usingXCTest: true) + settings[.MACOSX_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .macOS, usingXCTest: true) + settings[.IPHONEOS_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .iOS, usingXCTest: true) + settings[.TVOS_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .tvOS, usingXCTest: true) + settings[.WATCHOS_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .watchOS, usingXCTest: true) + settings[.XROS_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .visionOS, usingXCTest: true) } if product.type == .executable { @@ -1417,7 +1417,13 @@ extension Array where Element == ResolvedTarget.Dependency { } } -extension SupportedPlatforms { +extension ResolvedPackage { + func deploymentTarget(for platform: PackageModel.Platform, usingXCTest: Bool = false) -> String? { + return self.getDerived(for: platform, usingXCTest: usingXCTest).version.versionString + } +} + +extension ResolvedTarget { func deploymentTarget(for platform: PackageModel.Platform, usingXCTest: Bool = false) -> String? { return self.getDerived(for: platform, usingXCTest: usingXCTest).version.versionString } diff --git a/Tests/PackageGraphTests/TargetTests.swift b/Tests/PackageGraphTests/TargetTests.swift index c8c1e4a2f05..f09d5f119e6 100644 --- a/Tests/PackageGraphTests/TargetTests.swift +++ b/Tests/PackageGraphTests/TargetTests.swift @@ -30,7 +30,8 @@ private extension ResolvedTarget { ), dependencies: deps.map { .target($0, conditions: []) }, defaultLocalization: nil, - platforms: .init(declared: [], derivedXCTestPlatformProvider: .none) + supportedPlatforms: [], + platformVersionProvider: .init(implementation: .empty) ) } } From 1dee1129ea784573c3de1d2f115fa3e4feebe045 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 19:21:22 +0000 Subject: [PATCH 11/24] Use value types for storage in `Resolved*` types of `PackageGraph` --- .../ClangTargetBuildDescription.swift | 8 +- .../BuildDescription/PluginDescription.swift | 2 +- .../ProductBuildDescription.swift | 2 +- .../SharedTargetBuildDescription.swift | 2 +- .../SwiftTargetBuildDescription.swift | 10 +- .../LLBuildManifestBuilder+Swift.swift | 10 +- Sources/Build/BuildOperation.swift | 2 +- Sources/Build/BuildPlan/BuildPlan+Clang.swift | 2 +- .../Build/BuildPlan/BuildPlan+Product.swift | 16 +- Sources/Build/BuildPlan/BuildPlan+Swift.swift | 2 +- Sources/Build/BuildPlan/BuildPlan+Test.swift | 12 +- Sources/Build/BuildPlan/BuildPlan.swift | 16 +- Sources/Commands/PackageTools/APIDiff.swift | 4 +- .../PackageTools/CompletionTool.swift | 4 +- .../PackageTools/InstalledPackages.swift | 4 +- .../Commands/PackageTools/PluginCommand.swift | 2 +- Sources/Commands/Utilities/APIDigester.swift | 2 +- .../Commands/Utilities/PluginDelegate.swift | 7 +- Sources/PackageGraph/ModuleAliasTracker.swift | 59 +++--- .../PackageGraph/PackageGraph+Loading.swift | 46 ++--- Sources/PackageGraph/PackageGraphRoot.swift | 1 + .../Resolution/ResolvedPackage.swift | 94 ++++++--- .../Resolution/ResolvedProduct.swift | 194 ++++++++++++------ .../Resolution/ResolvedTarget.swift | 110 +++++----- Sources/PackageLoading/PackageBuilder.swift | 10 +- Sources/PackageModel/BuildConfiguration.swift | 2 +- .../PackageConditionDescription.swift | 8 +- .../Manifest/PlatformDescription.swift | 2 +- .../Manifest/ProductDescription.swift | 2 +- .../SystemPackageProviderDescription.swift | 2 +- .../TargetBuildSettingDescription.swift | 9 +- .../Manifest/TargetDescription.swift | 21 +- Sources/PackageModel/PackageModel.swift | 10 + Sources/PackageModel/PackageReference.swift | 2 +- Sources/PackageModel/Platform.swift | 8 +- .../RegistryReleaseMetadata.swift | 14 +- .../PackageModel/SwiftLanguageVersion.swift | 2 +- Sources/PackageModel/Target/Target.swift | 3 +- .../Plugins/PluginContextSerializer.swift | 13 +- .../Plugins/PluginInvocation.swift | 11 +- Sources/Workspace/Workspace.swift | 4 +- Sources/XCBuildSupport/PIFBuilder.swift | 30 +-- Tests/FunctionalTests/PluginTests.swift | 10 +- .../PluginInvocationTests.swift | 6 +- 44 files changed, 458 insertions(+), 322 deletions(-) diff --git a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift index 5bc56f647f3..6e4af61226b 100644 --- a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift @@ -27,7 +27,7 @@ public final class ClangTargetBuildDescription { /// The underlying clang target. public var clangTarget: ClangTarget { - target.underlyingTarget as! ClangTarget + target.underlying as! ClangTarget } /// The tools version of the package that declared the target. This can @@ -45,7 +45,7 @@ public final class ClangTargetBuildDescription { /// The list of all resource files in the target, including the derived ones. public var resources: [Resource] { - self.target.underlyingTarget.resources + self.pluginDerivedResources + self.target.underlying.resources + self.pluginDerivedResources } /// Path to the bundle generated for this module (if any). @@ -54,7 +54,7 @@ public final class ClangTargetBuildDescription { return .none } - if let bundleName = target.underlyingTarget.potentialBundleName { + if let bundleName = target.underlying.potentialBundleName { return self.buildParameters.bundlePath(named: bundleName) } else { return .none @@ -119,7 +119,7 @@ public final class ClangTargetBuildDescription { fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws { - guard target.underlyingTarget is ClangTarget else { + guard target.underlying is ClangTarget else { throw InternalError("underlying target type mismatch \(target)") } diff --git a/Sources/Build/BuildDescription/PluginDescription.swift b/Sources/Build/BuildDescription/PluginDescription.swift index 82abf9a3b3e..03a8d62dc0b 100644 --- a/Sources/Build/BuildDescription/PluginDescription.swift +++ b/Sources/Build/BuildDescription/PluginDescription.swift @@ -50,7 +50,7 @@ public final class PluginDescription: Codable { testDiscoveryTarget: Bool = false, fileSystem: FileSystem ) throws { - guard target.underlyingTarget is PluginTarget else { + guard target.underlying is PluginTarget else { throw InternalError("underlying target type mismatch \(target)") } diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index a779f548070..c628be7fe41 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -234,7 +234,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription // Support for linking tests against executables is conditional on the tools // version of the package that defines the executable product. let executableTarget = try product.executableTarget - if let target = executableTarget.underlyingTarget as? SwiftTarget, + if let target = executableTarget.underlying as? SwiftTarget, self.toolsVersion >= .v5_5, self.buildParameters.driverParameters.canRenameEntrypointFunctionName, target.supportsTestableExecutablesFeature diff --git a/Sources/Build/BuildDescription/SharedTargetBuildDescription.swift b/Sources/Build/BuildDescription/SharedTargetBuildDescription.swift index f159669db86..a4c0e467676 100644 --- a/Sources/Build/BuildDescription/SharedTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SharedTargetBuildDescription.swift @@ -53,7 +53,7 @@ struct SharedTargetBuildDescription { additionalFileRules: additionalFileRules, defaultLocalization: target.defaultLocalization, targetName: target.name, - targetPath: target.underlyingTarget.path, + targetPath: target.underlying.path, observabilityScope: observabilityScope ) let pluginDerivedResources = derivedResources diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index 1816daef37a..3a53b1004e4 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -57,7 +57,7 @@ public final class SwiftTargetBuildDescription { /// Path to the bundle generated for this module (if any). var bundlePath: AbsolutePath? { - if let bundleName = target.underlyingTarget.potentialBundleName, needsResourceBundle { + if let bundleName = target.underlying.potentialBundleName, needsResourceBundle { return self.buildParameters.bundlePath(named: bundleName) } else { return .none @@ -83,7 +83,7 @@ public final class SwiftTargetBuildDescription { /// The list of all resource files in the target, including the derived ones. public var resources: [Resource] { - self.target.underlyingTarget.resources + self.pluginDerivedResources + self.target.underlying.resources + self.pluginDerivedResources } /// The objects in this target, containing either machine code or bitcode @@ -136,7 +136,7 @@ public final class SwiftTargetBuildDescription { /// The swift version for this target. var swiftVersion: SwiftLanguageVersion { - (self.target.underlyingTarget as! SwiftTarget).swiftVersion + (self.target.underlying as! SwiftTarget).swiftVersion } /// Describes the purpose of a test target, including any special roles such as containing a list of discovered @@ -250,7 +250,7 @@ public final class SwiftTargetBuildDescription { fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws { - guard target.underlyingTarget is SwiftTarget else { + guard target.underlying is SwiftTarget else { throw InternalError("underlying target type mismatch \(target)") } @@ -494,7 +494,7 @@ public final class SwiftTargetBuildDescription { // when we link the executable, we will ask the linker to rename the entry point // symbol to just `_main` again (or if the linker doesn't support it, we'll // generate a source containing a redirect). - if (self.target.underlyingTarget as? SwiftTarget)?.supportsTestableExecutablesFeature == true + if (self.target.underlying as? SwiftTarget)?.supportsTestableExecutablesFeature == true && !self.isTestTarget && self.toolsVersion >= .v5_5 { // We only do this if the linker supports it, as indicated by whether we diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift index 11403ec9ed7..f2ca6f6c7b4 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift @@ -212,8 +212,8 @@ extension LLBuildManifestBuilder { // product into its constituent targets. continue } - guard target.underlyingTarget.type != .systemModule, - target.underlyingTarget.type != .binary + guard target.underlying.type != .systemModule, + target.underlying.type != .binary else { // Much like non-Swift targets, system modules will consist of a modulemap // somewhere in the filesystem, with the path to that module being either @@ -410,11 +410,11 @@ extension LLBuildManifestBuilder { func addStaticTargetInputs(_ target: ResolvedTarget) throws { // Ignore C Modules. - if target.underlyingTarget is SystemLibraryTarget { return } + if target.underlying is SystemLibraryTarget { return } // Ignore Binary Modules. - if target.underlyingTarget is BinaryTarget { return } + if target.underlying is BinaryTarget { return } // Ignore Plugin Targets. - if target.underlyingTarget is PluginTarget { return } + if target.underlying is PluginTarget { return } // Depend on the binary for executable targets. if target.type == .executable { diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index 94b8328502a..f1db9c79198 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -498,7 +498,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS for package in graph.rootPackages where package.manifest.toolsVersion >= .v5_3 { for target in package.targets { // Get the set of unhandled files in targets. - var unhandledFiles = Set(target.underlyingTarget.others) + var unhandledFiles = Set(target.underlying.others) if unhandledFiles.isEmpty { continue } // Subtract out any that were inputs to any commands generated by plugins. diff --git a/Sources/Build/BuildPlan/BuildPlan+Clang.swift b/Sources/Build/BuildPlan/BuildPlan+Clang.swift index 616fdf9b85b..fa5ed58da50 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Clang.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Clang.swift @@ -19,7 +19,7 @@ extension BuildPlan { /// Plan a Clang target. func plan(clangTarget: ClangTargetBuildDescription) throws { for case .target(let dependency, _) in try clangTarget.target.recursiveDependencies(satisfying: buildEnvironment) { - switch dependency.underlyingTarget { + switch dependency.underlying { case is SwiftTarget: if case let .swift(dependencyTargetDescription)? = targetMap[dependency] { if let moduleMap = dependencyTargetDescription.moduleMap { diff --git a/Sources/Build/BuildPlan/BuildPlan+Product.swift b/Sources/Build/BuildPlan/BuildPlan+Product.swift index 78af6a044a5..aef0f87a27f 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Product.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Product.swift @@ -30,7 +30,7 @@ extension BuildPlan { // Add flags for system targets. for systemModule in dependencies.systemModules { - guard case let target as SystemLibraryTarget = systemModule.underlyingTarget else { + guard case let target as SystemLibraryTarget = systemModule.underlying else { throw InternalError("This should not be possible.") } // Add pkgConfig libs arguments. @@ -51,7 +51,7 @@ extension BuildPlan { // Link C++ if needed. // Note: This will come from build settings in future. for target in dependencies.staticTargets { - if case let target as ClangTarget = target.underlyingTarget, target.isCXX { + if case let target as ClangTarget = target.underlying, target.isCXX { if buildParameters.targetTriple.isDarwin() { buildProduct.additionalFlags += ["-lc++"] } else if buildParameters.targetTriple.isWindows() { @@ -64,7 +64,7 @@ extension BuildPlan { } for target in dependencies.staticTargets { - switch target.underlyingTarget { + switch target.underlying { case is SwiftTarget: // Swift targets are guaranteed to have a corresponding Swift description. guard case .swift(let description) = targetMap[target] else { @@ -127,7 +127,7 @@ extension BuildPlan { // For test targets, we need to consider the first level of transitive dependencies since the first level is always test targets. let topLevelDependencies: [PackageModel.Target] if product.type == .test { - topLevelDependencies = product.targets.flatMap { $0.underlyingTarget.dependencies }.compactMap { + topLevelDependencies = product.targets.flatMap { $0.underlying.dependencies }.compactMap { switch $0 { case .product: return nil @@ -145,7 +145,7 @@ extension BuildPlan { switch dependency { // Include all the dependencies of a target. case .target(let target, _): - let isTopLevel = topLevelDependencies.contains(target.underlyingTarget) || product.targets.contains(target) + let isTopLevel = topLevelDependencies.contains(target.underlying) || product.targets.contains(target) let topLevelIsMacro = isTopLevel && product.type == .macro let topLevelIsPlugin = isTopLevel && product.type == .plugin let topLevelIsTest = isTopLevel && product.type == .test @@ -196,9 +196,9 @@ extension BuildPlan { case .executable, .snippet, .macro: if product.targets.contains(target) { staticTargets.append(target) - } else if product.type == .test && (target.underlyingTarget as? SwiftTarget)?.supportsTestableExecutablesFeature == true { + } else if product.type == .test && (target.underlying as? SwiftTarget)?.supportsTestableExecutablesFeature == true { // Only "top-level" targets should really be considered here, not transitive ones. - let isTopLevel = topLevelDependencies.contains(target.underlyingTarget) || product.targets.contains(target) + let isTopLevel = topLevelDependencies.contains(target.underlying) || product.targets.contains(target) if let toolsVersion = graph.package(for: product)?.manifest.toolsVersion, toolsVersion >= .v5_5, isTopLevel { staticTargets.append(target) } @@ -216,7 +216,7 @@ extension BuildPlan { systemModules.append(target) // Add binary to binary paths set. case .binary: - guard let binaryTarget = target.underlyingTarget as? BinaryTarget else { + guard let binaryTarget = target.underlying as? BinaryTarget else { throw InternalError("invalid binary target '\(target.name)'") } switch binaryTarget.kind { diff --git a/Sources/Build/BuildPlan/BuildPlan+Swift.swift b/Sources/Build/BuildPlan/BuildPlan+Swift.swift index c5183f4d72c..9a79fb5d460 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Swift.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Swift.swift @@ -20,7 +20,7 @@ extension BuildPlan { // We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target // depends on. for case .target(let dependency, _) in try swiftTarget.target.recursiveDependencies(satisfying: buildEnvironment) { - switch dependency.underlyingTarget { + switch dependency.underlying { case let underlyingTarget as ClangTarget where underlyingTarget.type == .library: guard case let .clang(target)? = targetMap[dependency] else { throw InternalError("unexpected clang target \(underlyingTarget)") diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index 70c0faee9e5..945f9c9fed8 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -58,7 +58,7 @@ extension BuildPlan { // tests into a separate target/module named "PackageDiscoveredTests". Then, that entry point file may import that module and // obtain that list to pass it to the `XCTMain(...)` function and avoid needing to maintain a list of tests itself. if testProduct.testEntryPointTarget != nil && explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly { - let testEntryPointName = testProduct.underlyingProduct.testEntryPointPath?.basename ?? SwiftTarget.defaultTestEntryPointName + let testEntryPointName = testProduct.underlying.testEntryPointPath?.basename ?? SwiftTarget.defaultTestEntryPointName observabilityScope.emit(warning: "'--enable-test-discovery' was specified so the '\(testEntryPointName)' entry point file for '\(testProduct.name)' will be ignored and an entry point will be generated automatically. To use test discovery with a custom entry point file, pass '--experimental-test-entry-point-path '.") } else if testProduct.testEntryPointTarget == nil, let testEntryPointPath = explicitlySpecifiedPath, !fileSystem.exists(testEntryPointPath) { observabilityScope.emit(error: "'--experimental-test-entry-point-path' was specified but the file '\(testEntryPointPath)' could not be found.") @@ -79,7 +79,7 @@ extension BuildPlan { let discoveryTarget = SwiftTarget( name: discoveryTargetName, - dependencies: testProduct.underlyingProduct.targets.map { .target($0, conditions: []) }, + dependencies: testProduct.underlying.targets.map { .target($0, conditions: []) }, packageAccess: true, // test target is allowed access to package decls by default testDiscoverySrc: Sources(paths: discoveryPaths, root: discoveryDerivedDir) ) @@ -113,7 +113,7 @@ extension BuildPlan { let entryPointTarget = SwiftTarget( name: testProduct.name, type: .library, - dependencies: testProduct.underlyingProduct.targets.map { .target($0, conditions: []) } + [.target(discoveryTarget, conditions: [])], + dependencies: testProduct.underlying.targets.map { .target($0, conditions: []) } + [.target(discoveryTarget, conditions: [])], packageAccess: true, // test target is allowed access to package decls testEntryPointSources: entryPointSources ) @@ -142,10 +142,10 @@ extension BuildPlan { if isEntryPointPathSpecifiedExplicitly { // Allow using the explicitly-specified test entry point target, but still perform test discovery and thus declare a dependency on the discovery targets. let entryPointTarget = SwiftTarget( - name: entryPointResolvedTarget.underlyingTarget.name, - dependencies: entryPointResolvedTarget.underlyingTarget.dependencies + [.target(discoveryTargets.target, conditions: [])], + name: entryPointResolvedTarget.underlying.name, + dependencies: entryPointResolvedTarget.underlying.dependencies + [.target(discoveryTargets.target, conditions: [])], packageAccess: entryPointResolvedTarget.packageAccess, - testEntryPointSources: entryPointResolvedTarget.underlyingTarget.sources + testEntryPointSources: entryPointResolvedTarget.underlying.sources ) let entryPointResolvedTarget = ResolvedTarget( target: entryPointTarget, diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index a076bddb7b8..0a90242e3a8 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -158,7 +158,7 @@ extension BuildParameters { /// Returns the scoped view of build settings for a given target. func createScope(for target: ResolvedTarget) -> BuildSettings.Scope { - return BuildSettings.Scope(target.underlyingTarget.buildSettings, environment: buildEnvironment) + return BuildSettings.Scope(target.underlying.buildSettings, environment: buildEnvironment) } } @@ -307,13 +307,13 @@ public class BuildPlan: SPMBuildCore.BuildPlan { // This can affect what flags to pass and other semantics. let toolsVersion = graph.package(for: target)?.manifest.toolsVersion ?? .v5_5 - switch target.underlyingTarget { + switch target.underlying { case is SwiftTarget: guard let package = graph.package(for: target) else { throw InternalError("package not found for \(target)") } - let requiredMacroProducts = try target.recursiveTargetDependencies().filter { $0.underlyingTarget.type == .macro }.compactMap { macroProductsByTarget[$0] } + let requiredMacroProducts = try target.recursiveTargetDependencies().filter { $0.underlying.type == .macro }.compactMap { macroProductsByTarget[$0] } var generateTestObservation = false if target.type == .test && shouldGenerateTestObservation { @@ -358,7 +358,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { case is SystemLibraryTarget, is BinaryTarget: break default: - throw InternalError("unhandled \(target.underlyingTarget)") + throw InternalError("unhandled \(target.underlying)") } } @@ -480,7 +480,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { // Add search paths from the system library targets. for target in graph.reachableTargets { - if let systemLib = target.underlyingTarget as? SystemLibraryTarget { + if let systemLib = target.underlying as? SystemLibraryTarget { arguments.append(contentsOf: try self.pkgConfig(for: systemLib).cFlags) // Add the path to the module map. arguments += ["-I", systemLib.moduleMapPath.parentDirectory.pathString] @@ -516,7 +516,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { // Add search paths from the system library targets. for target in graph.reachableTargets { - if let systemLib = target.underlyingTarget as? SystemLibraryTarget { + if let systemLib = target.underlying as? SystemLibraryTarget { arguments += try self.pkgConfig(for: systemLib).cFlags } } @@ -638,7 +638,7 @@ extension Basics.Triple { extension ResolvedPackage { var isRemote: Bool { - switch self.underlyingPackage.manifest.packageKind { + switch self.underlying.manifest.packageKind { case .registry, .remoteSourceControl, .localSourceControl: return true case .root, .fileSystem: @@ -653,7 +653,7 @@ extension ResolvedProduct { } private var isBinaryOnly: Bool { - return self.targets.filter({ !($0.underlyingTarget is BinaryTarget) }).isEmpty + return self.targets.filter({ !($0.underlying is BinaryTarget) }).isEmpty } private var isPlugin: Bool { diff --git a/Sources/Commands/PackageTools/APIDiff.swift b/Sources/Commands/PackageTools/APIDiff.swift index 3f60f0204cd..e48002a1545 100644 --- a/Sources/Commands/PackageTools/APIDiff.swift +++ b/Sources/Commands/PackageTools/APIDiff.swift @@ -176,7 +176,7 @@ struct APIDiff: SwiftCommand { observabilityScope.emit(error: "'\(productName)' is not a library product") continue } - modulesToDiff.formUnion(product.targets.filter { $0.underlyingTarget is SwiftTarget }.map(\.c99name)) + modulesToDiff.formUnion(product.targets.filter { $0.underlying is SwiftTarget }.map(\.c99name)) } for targetName in targets { guard let target = packageGraph @@ -190,7 +190,7 @@ struct APIDiff: SwiftCommand { observabilityScope.emit(error: "'\(targetName)' is not a library target") continue } - guard target.underlyingTarget is SwiftTarget else { + guard target.underlying is SwiftTarget else { observabilityScope.emit(error: "'\(targetName)' is not a Swift language target") continue } diff --git a/Sources/Commands/PackageTools/CompletionTool.swift b/Sources/Commands/PackageTools/CompletionTool.swift index b7423bd29f6..56f9ddbe517 100644 --- a/Sources/Commands/PackageTools/CompletionTool.swift +++ b/Sources/Commands/PackageTools/CompletionTool.swift @@ -69,14 +69,14 @@ extension SwiftPackageTool { ShowDependencies.dumpDependenciesOf(rootPackage: graph.rootPackages[0], mode: .flatlist, on: TSCBasic.stdoutStream) case .listExecutables: let graph = try swiftTool.loadPackageGraph() - let package = graph.rootPackages[0].underlyingPackage + let package = graph.rootPackages[0].underlying let executables = package.targets.filter { $0.type == .executable } for executable in executables { print(executable.name) } case .listSnippets: let graph = try swiftTool.loadPackageGraph() - let package = graph.rootPackages[0].underlyingPackage + let package = graph.rootPackages[0].underlying let executables = package.targets.filter { $0.type == .snippet } for executable in executables { print(executable.name) diff --git a/Sources/Commands/PackageTools/InstalledPackages.swift b/Sources/Commands/PackageTools/InstalledPackages.swift index fb6b5bf9640..dc834a4fdb8 100644 --- a/Sources/Commands/PackageTools/InstalledPackages.swift +++ b/Sources/Commands/PackageTools/InstalledPackages.swift @@ -62,7 +62,7 @@ extension SwiftPackageTool { case 0: throw StringError("No Executable Products in Package.swift.") case 1: - productToInstall = possibleCandidates[0].underlyingProduct + productToInstall = possibleCandidates[0].underlying default: guard let product, let first = possibleCandidates.first(where: { $0.name == product }) else { throw StringError( @@ -73,7 +73,7 @@ extension SwiftPackageTool { ) } - productToInstall = first.underlyingProduct + productToInstall = first.underlying } if let existingPkg = alreadyExisting.first(where: { $0.name == productToInstall.name }) { diff --git a/Sources/Commands/PackageTools/PluginCommand.swift b/Sources/Commands/PackageTools/PluginCommand.swift index 648656ebc6a..57f3e16098c 100644 --- a/Sources/Commands/PackageTools/PluginCommand.swift +++ b/Sources/Commands/PackageTools/PluginCommand.swift @@ -312,7 +312,7 @@ struct PluginCommand: SwiftCommand { let directDependencyPluginTargets = directDependencyPackages.flatMap { $0.products.filter { $0.type == .plugin } }.flatMap { $0.targets } // As well as any plugin targets in root packages. let rootPackageTargets = graph.rootPackages.filter { $0.matching(identity: packageIdentity) }.flatMap { $0.targets } - return (directDependencyPluginTargets + rootPackageTargets).compactMap { $0.underlyingTarget as? PluginTarget }.filter { + return (directDependencyPluginTargets + rootPackageTargets).compactMap { $0.underlying as? PluginTarget }.filter { switch $0.capability { case .buildTool: return false case .command: return true diff --git a/Sources/Commands/Utilities/APIDigester.swift b/Sources/Commands/Utilities/APIDigester.swift index 6b67692418f..c0f789d80a7 100644 --- a/Sources/Commands/Utilities/APIDigester.swift +++ b/Sources/Commands/Utilities/APIDigester.swift @@ -311,7 +311,7 @@ extension PackageGraph { .flatMap(\.products) .filter { $0.type.isLibrary } .flatMap(\.targets) - .filter { $0.underlyingTarget is SwiftTarget } + .filter { $0.underlying is SwiftTarget } .map { $0.c99name } } } diff --git a/Sources/Commands/Utilities/PluginDelegate.swift b/Sources/Commands/Utilities/PluginDelegate.swift index f0378edd23b..9cbba1b3589 100644 --- a/Sources/Commands/Utilities/PluginDelegate.swift +++ b/Sources/Commands/Utilities/PluginDelegate.swift @@ -18,6 +18,7 @@ import SPMBuildCore import class TSCBasic.BufferedOutputByteStream import class TSCBasic.Process +import struct TSCBasic.ProcessResult final class PluginDelegate: PluginInvocationDelegate { let swiftTool: SwiftTool @@ -376,7 +377,7 @@ final class PluginDelegate: PluginInvocationDelegate { try swiftTool.fileSystem.removeFileTree(outputDir) // Run the symbol graph extractor on the target. - try symbolGraphExtractor.extractSymbolGraph( + let result = try symbolGraphExtractor.extractSymbolGraph( target: target, buildPlan: try buildSystem.buildPlan, outputRedirection: .collect, @@ -384,6 +385,10 @@ final class PluginDelegate: PluginInvocationDelegate { verboseOutput: self.swiftTool.logLevel <= .info ) + guard result.exitStatus == .terminated(code: 0) else { + throw ProcessResult.Error.nonZeroExit(result) + } + // Return the results to the plugin. return PluginInvocationSymbolGraphResult(directoryPath: outputDir.pathString) } diff --git a/Sources/PackageGraph/ModuleAliasTracker.swift b/Sources/PackageGraph/ModuleAliasTracker.swift index b57aac58917..9929bd7ecd8 100644 --- a/Sources/PackageGraph/ModuleAliasTracker.swift +++ b/Sources/PackageGraph/ModuleAliasTracker.swift @@ -15,9 +15,9 @@ import Basics // This is a helper class that tracks module aliases in a package dependency graph // and handles overriding upstream aliases where aliases themselves conflict. -class ModuleAliasTracker { - var aliasMap = [String: [ModuleAliasModel]]() - var idToAliasMap = [PackageIdentity: [String: [ModuleAliasModel]]]() +struct ModuleAliasTracker { + fileprivate var aliasMap = [String: [ModuleAliasModel]]() + fileprivate var idToAliasMap = [PackageIdentity: [String: [ModuleAliasModel]]]() var idToProductToAllTargets = [PackageIdentity: [String: [Target]]]() var productToDirectTargets = [String: [Target]]() var productToAllTargets = [String: [Target]]() @@ -27,7 +27,7 @@ class ModuleAliasTracker { var appliedAliases = Set() init() {} - func addTargetAliases(targets: [Target], package: PackageIdentity) throws { + mutating func addTargetAliases(targets: [Target], package: PackageIdentity) throws { let targetDependencies = targets.map{$0.dependencies}.flatMap{$0} for dep in targetDependencies { if case let .product(productRef, _) = dep, @@ -47,11 +47,13 @@ class ModuleAliasTracker { } } - func addAliases(_ aliases: [String: String], - productID: String, - productName: String, - originPackage: PackageIdentity, - consumingPackage: PackageIdentity) throws { + mutating func addAliases( + _ aliases: [String: String], + productID: String, + productName: String, + originPackage: PackageIdentity, + consumingPackage: PackageIdentity + ) throws { if let aliasDict = idToAliasMap[originPackage] { let existingAliases = aliasDict.values.flatMap{$0}.filter { aliases.keys.contains($0.name) } for existingAlias in existingAliases { @@ -70,8 +72,7 @@ class ModuleAliasTracker { } } - func addPackageIDChain(parent: PackageIdentity, - child: PackageIdentity) { + mutating func addPackageIDChain(parent: PackageIdentity, child: PackageIdentity) { if parentToChildIDs[parent]?.contains(child) ?? false { // Already added } else { @@ -82,8 +83,7 @@ class ModuleAliasTracker { } // This func should be called once per product - func trackTargetsPerProduct(product: Product, - package: PackageIdentity) { + mutating func trackTargetsPerProduct(product: Product, package: PackageIdentity) { let targetDeps = product.targets.map{$0.dependencies}.flatMap{$0} var allTargetDeps = product.targets.map{$0.recursiveDependentTargets.map{$0.dependencies}}.flatMap{$0}.flatMap{$0} allTargetDeps.append(contentsOf: targetDeps) @@ -114,7 +114,7 @@ class ModuleAliasTracker { } } - func propagateAliases(observabilityScope: ObservabilityScope) { + mutating func propagateAliases(observabilityScope: ObservabilityScope) { // First get the root package ID var pkgID = childToParentID.first?.key var rootPkg = pkgID @@ -144,9 +144,11 @@ class ModuleAliasTracker { // Propagate defined aliases upstream. If they are chained, the final // alias value will be applied - func propagate(productID: String, - observabilityScope: ObservabilityScope, - aliasBuffer: inout [String: ModuleAliasModel]) { + mutating private func propagate( + productID: String, + observabilityScope: ObservabilityScope, + aliasBuffer: inout [String: ModuleAliasModel] + ) { let productAliases = aliasMap[productID] ?? [] for aliasModel in productAliases { // Alias buffer is used to carry down aliases defined upstream @@ -197,8 +199,7 @@ class ModuleAliasTracker { } // Merge all the upstream aliases and override them if necessary - func merge(productID: String, - observabilityScope: ObservabilityScope) { + mutating func merge(productID: String, observabilityScope: ObservabilityScope) { guard let children = parentToChildProducts[productID] else { return } @@ -259,7 +260,7 @@ class ModuleAliasTracker { // chain but not in a product consumed by other packages. Such targets still // need to have aliases applied to them so they can be built with correct // dependent binary names - func fillInRest(package: PackageIdentity) { + mutating func fillInRest(package: PackageIdentity) { if let productToTargets = idToProductToAllTargets[package] { for (_, productTargets) in productToTargets { let unAliased = productTargets.contains{$0.moduleAliases == nil} @@ -290,13 +291,15 @@ class ModuleAliasTracker { } } - private func chainModuleAliases(targets: [Target], - checkedTargets: [Target], - targetAliases: [String: [String]], - childTargets: [Target], - childAliases: [String: [String]], - childPrechainAliases: [String: [String]], - observabilityScope: ObservabilityScope) { + private mutating func chainModuleAliases( + targets: [Target], + checkedTargets: [Target], + targetAliases: [String: [String]], + childTargets: [Target], + childAliases: [String: [String]], + childPrechainAliases: [String: [String]], + observabilityScope: ObservabilityScope + ) { guard !targets.isEmpty else { return } var aliasDict = [String: String]() var prechainAliasDict = [String: [String]]() @@ -419,7 +422,7 @@ class ModuleAliasTracker { } // Used to keep track of module alias info for each package -class ModuleAliasModel { +private class ModuleAliasModel { let name: String var alias: String let originPackage: PackageIdentity diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index 5b81ba146f2..31395de1a93 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -19,7 +19,6 @@ import func TSCBasic.topologicalSort import func TSCBasic.bestMatch extension PackageGraph { - /// Load the package graph for the given package path. public static func load( root: PackageGraphRoot, @@ -206,7 +205,10 @@ private func checkAllDependenciesAreUsed(_ rootPackages: [ResolvedPackage], obse } // Make sure that any diagnostics we emit below are associated with the package. - let packageDiagnosticsScope = observabilityScope.makeChildScope(description: "Package Dependency Validation", metadata: package.underlyingPackage.diagnosticsMetadata) + let packageDiagnosticsScope = observabilityScope.makeChildScope( + description: "Package Dependency Validation", + metadata: package.underlying.diagnosticsMetadata + ) // Otherwise emit a warning if none of the dependency package's products are used. let dependencyIsUsed = dependency.products.contains(where: productDependencies.contains) @@ -221,7 +223,7 @@ fileprivate extension ResolvedProduct { /// Returns true if and only if the product represents a command plugin target. var isCommandPlugin: Bool { guard type == .plugin else { return false } - guard let target = underlyingProduct.targets.compactMap({ $0 as? PluginTarget }).first else { return false } + guard let target = underlying.targets.compactMap({ $0 as? PluginTarget }).first else { return false } guard case .command = target.capability else { return false } return true } @@ -776,7 +778,7 @@ private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder], } guard hasAliases else { return false } - let aliasTracker = ModuleAliasTracker() + var aliasTracker = ModuleAliasTracker() for packageBuilder in packageBuilders { try aliasTracker.addTargetAliases(targets: packageBuilder.package.targets, package: packageBuilder.package.identity) @@ -861,7 +863,6 @@ private final class ResolvedProductBuilder: ResolvedBuilder { /// Builder for resolved target. private final class ResolvedTargetBuilder: ResolvedBuilder { - /// Enumeration to represent target dependencies. enum Dependency { @@ -875,9 +876,6 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { /// The target reference. let target: Target - /// DiagnosticsEmitter with which to emit diagnostics - let diagnosticsEmitter: DiagnosticsEmitter - /// The target dependencies of this target. var dependencies: [Dependency] = [] @@ -887,6 +885,7 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { /// The platforms supported by this package. var supportedPlatforms: [SupportedPlatform] = [] + let observabilityScope: ObservabilityScope let platformVersionProvider: PlatformVersionProvider init( @@ -895,24 +894,17 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { platformVersionProvider: PlatformVersionProvider ) { self.target = target - self.diagnosticsEmitter = observabilityScope.makeDiagnosticsEmitter() { - var metadata = ObservabilityMetadata() - metadata.targetName = target.name - return metadata - } + self.observabilityScope = observabilityScope self.platformVersionProvider = platformVersionProvider } - func diagnoseInvalidUseOfUnsafeFlags(_ product: ResolvedProduct) throws { - // Diagnose if any target in this product uses an unsafe flag. - for target in try product.recursiveTargetDependencies() { - if target.underlyingTarget.usesUnsafeFlags { - self.diagnosticsEmitter.emit(.productUsesUnsafeFlags(product: product.name, target: target.name)) - } + override func constructImpl() throws -> ResolvedTarget { + let diagnosticsEmitter = self.observabilityScope.makeDiagnosticsEmitter() { + var metadata = ObservabilityMetadata() + metadata.targetName = target.name + return metadata } - } - override func constructImpl() throws -> ResolvedTarget { let dependencies = try self.dependencies.map { dependency -> ResolvedTarget.Dependency in switch dependency { case .target(let targetBuilder, let conditions): @@ -925,17 +917,19 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { ) let product = try productBuilder.construct() if !productBuilder.packageBuilder.isAllowedToVendUnsafeProducts { - try self.diagnoseInvalidUseOfUnsafeFlags(product) + try product.diagnoseInvalidUseOfUnsafeFlags(diagnosticsEmitter) } return .product(product, conditions: conditions) } } return ResolvedTarget( - target: self.target, - dependencies: dependencies, - defaultLocalization: self.defaultLocalization, - supportedPlatforms: self.supportedPlatforms, + storage: .init( + underlying: self.target, + dependencies: dependencies, + defaultLocalization: self.defaultLocalization, + supportedPlatforms: self.platforms + ), platformVersionProvider: self.platformVersionProvider ) } diff --git a/Sources/PackageGraph/PackageGraphRoot.swift b/Sources/PackageGraph/PackageGraphRoot.swift index 88ecbf49f65..3a0e8058857 100644 --- a/Sources/PackageGraph/PackageGraphRoot.swift +++ b/Sources/PackageGraph/PackageGraphRoot.swift @@ -102,6 +102,7 @@ public struct PackageGraphRoot { // at which time the current special casing can be deprecated. var adjustedDependencies = input.dependencies if let explicitProduct { + // FIXME: `dependenciesRequired` modifies manifests and prevents conversion of `Manifest` to a value type for dependency in manifests.values.lazy.map({ $0.dependenciesRequired(for: .everything) }).joined() { adjustedDependencies.append(dependency.filtered(by: .specific([explicitProduct]))) } diff --git a/Sources/PackageGraph/Resolution/ResolvedPackage.swift b/Sources/PackageGraph/Resolution/ResolvedPackage.swift index 72916172972..bb8078d2f2b 100644 --- a/Sources/PackageGraph/Resolution/ResolvedPackage.swift +++ b/Sources/PackageGraph/Resolution/ResolvedPackage.swift @@ -15,67 +15,105 @@ import PackageModel /// A fully resolved package. Contains resolved targets, products and dependencies of the package. public final class ResolvedPackage { - /// The underlying package reference. - public let underlyingPackage: Package - // The identity of the package. public var identity: PackageIdentity { - return self.underlyingPackage.identity + return self.storage.underlying.identity } /// The manifest describing the package. public var manifest: Manifest { - return self.underlyingPackage.manifest + return self.storage.underlying.manifest } /// The local path of the package. public var path: AbsolutePath { - return self.underlyingPackage.path + return self.storage.underlying.path + } + + /// The underlying package model. + public var underlying: Package { + self.storage.underlying } /// The targets contained in the package. - public let targets: [ResolvedTarget] + public var targets: [ResolvedTarget] { + self.storage.targets + } /// The products produced by the package. - public let products: [ResolvedProduct] + public var products: [ResolvedProduct] { + self.storage.products + } /// The dependencies of the package. - public let dependencies: [ResolvedPackage] + public var dependencies: [ResolvedPackage] { + self.storage.dependencies + } - /// The default localization for resources. - public let defaultLocalization: String? + /// If the given package's source is a registry release, this provides additional metadata and signature information. + public var registryMetadata: RegistryReleaseMetadata? { + self.storage.registryMetadata + } - /// The list of platforms that are supported by this target. - public let supportedPlatforms: [SupportedPlatform] + struct Storage: Hashable { + /// The underlying package reference. + public let underlying: Package + + /// The targets contained in the package. + public let targets: [ResolvedTarget] + + /// The products produced by the package. + public let products: [ResolvedProduct] + + /// The dependencies of the package. + public let dependencies: [ResolvedPackage] + + /// The default localization for resources. + public let defaultLocalization: String? + + /// The list of platforms that are supported by this target. + public let platforms: [SupportedPlatform] + + /// If the given package's source is a registry release, this provides additional metadata and signature information. + public let registryMetadata: RegistryReleaseMetadata? + } - /// If the given package's source is a registry release, this provides additional metadata and signature information. - public let registryMetadata: RegistryReleaseMetadata? + private let storage: Storage private let platformVersionProvider: PlatformVersionProvider - public init( + public convenience init( package: Package, defaultLocalization: String?, - supportedPlatforms: [SupportedPlatform], + platforms: [SupportedPlatform], dependencies: [ResolvedPackage], targets: [ResolvedTarget], products: [ResolvedProduct], registryMetadata: RegistryReleaseMetadata?, platformVersionProvider: PlatformVersionProvider ) { - self.underlyingPackage = package - self.defaultLocalization = defaultLocalization - self.supportedPlatforms = supportedPlatforms - self.dependencies = dependencies - self.targets = targets - self.products = products - self.registryMetadata = registryMetadata + self.init( + storage: .init( + underlying: package, + targets: targets, + products: products, + dependencies: dependencies, + defaultLocalization: defaultLocalization, + platforms: platforms, + registryMetadata: registryMetadata + ), + platformVersionProvider: platformVersionProvider + ) + } + + init(storage: Storage, platformVersionProvider: PlatformVersionProvider) { + self.storage = storage self.platformVersionProvider = platformVersionProvider - } + } public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.supportedPlatforms, + declared: self.storage.platforms, for: platform, usingXCTest: usingXCTest ) @@ -84,11 +122,11 @@ public final class ResolvedPackage { extension ResolvedPackage: Hashable { public func hash(into hasher: inout Hasher) { - hasher.combine(ObjectIdentifier(self)) + hasher.combine(self.storage) } public static func == (lhs: ResolvedPackage, rhs: ResolvedPackage) -> Bool { - ObjectIdentifier(lhs) == ObjectIdentifier(rhs) + lhs.storage == rhs.storage } } diff --git a/Sources/PackageGraph/Resolution/ResolvedProduct.swift b/Sources/PackageGraph/Resolution/ResolvedProduct.swift index e677c5cc1a1..7c11e5d2f65 100644 --- a/Sources/PackageGraph/Resolution/ResolvedProduct.swift +++ b/Sources/PackageGraph/Resolution/ResolvedProduct.swift @@ -14,135 +14,209 @@ import Basics import PackageModel public final class ResolvedProduct { - /// The underlying product. - public let underlyingProduct: Product - /// The name of this product. public var name: String { - return underlyingProduct.name + self.storage.underlying.name } - /// The top level targets contained in this product. - public let targets: [ResolvedTarget] - /// The type of this product. public var type: ProductType { - return underlyingProduct.type + self.storage.underlying.type + } + + /// The underlying product model. + public var underlying: Product { + self.storage.underlying + } + + /// The top level targets contained in this product. + public var targets: [ResolvedTarget] { + self.storage.targets } /// Executable target for test entry point file. - public let testEntryPointTarget: ResolvedTarget? + public var testEntryPointTarget: ResolvedTarget? { + self.storage.testEntryPointTarget + } /// The default localization for resources. - public let defaultLocalization: String? + public var defaultLocalization: String? { + self.storage.defaultLocalization + } /// The list of platforms that are supported by this product. - public let supportedPlatforms: [SupportedPlatform] + public var platforms: [SupportedPlatform] { + self.storage.platforms + } public let platformVersionProvider: PlatformVersionProvider + struct Storage: Hashable { + /// The underlying product. + let underlying: Product + + /// The top level targets contained in this product. + let targets: [ResolvedTarget] + + /// Executable target for test entry point file. + let testEntryPointTarget: ResolvedTarget? + + /// The default localization for resources. + let defaultLocalization: String? + + /// The list of platforms that are supported by this product. + let platforms: [SupportedPlatform] + } + + private let storage: Storage + /// The main executable target of product. /// /// Note: This property is only valid for executable products. public var executableTarget: ResolvedTarget { get throws { - guard type == .executable || type == .snippet || type == .macro else { + guard self.type == .executable || self.type == .snippet || self.type == .macro else { throw InternalError("`executableTarget` should only be called for executable targets") } - guard let underlyingExecutableTarget = targets.map({ $0.underlyingTarget }).executables.first, let executableTarget = targets.first(where: { $0.underlyingTarget == underlyingExecutableTarget }) else { + guard let underlyingExecutableTarget = targets.map(\.underlying).executables.first, + let executableTarget = targets.first(where: { $0.underlying == underlyingExecutableTarget }) + else { throw InternalError("could not determine executable target") } return executableTarget } } - public init(product: Product, targets: [ResolvedTarget]) { - assert(product.targets.count == targets.count && product.targets.map({ $0.name }) == targets.map({ $0.name })) - self.underlyingProduct = product - self.targets = targets - - // defaultLocalization is currently shared across the entire package - // this may need to be enhanced if / when we support localization per target or product - let defaultLocalization = self.targets.first?.defaultLocalization - self.defaultLocalization = defaultLocalization - + public convenience init(product: Product, targets: [ResolvedTarget]) { + assert(product.targets.count == targets.count && product.targets.map(\.name) == targets.map(\.name)) let (platforms, platformVersionProvider) = Self.computePlatforms(targets: targets) - self.supportedPlatforms = platforms - self.platformVersionProvider = platformVersionProvider + let defaultLocalization = targets.first?.defaultLocalization + + self.init( + storage: .init( + underlying: product, + targets: targets, + testEntryPointTarget: product.testEntryPointPath.map { testEntryPointPath in + // Create an executable resolved target with the entry point file, adding product's targets as dependencies. + let dependencies: [Target.Dependency] = product.targets.map { .target($0, conditions: []) } + let swiftTarget = SwiftTarget( + name: product.name, + dependencies: dependencies, + packageAccess: true, // entry point target so treated as a part of the package + testEntryPointPath: testEntryPointPath + ) + return ResolvedTarget( + storage: .init( + underlying: swiftTarget, + dependencies: targets.map { + .target($0, conditions: []) + }, + defaultLocalization: defaultLocalization, + supportedPlatforms: platforms + ), + platformVersionProvider: platformVersionProvider + ) + }, + // defaultLocalization is currently shared across the entire package + // this may need to be enhanced if / when we support localization per target or product + defaultLocalization: defaultLocalization, + platforms: platforms + ), + platformVersionProvider: platformVersionProvider + ) + } - self.testEntryPointTarget = product.testEntryPointPath.map { testEntryPointPath in - // Create an executable resolved target with the entry point file, adding product's targets as dependencies. - let dependencies: [Target.Dependency] = product.targets.map { .target($0, conditions: []) } - let swiftTarget = SwiftTarget(name: product.name, - dependencies: dependencies, - packageAccess: true, // entry point target so treated as a part of the package - testEntryPointPath: testEntryPointPath) - return ResolvedTarget( - target: swiftTarget, - dependencies: targets.map { .target($0, conditions: []) }, - defaultLocalization: defaultLocalization ?? .none, // safe since this is a derived product - supportedPlatforms: platforms, - platformVersionProvider: platformVersionProvider - ) - } + init(storage: Storage, platformVersionProvider: PlatformVersionProvider) { + self.storage = storage + self.platformVersionProvider = platformVersionProvider } /// True if this product contains Swift targets. public var containsSwiftTargets: Bool { - // C targets can't import Swift targets in SwiftPM (at least not right - // now), so we can just look at the top-level targets. - // - // If that ever changes, we'll need to do something more complex here, - // recursively checking dependencies for SwiftTargets, and considering - // dynamic library targets to be Swift targets (since the dylib could - // contain Swift code we don't know about as part of this build). - return targets.contains { $0.underlyingTarget is SwiftTarget } + // C targets can't import Swift targets in SwiftPM (at least not right + // now), so we can just look at the top-level targets. + // + // If that ever changes, we'll need to do something more complex here, + // recursively checking dependencies for SwiftTargets, and considering + // dynamic library targets to be Swift targets (since the dylib could + // contain Swift code we don't know about as part of this build). + self.targets.contains { $0.underlying is SwiftTarget } } /// Returns the recursive target dependencies. public func recursiveTargetDependencies() throws -> [ResolvedTarget] { let recursiveDependencies = try targets.lazy.flatMap { try $0.recursiveTargetDependencies() } - return Array(Set(targets).union(recursiveDependencies)) + return Array(Set(self.targets).union(recursiveDependencies)) } + private static func computePlatforms(targets: [ResolvedTarget]) -> ([SupportedPlatform], PlatformVersionProvider) { - let declaredPlatforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in - merge(into: &partial, platforms: item.supportedPlatforms) + // merging two sets of supported platforms, preferring the max constraint + func merge(into partial: inout [SupportedPlatform], platforms: [SupportedPlatform]) { + for platformSupport in platforms { + if let existing = partial.firstIndex(where: { $0.platform == platformSupport.platform }) { + if partial[existing].version < platformSupport.version { + partial.remove(at: existing) + partial.append(platformSupport) + } + } else { + partial.append(platformSupport) + } + } + } + + let declared = targets.reduce(into: [SupportedPlatform]()) { partial, item in + merge(into: &partial, platforms: item.platforms) } return ( - declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }), - PlatformVersionProvider(implementation: .mergingFromTargets(targets)) + declared.sorted(by: { $0.platform.name < $1.platform.name }), + .init(derivedXCTestPlatformProvider: { declared in + let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in + merge(into: &partial, platforms: [item.getDerived(for: declared, usingXCTest: item.type == .test)]) + } + return platforms.first!.version + }) ) - } + } public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.supportedPlatforms, + declared: self.platforms, for: platform, usingXCTest: usingXCTest ) } + + func diagnoseInvalidUseOfUnsafeFlags(_ diagnosticsEmitter: DiagnosticsEmitter) throws { + // Diagnose if any target in this product uses an unsafe flag. + for target in try self.recursiveTargetDependencies() { + if target.underlying.usesUnsafeFlags { + diagnosticsEmitter.emit(.productUsesUnsafeFlags(product: self.name, target: target.name)) + } + } + } } extension ResolvedProduct: Hashable { public func hash(into hasher: inout Hasher) { - hasher.combine(ObjectIdentifier(self)) + hasher.combine(self.storage) } public static func == (lhs: ResolvedProduct, rhs: ResolvedProduct) -> Bool { - ObjectIdentifier(lhs) == ObjectIdentifier(rhs) + lhs.storage == rhs.storage } } extension ResolvedProduct: CustomStringConvertible { public var description: String { - return "" + "" } } extension ResolvedProduct { public var isLinkingXCTest: Bool { - // To retain existing behavior, we have to check both the product type, as well as the types of all of its targets. - return self.type == .test || self.targets.contains(where: { $0.type == .test }) + // To retain existing behavior, we have to check both the product type, as well as the types of all of its + // targets. + self.type == .test || self.targets.contains(where: { $0.type == .test }) } } diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index d67e6335729..3984bf80064 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -17,7 +17,7 @@ import func TSCBasic.topologicalSort /// Represents a fully resolved target. All the dependencies for the target are resolved. public final class ResolvedTarget { /// Represents dependency of a resolved target. - public enum Dependency { + public enum Dependency: Hashable { /// Direct dependency of the target. This target is in the same package and should be statically linked. case target(_ target: ResolvedTarget, conditions: [PackageCondition]) @@ -70,17 +70,11 @@ public final class ResolvedTarget { } } - /// The underlying target represented in this resolved target. - public let underlyingTarget: Target - /// The name of this target. public var name: String { - return underlyingTarget.name + return underlying.name } - /// The dependencies of this target. - public let dependencies: [Dependency] - /// Returns dependencies which satisfy the input build environment, based on their conditions. /// - Parameters: /// - environment: The build environment to use to filter dependencies on. @@ -110,57 +104,99 @@ public final class ResolvedTarget { /// The language-level target name. public var c99name: String { - return underlyingTarget.c99name + return underlying.c99name } /// Module aliases for dependencies of this target. The key is an /// original target name and the value is a new unique name mapped /// to the name of its .swiftmodule binary. public var moduleAliases: [String: String]? { - return underlyingTarget.moduleAliases + return underlying.moduleAliases } /// Allows access to package symbols from other targets in the package public var packageAccess: Bool { - return underlyingTarget.packageAccess + return underlying.packageAccess } /// The "type" of target. public var type: Target.Kind { - return underlyingTarget.type + return underlying.type } /// The sources for the target. public var sources: Sources { - return underlyingTarget.sources + return underlying.sources + } + + /// The dependencies of this target. + public var dependencies: [Dependency] { + self.storage.dependencies + } + + /// The underlying target model represented in this resolved target. + public var underlying: Target { + self.storage.underlying } /// The default localization for resources. - public let defaultLocalization: String? + public var defaultLocalization: String? { + self.storage.defaultLocalization + } /// The list of platforms that are supported by this target. - public let supportedPlatforms: [SupportedPlatform] + public var platforms: [SupportedPlatform] { + self.storage.supportedPlatforms + } + + struct Storage: Hashable { + /// The underlying target represented in this resolved target. + let underlying: Target + + /// The dependencies of this target. + let dependencies: [Dependency] + + /// The default localization for resources. + let defaultLocalization: String? + + /// The list of platforms that are supported by this target. + let supportedPlatforms: [SupportedPlatform] + } + + private let storage: Storage private let platformVersionProvider: PlatformVersionProvider - /// Create a resolved target instance. - public init( + /// Create a target instance. + public convenience init( target: Target, dependencies: [Dependency], defaultLocalization: String?, - supportedPlatforms: [SupportedPlatform], + platforms: [SupportedPlatform], platformVersionProvider: PlatformVersionProvider ) { - self.underlyingTarget = target - self.dependencies = dependencies - self.defaultLocalization = defaultLocalization - self.supportedPlatforms = supportedPlatforms + self.init( + storage: .init( + underlying: target, + dependencies: dependencies, + defaultLocalization: defaultLocalization, + supportedPlatforms: platforms + ), + platformVersionProvider: platformVersionProvider + ) + } + + init( + storage: Storage, + platformVersionProvider: PlatformVersionProvider + ) { + self.storage = storage self.platformVersionProvider = platformVersionProvider } public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.supportedPlatforms, + declared: self.storage.supportedPlatforms, for: platform, usingXCTest: usingXCTest ) @@ -169,11 +205,11 @@ public final class ResolvedTarget { extension ResolvedTarget: Hashable { public func hash(into hasher: inout Hasher) { - hasher.combine(ObjectIdentifier(self)) + hasher.combine(self.storage) } public static func == (lhs: ResolvedTarget, rhs: ResolvedTarget) -> Bool { - ObjectIdentifier(lhs) == ObjectIdentifier(rhs) + lhs.storage == rhs.storage } } @@ -183,30 +219,6 @@ extension ResolvedTarget: CustomStringConvertible { } } -extension ResolvedTarget.Dependency: Equatable { - public static func == (lhs: ResolvedTarget.Dependency, rhs: ResolvedTarget.Dependency) -> Bool { - switch (lhs, rhs) { - case (.target(let lhsTarget, _), .target(let rhsTarget, _)): - return lhsTarget == rhsTarget - case (.product(let lhsProduct, _), .product(let rhsProduct, _)): - return lhsProduct == rhsProduct - case (.product, .target), (.target, .product): - return false - } - } -} - -extension ResolvedTarget.Dependency: Hashable { - public func hash(into hasher: inout Hasher) { - switch self { - case .target(let target, _): - hasher.combine(target) - case .product(let product, _): - hasher.combine(product) - } - } -} - extension ResolvedTarget.Dependency: CustomStringConvertible { public var description: String { var str = " [PackageCondition] { - var conditions = [PackageCondition]() + var conditions: [PackageCondition] = [] if let config = condition?.config.flatMap({ BuildConfiguration(rawValue: $0) }) { - let condition = ConfigurationCondition(configuration: config) - conditions.append(.configuration(condition)) + let condition = PackageCondition(configuration: config) + conditions.append(condition) } if let platforms = condition?.platformNames.map({ @@ -1162,8 +1162,8 @@ public final class PackageBuilder { }), !platforms.isEmpty { - let condition = PlatformsCondition(platforms: platforms) - conditions.append(.platforms(condition)) + let condition = PackageCondition(platforms: platforms) + conditions.append(condition) } return conditions diff --git a/Sources/PackageModel/BuildConfiguration.swift b/Sources/PackageModel/BuildConfiguration.swift index f4df1d7dde7..4bf08bef3a3 100644 --- a/Sources/PackageModel/BuildConfiguration.swift +++ b/Sources/PackageModel/BuildConfiguration.swift @@ -11,7 +11,7 @@ //===----------------------------------------------------------------------===// /// The configuration of the build environment. -public enum BuildConfiguration: String, CaseIterable, Codable { +public enum BuildConfiguration: String, CaseIterable, Codable, Sendable { case debug case release diff --git a/Sources/PackageModel/Manifest/PackageConditionDescription.swift b/Sources/PackageModel/Manifest/PackageConditionDescription.swift index a8fef073cba..f875bafd16e 100644 --- a/Sources/PackageModel/Manifest/PackageConditionDescription.swift +++ b/Sources/PackageModel/Manifest/PackageConditionDescription.swift @@ -11,7 +11,7 @@ //===----------------------------------------------------------------------===// /// Represents a manifest condition. -public struct PackageConditionDescription: Codable, Equatable, Sendable { +public struct PackageConditionDescription: Codable, Hashable, Sendable { public let platformNames: [String] public let config: String? @@ -77,7 +77,7 @@ struct PackageConditionWrapper: Codable, Equatable, Hashable { /// One of possible conditions used in package manifests to restrict targets from being built for certain platforms or /// build configurations. -public enum PackageCondition: Hashable { +public enum PackageCondition: Hashable, Sendable { case platforms(PlatformsCondition) case configuration(ConfigurationCondition) @@ -116,7 +116,7 @@ public enum PackageCondition: Hashable { } /// Platforms condition implies that an assignment is valid on these platforms. -public struct PlatformsCondition: PackageConditionProtocol, Equatable, Hashable { +public struct PlatformsCondition: PackageConditionProtocol, Hashable, Sendable { public let platforms: [Platform] public init(platforms: [Platform]) { @@ -131,7 +131,7 @@ public struct PlatformsCondition: PackageConditionProtocol, Equatable, Hashable /// A configuration condition implies that an assignment is valid on /// a particular build configuration. -public struct ConfigurationCondition: PackageConditionProtocol, Equatable, Hashable { +public struct ConfigurationCondition: PackageConditionProtocol, Hashable, Sendable { public let configuration: BuildConfiguration public init(configuration: BuildConfiguration) { diff --git a/Sources/PackageModel/Manifest/PlatformDescription.swift b/Sources/PackageModel/Manifest/PlatformDescription.swift index 5d091f0cd93..dbd5875eff0 100644 --- a/Sources/PackageModel/Manifest/PlatformDescription.swift +++ b/Sources/PackageModel/Manifest/PlatformDescription.swift @@ -10,7 +10,7 @@ // //===----------------------------------------------------------------------===// -public struct PlatformDescription: Codable, Equatable, Sendable { +public struct PlatformDescription: Codable, Hashable, Sendable { public let platformName: String public let version: String public let options: [String] diff --git a/Sources/PackageModel/Manifest/ProductDescription.swift b/Sources/PackageModel/Manifest/ProductDescription.swift index 465caf3194a..dc0f05349f2 100644 --- a/Sources/PackageModel/Manifest/ProductDescription.swift +++ b/Sources/PackageModel/Manifest/ProductDescription.swift @@ -13,7 +13,7 @@ import Basics /// The product description -public struct ProductDescription: Equatable, Codable, Sendable { +public struct ProductDescription: Hashable, Codable, Sendable { /// The name of the product. public let name: String diff --git a/Sources/PackageModel/Manifest/SystemPackageProviderDescription.swift b/Sources/PackageModel/Manifest/SystemPackageProviderDescription.swift index f9c35822b0d..cef227bee18 100644 --- a/Sources/PackageModel/Manifest/SystemPackageProviderDescription.swift +++ b/Sources/PackageModel/Manifest/SystemPackageProviderDescription.swift @@ -11,7 +11,7 @@ //===----------------------------------------------------------------------===// /// Represents system package providers. -public enum SystemPackageProviderDescription: Equatable, Codable, Sendable { +public enum SystemPackageProviderDescription: Hashable, Codable, Sendable { case brew([String]) case apt([String]) case yum([String]) diff --git a/Sources/PackageModel/Manifest/TargetBuildSettingDescription.swift b/Sources/PackageModel/Manifest/TargetBuildSettingDescription.swift index adeb965da9a..a0eb9be2575 100644 --- a/Sources/PackageModel/Manifest/TargetBuildSettingDescription.swift +++ b/Sources/PackageModel/Manifest/TargetBuildSettingDescription.swift @@ -14,20 +14,20 @@ public enum TargetBuildSettingDescription { /// The tool for which a build setting is declared. - public enum Tool: String, Codable, Equatable, CaseIterable, Sendable { + public enum Tool: String, Codable, Hashable, CaseIterable, Sendable { case c case cxx case swift case linker } - public enum InteroperabilityMode: String, Codable, Equatable, Sendable { + public enum InteroperabilityMode: String, Codable, Hashable, Sendable { case C case Cxx } /// The kind of the build setting, with associate configuration - public enum Kind: Codable, Equatable, Sendable { + public enum Kind: Codable, Hashable, Sendable { case headerSearchPath(String) case define(String) case linkedLibrary(String) @@ -52,8 +52,7 @@ public enum TargetBuildSettingDescription { } /// An individual build setting. - public struct Setting: Codable, Equatable, Sendable { - + public struct Setting: Codable, Hashable, Sendable { /// The tool associated with this setting. public let tool: Tool diff --git a/Sources/PackageModel/Manifest/TargetDescription.swift b/Sources/PackageModel/Manifest/TargetDescription.swift index f21dafac8d3..d131c99f37e 100644 --- a/Sources/PackageModel/Manifest/TargetDescription.swift +++ b/Sources/PackageModel/Manifest/TargetDescription.swift @@ -11,10 +11,9 @@ //===----------------------------------------------------------------------===// /// The description of an individual target. -public struct TargetDescription: Equatable, Encodable, Sendable { - +public struct TargetDescription: Hashable, Encodable, Sendable { /// The target type. - public enum TargetType: String, Equatable, Encodable, Sendable { + public enum TargetType: String, Hashable, Encodable, Sendable { case regular case executable case test @@ -25,7 +24,7 @@ public struct TargetDescription: Equatable, Encodable, Sendable { } /// Represents a target's dependency on another entity. - public enum Dependency: Equatable, Sendable { + public enum Dependency: Hashable, Sendable { case target(name: String, condition: PackageConditionDescription?) case product(name: String, package: String?, moduleAliases: [String: String]? = nil, condition: PackageConditionDescription?) case byName(name: String, condition: PackageConditionDescription?) @@ -39,8 +38,8 @@ public struct TargetDescription: Equatable, Encodable, Sendable { } } - public struct Resource: Encodable, Equatable, Sendable { - public enum Rule: Encodable, Equatable, Sendable { + public struct Resource: Encodable, Hashable, Sendable { + public enum Rule: Encodable, Hashable, Sendable { case process(localization: Localization?) case copy case embedInCode @@ -111,18 +110,18 @@ public struct TargetDescription: Equatable, Encodable, Sendable { public let pluginCapability: PluginCapability? /// Represents the declared capability of a package plugin. - public enum PluginCapability: Equatable, Sendable { + public enum PluginCapability: Hashable, Sendable { case buildTool case command(intent: PluginCommandIntent, permissions: [PluginPermission]) } - public enum PluginCommandIntent: Equatable, Codable, Sendable { + public enum PluginCommandIntent: Hashable, Codable, Sendable { case documentationGeneration case sourceCodeFormatting case custom(verb: String, description: String) } - public enum PluginNetworkPermissionScope: Equatable, Codable, Sendable { + public enum PluginNetworkPermissionScope: Hashable, Codable, Sendable { case none case local(ports: [Int]) case all(ports: [Int]) @@ -141,7 +140,7 @@ public struct TargetDescription: Equatable, Encodable, Sendable { } } - public enum PluginPermission: Equatable, Codable, Sendable { + public enum PluginPermission: Hashable, Codable, Sendable { case allowNetworkConnections(scope: PluginNetworkPermissionScope, reason: String) case writeToPackageDirectory(reason: String) } @@ -156,7 +155,7 @@ public struct TargetDescription: Equatable, Encodable, Sendable { public let pluginUsages: [PluginUsage]? /// Represents a target's usage of a plugin target or product. - public enum PluginUsage: Equatable, Sendable { + public enum PluginUsage: Hashable, Sendable { case plugin(name: String, package: String?) } diff --git a/Sources/PackageModel/PackageModel.swift b/Sources/PackageModel/PackageModel.swift index 811849d92e5..3be8a42ca1e 100644 --- a/Sources/PackageModel/PackageModel.swift +++ b/Sources/PackageModel/PackageModel.swift @@ -95,6 +95,16 @@ public final class Package: Encodable { } } +extension Package: Hashable { + public func hash(into hasher: inout Hasher) { + hasher.combine(ObjectIdentifier(self)) + } + + public static func == (lhs: Package, rhs: Package) -> Bool { + ObjectIdentifier(lhs) == ObjectIdentifier(rhs) + } +} + extension Package { public var diagnosticsMetadata: ObservabilityMetadata { return .packageMetadata(identity: self.identity, kind: self.manifest.packageKind) diff --git a/Sources/PackageModel/PackageReference.swift b/Sources/PackageModel/PackageReference.swift index 7b62bfe511d..ff6b448101c 100644 --- a/Sources/PackageModel/PackageReference.swift +++ b/Sources/PackageModel/PackageReference.swift @@ -18,7 +18,7 @@ import Foundation /// This represents a reference to a package containing its identity and location. public struct PackageReference { /// The kind of package reference. - public enum Kind: Equatable, CustomStringConvertible, Sendable { + public enum Kind: Hashable, CustomStringConvertible, Sendable { /// A root package. case root(AbsolutePath) diff --git a/Sources/PackageModel/Platform.swift b/Sources/PackageModel/Platform.swift index 8cce1a3dc8b..509db8842d5 100644 --- a/Sources/PackageModel/Platform.swift +++ b/Sources/PackageModel/Platform.swift @@ -13,7 +13,7 @@ import struct TSCUtility.Version /// Represents a platform. -public struct Platform: Equatable, Hashable, Codable { +public struct Platform: Equatable, Hashable, Codable, Sendable { /// The name of the platform. public let name: String @@ -53,7 +53,7 @@ public struct Platform: Equatable, Hashable, Codable { } /// Represents a platform supported by a target. -public struct SupportedPlatform: Equatable, Codable { +public struct SupportedPlatform: Hashable, Codable, Sendable { /// The platform. public let platform: Platform @@ -71,8 +71,8 @@ public struct SupportedPlatform: Equatable, Codable { } /// Represents a platform version. -public struct PlatformVersion: Equatable, Hashable, Codable { - +public struct PlatformVersion: Equatable, Hashable, Codable, Sendable { + // FIXME: this should be optional /// The unknown platform version. public static let unknown: PlatformVersion = .init("0.0.0") diff --git a/Sources/PackageModel/RegistryReleaseMetadata.swift b/Sources/PackageModel/RegistryReleaseMetadata.swift index f0e1affc2e9..206f8f178f1 100644 --- a/Sources/PackageModel/RegistryReleaseMetadata.swift +++ b/Sources/PackageModel/RegistryReleaseMetadata.swift @@ -15,7 +15,7 @@ import struct Foundation.URL import struct TSCUtility.Version -public struct RegistryReleaseMetadata { +public struct RegistryReleaseMetadata: Hashable { public let source: Source public let metadata: Metadata public let signature: RegistrySignature? @@ -31,7 +31,7 @@ public struct RegistryReleaseMetadata { } /// Metadata of the given release, provided by the registry. - public struct Metadata { + public struct Metadata: Hashable { public let author: Author? public let description: String? public let licenseURL: URL? @@ -52,7 +52,7 @@ public struct RegistryReleaseMetadata { self.scmRepositoryURLs = scmRepositoryURLs } - public struct Author { + public struct Author: Hashable { public let name: String public let emailAddress: String? public let description: String? @@ -74,7 +74,7 @@ public struct RegistryReleaseMetadata { } } - public struct Organization { + public struct Organization: Hashable { public let name: String public let emailAddress: String? public let description: String? @@ -90,7 +90,7 @@ public struct RegistryReleaseMetadata { } /// Information from the signing certificate. - public struct RegistrySignature: Codable { + public struct RegistrySignature: Hashable, Codable { public let signedBy: SigningEntity? public let format: String public let value: [UInt8] @@ -106,13 +106,13 @@ public struct RegistryReleaseMetadata { } } - public enum SigningEntity: Codable, Equatable, Sendable { + public enum SigningEntity: Codable, Hashable, Sendable { case recognized(type: String, commonName: String?, organization: String?, identity: String?) case unrecognized(commonName: String?, organization: String?) } /// Information about the source of the release. - public enum Source: Equatable { + public enum Source: Hashable { case registry(URL) } } diff --git a/Sources/PackageModel/SwiftLanguageVersion.swift b/Sources/PackageModel/SwiftLanguageVersion.swift index 03408c8cba1..2aabc03c138 100644 --- a/Sources/PackageModel/SwiftLanguageVersion.swift +++ b/Sources/PackageModel/SwiftLanguageVersion.swift @@ -17,7 +17,7 @@ import struct TSCBasic.RegEx import struct TSCUtility.Version /// Represents a Swift language version. -public struct SwiftLanguageVersion: Sendable { +public struct SwiftLanguageVersion: Hashable, Sendable { /// Swift language version 3. public static let v3 = SwiftLanguageVersion(uncheckedString: "3") diff --git a/Sources/PackageModel/Target/Target.swift b/Sources/PackageModel/Target/Target.swift index faf1bb5cda0..541e37794de 100644 --- a/Sources/PackageModel/Target/Target.swift +++ b/Sources/PackageModel/Target/Target.swift @@ -11,7 +11,6 @@ //===----------------------------------------------------------------------===// import Basics -import Dispatch import protocol TSCUtility.PolymorphicCodableProtocol @@ -42,9 +41,9 @@ public class Target: PolymorphicCodableProtocol { case package case excluded } + /// A reference to a product from a target dependency. public struct ProductReference: Codable { - /// The name of the product dependency. public let name: String diff --git a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift index e93f24d5fe3..9dc4797dd30 100644 --- a/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift +++ b/Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift @@ -61,26 +61,25 @@ internal struct PluginContextSerializer { // Construct the FileList var targetFiles: [WireInput.Target.TargetInfo.File] = [] - targetFiles.append(contentsOf: try target.underlyingTarget.sources.paths.map { + targetFiles.append(contentsOf: try target.underlying.sources.paths.map { .init(basePathId: try serialize(path: $0.parentDirectory), name: $0.basename, type: .source) }) - targetFiles.append(contentsOf: try target.underlyingTarget.resources.map { + targetFiles.append(contentsOf: try target.underlying.resources.map { .init(basePathId: try serialize(path: $0.path.parentDirectory), name: $0.path.basename, type: .resource) }) - targetFiles.append(contentsOf: try target.underlyingTarget.ignored.map { + targetFiles.append(contentsOf: try target.underlying.ignored.map { .init(basePathId: try serialize(path: $0.parentDirectory), name: $0.basename, type: .unknown) }) - targetFiles.append(contentsOf: try target.underlyingTarget.others.map { + targetFiles.append(contentsOf: try target.underlying.others.map { .init(basePathId: try serialize(path: $0.parentDirectory), name: $0.basename, type: .unknown) }) // Create a scope for evaluating build settings. - let scope = BuildSettings.Scope(target.underlyingTarget.buildSettings, environment: buildEnvironment) + let scope = BuildSettings.Scope(target.underlying.buildSettings, environment: buildEnvironment) // Look at the target and decide what to serialize. At this point we may decide to not serialize it at all. let targetInfo: WireInput.Target.TargetInfo - switch target.underlyingTarget { - + switch target.underlying { case let target as SwiftTarget: targetInfo = .swiftSourceModuleInfo( moduleName: target.c99name, diff --git a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift index e7414507105..3997c426d35 100644 --- a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift +++ b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift @@ -348,15 +348,15 @@ extension PackageGraph { for dependency in target.dependencies(satisfying: buildEnvironment) { switch dependency { case .target(let target, _): - if let pluginTarget = target.underlyingTarget as? PluginTarget { + if let pluginTarget = target.underlying as? PluginTarget { assert(pluginTarget.capability == .buildTool) pluginTargets.append(pluginTarget) } else { - dependencyTargets.append(target.underlyingTarget) + dependencyTargets.append(target.underlying) } case .product(let product, _): - pluginTargets.append(contentsOf: product.targets.compactMap{ $0.underlyingTarget as? PluginTarget }) + pluginTargets.append(contentsOf: product.targets.compactMap{ $0.underlying as? PluginTarget }) } } @@ -537,7 +537,10 @@ public extension PluginTarget { builtToolName = target.name executableOrBinaryTarget = target case .product(let productRef, _): - guard let product = packageGraph.allProducts.first(where: { $0.name == productRef.name }), let executableTarget = product.targets.map({ $0.underlyingTarget }).executables.spm_only else { + guard + let product = packageGraph.allProducts.first(where: { $0.name == productRef.name }), + let executableTarget = product.targets.map({ $0.underlying }).executables.spm_only + else { throw StringError("no product named \(productRef.name)") } builtToolName = productRef.name diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index ac27f2c3220..0aaa89c39cb 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -1149,9 +1149,9 @@ extension Workspace { self.loadManifest( packageIdentity: identity, - packageKind: previousPackage.underlyingPackage.manifest.packageKind, + packageKind: previousPackage.underlying.manifest.packageKind, packagePath: previousPackage.path, - packageLocation: previousPackage.underlyingPackage.manifest.packageLocation, + packageLocation: previousPackage.underlying.manifest.packageLocation, observabilityScope: observabilityScope ) { result in let result = result.tryMap { manifest -> Package in diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index b984499c105..925e4c5efe5 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -247,7 +247,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { self.fileSystem = fileSystem self.observabilityScope = observabilityScope.makeChildScope( description: "Package PIF Builder", - metadata: package.underlyingPackage.diagnosticsMetadata + metadata: package.underlying.diagnosticsMetadata ) executableTargetProductMap = try Dictionary(throwingUniqueKeysWithValues: @@ -429,7 +429,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { } // Tests can have a custom deployment target based on the minimum supported by XCTest. - if mainTarget.underlyingTarget.type == .test { + if mainTarget.underlying.type == .test { settings[.MACOSX_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .macOS, usingXCTest: true) settings[.IPHONEOS_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .iOS, usingXCTest: true) settings[.TVOS_DEPLOYMENT_TARGET] = mainTarget.deploymentTarget(for: .tvOS, usingXCTest: true) @@ -456,12 +456,12 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { settings[.GENERATE_INFOPLIST_FILE] = "YES" } - if let clangTarget = mainTarget.underlyingTarget as? ClangTarget { + if let clangTarget = mainTarget.underlying as? ClangTarget { // Let the target itself find its own headers. settings[.HEADER_SEARCH_PATHS, default: ["$(inherited)"]].append(clangTarget.includeDir.pathString) settings[.GCC_C_LANGUAGE_STANDARD] = clangTarget.cLanguageStandard settings[.CLANG_CXX_LANGUAGE_STANDARD] = clangTarget.cxxLanguageStandard - } else if let swiftTarget = mainTarget.underlyingTarget as? SwiftTarget { + } else if let swiftTarget = mainTarget.underlying as? SwiftTarget { settings[.SWIFT_VERSION] = swiftTarget.swiftVersion.description } @@ -477,7 +477,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { var impartedSettings = PIF.BuildSettings() try addManifestBuildSettings( - from: mainTarget.underlyingTarget, + from: mainTarget.underlying, debugSettings: &debugSettings, releaseSettings: &releaseSettings, impartedSettings: &impartedSettings @@ -534,7 +534,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { } var settings = PIF.BuildSettings() - let usesUnsafeFlags = dependencies.contains { $0.target?.underlyingTarget.usesUnsafeFlags == true } + let usesUnsafeFlags = dependencies.contains { $0.target?.underlying.usesUnsafeFlags == true } settings[.USES_SWIFTPM_UNSAFE_FLAGS] = usesUnsafeFlags ? "YES" : "NO" // If there are no system modules in the dependency graph, mark the target as extension-safe. @@ -612,7 +612,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { let moduleMapFileContents: String? let shouldImpartModuleMap: Bool - if let clangTarget = target.underlyingTarget as? ClangTarget { + if let clangTarget = target.underlying as? ClangTarget { // Let the target itself find its own headers. settings[.HEADER_SEARCH_PATHS, default: ["$(inherited)"]].append(clangTarget.includeDir.pathString) settings[.GCC_C_LANGUAGE_STANDARD] = clangTarget.cLanguageStandard @@ -637,7 +637,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { moduleMapFileContents = nil shouldImpartModuleMap = false } - } else if let swiftTarget = target.underlyingTarget as? SwiftTarget { + } else if let swiftTarget = target.underlying as? SwiftTarget { settings[.SWIFT_VERSION] = swiftTarget.swiftVersion.description // Generate ObjC compatibility header for Swift library targets. settings[.SWIFT_OBJC_INTERFACE_HEADER_DIR] = "$(OBJROOT)/GeneratedModuleMaps/$(PLATFORM_NAME)" @@ -666,7 +666,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { } impartedSettings[.OTHER_LDRFLAGS] = [] - if target.underlyingTarget.isCxx { + if target.underlying.isCxx { impartedSettings[.OTHER_LDFLAGS, default: ["$(inherited)"]].append("-lc++") } @@ -690,7 +690,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { var releaseSettings = settings try addManifestBuildSettings( - from: target.underlyingTarget, + from: target.underlying, debugSettings: &debugSettings, releaseSettings: &releaseSettings, impartedSettings: &impartedSettings @@ -703,7 +703,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { } private func addSystemTarget(for target: ResolvedTarget) throws { - guard let systemTarget = target.underlyingTarget as? SystemLibraryTarget else { + guard let systemTarget = target.underlying as? SystemLibraryTarget else { throw InternalError("unexpected target type") } @@ -785,7 +785,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { linkProduct: Bool ) { // Only add the binary target as a library when we want to link against the product. - if let binaryTarget = target.underlyingTarget as? BinaryTarget { + if let binaryTarget = target.underlying as? BinaryTarget { let ref = binaryGroup.addFileReference(path: binaryTarget.artifactPath.pathString) pifTarget.addLibrary(ref, platformFilters: conditions.toPlatformFilters()) } else { @@ -814,7 +814,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { } private func addResourceBundle(for target: ResolvedTarget, in pifTarget: PIFTargetBuilder) -> String? { - guard !target.underlyingTarget.resources.isEmpty else { + guard !target.underlying.resources.isEmpty else { return nil } @@ -845,7 +845,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { resourcesTarget.addBuildConfiguration(name: "Release", settings: settings) let coreDataFileTypes = [XCBuildFileType.xcdatamodeld, .xcdatamodel].flatMap { $0.fileTypes } - for resource in target.underlyingTarget.resources { + for resource in target.underlying.resources { // FIXME: Handle rules here. let resourceFile = groupTree.addFileReference( path: resource.path.pathString, @@ -1378,7 +1378,7 @@ extension ResolvedProduct { var pifTargetGUID: PIF.GUID { "PACKAGE-PRODUCT:\(name)" } var mainTarget: ResolvedTarget { - targets.first { $0.type == underlyingProduct.type.targetType }! + targets.first { $0.type == underlying.type.targetType }! } /// Returns the recursive dependencies, limited to the target's package, which satisfy the input build environment, diff --git a/Tests/FunctionalTests/PluginTests.swift b/Tests/FunctionalTests/PluginTests.swift index 43da00c4590..82d0244cab5 100644 --- a/Tests/FunctionalTests/PluginTests.swift +++ b/Tests/FunctionalTests/PluginTests.swift @@ -383,7 +383,7 @@ class PluginTests: XCTestCase { let package = try XCTUnwrap(packageGraph.rootPackages.first) // Find the regular target in our test package. - let libraryTarget = try XCTUnwrap(package.targets.map(\.underlyingTarget).first{ $0.name == "MyLibrary" } as? SwiftTarget) + let libraryTarget = try XCTUnwrap(package.targets.map(\.underlying).first{ $0.name == "MyLibrary" } as? SwiftTarget) XCTAssertEqual(libraryTarget.type, .library) // Set up a delegate to handle callbacks from the command plugin. @@ -436,7 +436,7 @@ class PluginTests: XCTestCase { diagnosticsChecker: (DiagnosticsTestResult) throws -> Void ) { // Find the named plugin. - let plugins = package.targets.compactMap{ $0.underlyingTarget as? PluginTarget } + let plugins = package.targets.compactMap{ $0.underlying as? PluginTarget } guard let plugin = plugins.first(where: { $0.name == pluginName }) else { return XCTFail("There is no plugin target named ‘\(pluginName)’") } @@ -445,7 +445,7 @@ class PluginTests: XCTestCase { // Find the named input targets to the plugin. var targets: [ResolvedTarget] = [] for targetName in targetNames { - guard let target = package.targets.first(where: { $0.underlyingTarget.name == targetName }) else { + guard let target = package.targets.first(where: { $0.underlying.name == targetName }) else { return XCTFail("There is no target named ‘\(targetName)’") } XCTAssertTrue(target.type != .plugin, "Target \(target) is a plugin") @@ -665,7 +665,7 @@ class PluginTests: XCTestCase { // Find the regular target in our test package. let libraryTarget = try XCTUnwrap( package.targets - .map(\.underlyingTarget) + .map(\.underlying) .first{ $0.name == "MyLibrary" } as? SwiftTarget ) XCTAssertEqual(libraryTarget.type, .library) @@ -718,7 +718,7 @@ class PluginTests: XCTestCase { } // Find the relevant plugin. - let plugins = package.targets.compactMap{ $0.underlyingTarget as? PluginTarget } + let plugins = package.targets.compactMap { $0.underlying as? PluginTarget } guard let plugin = plugins.first(where: { $0.name == "NeverendingPlugin" }) else { return XCTFail("There is no plugin target named ‘NeverendingPlugin’") } diff --git a/Tests/SPMBuildCoreTests/PluginInvocationTests.swift b/Tests/SPMBuildCoreTests/PluginInvocationTests.swift index 83cf4fca585..045d87b13c3 100644 --- a/Tests/SPMBuildCoreTests/PluginInvocationTests.swift +++ b/Tests/SPMBuildCoreTests/PluginInvocationTests.swift @@ -300,7 +300,7 @@ class PluginInvocationTests: XCTestCase { XCTAssert(packageGraph.packages.count == 1, "\(packageGraph.packages)") // Find the build tool plugin. - let buildToolPlugin = try XCTUnwrap(packageGraph.packages.first?.targets.map(\.underlyingTarget).first{ $0.name == "MyPlugin" } as? PluginTarget) + let buildToolPlugin = try XCTUnwrap(packageGraph.packages.first?.targets.map(\.underlying).first{ $0.name == "MyPlugin" } as? PluginTarget) XCTAssertEqual(buildToolPlugin.name, "MyPlugin") XCTAssertEqual(buildToolPlugin.capability, .buildTool) @@ -879,7 +879,7 @@ class PluginInvocationTests: XCTestCase { XCTAssert(packageGraph.packages.count == 1, "\(packageGraph.packages)") // Find the build tool plugin. - let buildToolPlugin = try XCTUnwrap(packageGraph.packages.first?.targets.map(\.underlyingTarget).filter{ $0.name == "X" }.first as? PluginTarget) + let buildToolPlugin = try XCTUnwrap(packageGraph.packages.first?.targets.map(\.underlying).filter{ $0.name == "X" }.first as? PluginTarget) XCTAssertEqual(buildToolPlugin.name, "X") XCTAssertEqual(buildToolPlugin.capability, .buildTool) @@ -1207,7 +1207,7 @@ class PluginInvocationTests: XCTestCase { // Find the build tool plugin. let buildToolPlugin = try XCTUnwrap(packageGraph.packages.first?.targets - .map(\.underlyingTarget) + .map(\.underlying) .filter { $0.name == "Foo" } .first as? PluginTarget) XCTAssertEqual(buildToolPlugin.name, "Foo") From 086d0f9ae47389aca89980fb6123a96747d76237 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 20:30:20 +0000 Subject: [PATCH 12/24] Make `Resolved*` in `PackageGraph` value types --- .../ClangTargetBuildDescription.swift | 2 +- .../TargetBuildDescription.swift | 2 +- .../LLBuildManifestBuilder+Clang.swift | 2 +- .../LLBuildManifestBuilder+Swift.swift | 2 +- .../Build/BuildPlan/BuildPlan+Product.swift | 4 +- Sources/Build/BuildPlan/BuildPlan+Test.swift | 10 +- .../PackageGraph/PackageGraph+Loading.swift | 10 +- .../Resolution/ResolvedPackage.swift | 97 +++--------- .../Resolution/ResolvedProduct.swift | 142 +++++------------- .../Resolution/ResolvedTarget.swift | 82 +++------- Tests/PackageGraphTests/TargetTests.swift | 4 +- 11 files changed, 97 insertions(+), 260 deletions(-) diff --git a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift index 6e4af61226b..5baded0ef17 100644 --- a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift @@ -13,7 +13,7 @@ import Basics import PackageLoading import PackageModel -import class PackageGraph.ResolvedTarget +import struct PackageGraph.ResolvedTarget import struct SPMBuildCore.BuildParameters import struct SPMBuildCore.BuildToolPluginInvocationResult import struct SPMBuildCore.PrebuildCommandResult diff --git a/Sources/Build/BuildDescription/TargetBuildDescription.swift b/Sources/Build/BuildDescription/TargetBuildDescription.swift index c177a0b7dcd..a4042fa3f9e 100644 --- a/Sources/Build/BuildDescription/TargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/TargetBuildDescription.swift @@ -11,7 +11,7 @@ //===----------------------------------------------------------------------===// import Basics -import class PackageGraph.ResolvedTarget +import struct PackageGraph.ResolvedTarget import struct PackageModel.Resource import struct SPMBuildCore.BuildToolPluginInvocationResult diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift index f5626357d4f..5e57c697406 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift @@ -13,7 +13,7 @@ import struct LLBuildManifest.Node import struct Basics.AbsolutePath import struct Basics.InternalError -import class PackageGraph.ResolvedTarget +import struct PackageGraph.ResolvedTarget import PackageModel extension LLBuildManifestBuilder { diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift index f2ca6f6c7b4..a243e69ed8f 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift @@ -17,7 +17,7 @@ import struct Basics.TSCAbsolutePath import struct LLBuildManifest.Node import struct LLBuildManifest.LLBuildManifest import struct SPMBuildCore.BuildParameters -import class PackageGraph.ResolvedTarget +import struct PackageGraph.ResolvedTarget import protocol TSCBasic.FileSystem import enum TSCBasic.ProcessEnv import func TSCBasic.topologicalSort diff --git a/Sources/Build/BuildPlan/BuildPlan+Product.swift b/Sources/Build/BuildPlan/BuildPlan+Product.swift index aef0f87a27f..4bffbcf5dd1 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Product.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Product.swift @@ -12,8 +12,8 @@ import struct Basics.AbsolutePath import struct Basics.InternalError -import class PackageGraph.ResolvedProduct -import class PackageGraph.ResolvedTarget +import struct PackageGraph.ResolvedProduct +import struct PackageGraph.ResolvedTarget import class PackageModel.BinaryTarget import class PackageModel.ClangTarget import class PackageModel.Target diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index 945f9c9fed8..c9bc9d6b3c1 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -16,8 +16,8 @@ import struct Basics.AbsolutePath import struct LLBuildManifest.TestDiscoveryTool import struct LLBuildManifest.TestEntryPointTool import struct PackageGraph.PackageGraph -import class PackageGraph.ResolvedProduct -import class PackageGraph.ResolvedTarget +import struct PackageGraph.ResolvedProduct +import struct PackageGraph.ResolvedTarget import struct PackageModel.Sources import class PackageModel.SwiftTarget import class PackageModel.Target @@ -84,7 +84,7 @@ extension BuildPlan { testDiscoverySrc: Sources(paths: discoveryPaths, root: discoveryDerivedDir) ) let discoveryResolvedTarget = ResolvedTarget( - target: discoveryTarget, + underlying: discoveryTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) }, defaultLocalization: testProduct.defaultLocalization, supportedPlatforms: testProduct.supportedPlatforms, @@ -118,7 +118,7 @@ extension BuildPlan { testEntryPointSources: entryPointSources ) let entryPointResolvedTarget = ResolvedTarget( - target: entryPointTarget, + underlying: entryPointTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) } + [.target(discoveryResolvedTarget, conditions: [])], defaultLocalization: testProduct.defaultLocalization, supportedPlatforms: testProduct.supportedPlatforms, @@ -148,7 +148,7 @@ extension BuildPlan { testEntryPointSources: entryPointResolvedTarget.underlying.sources ) let entryPointResolvedTarget = ResolvedTarget( - target: entryPointTarget, + underlying: entryPointTarget, dependencies: entryPointResolvedTarget.dependencies + [.target(discoveryTargets.resolved, conditions: [])], defaultLocalization: testProduct.defaultLocalization, supportedPlatforms: testProduct.supportedPlatforms, diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index 31395de1a93..c20f0080e34 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -924,12 +924,10 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { } return ResolvedTarget( - storage: .init( - underlying: self.target, - dependencies: dependencies, - defaultLocalization: self.defaultLocalization, - supportedPlatforms: self.platforms - ), + underlying: self.target, + dependencies: dependencies, + defaultLocalization: self.defaultLocalization, + supportedPlatforms: self.platforms, platformVersionProvider: self.platformVersionProvider ) } diff --git a/Sources/PackageGraph/Resolution/ResolvedPackage.swift b/Sources/PackageGraph/Resolution/ResolvedPackage.swift index bb8078d2f2b..83ecd2d7c21 100644 --- a/Sources/PackageGraph/Resolution/ResolvedPackage.swift +++ b/Sources/PackageGraph/Resolution/ResolvedPackage.swift @@ -14,75 +14,47 @@ import Basics import PackageModel /// A fully resolved package. Contains resolved targets, products and dependencies of the package. -public final class ResolvedPackage { +public struct ResolvedPackage: Hashable { // The identity of the package. public var identity: PackageIdentity { - return self.storage.underlying.identity + return self.underlying.identity } /// The manifest describing the package. public var manifest: Manifest { - return self.storage.underlying.manifest + return self.underlying.manifest } /// The local path of the package. public var path: AbsolutePath { - return self.storage.underlying.path + return self.underlying.path } - /// The underlying package model. - public var underlying: Package { - self.storage.underlying - } + /// The underlying package reference. + public let underlying: Package /// The targets contained in the package. - public var targets: [ResolvedTarget] { - self.storage.targets - } + public let targets: [ResolvedTarget] /// The products produced by the package. - public var products: [ResolvedProduct] { - self.storage.products - } + public let products: [ResolvedProduct] /// The dependencies of the package. - public var dependencies: [ResolvedPackage] { - self.storage.dependencies - } + public let dependencies: [ResolvedPackage] - /// If the given package's source is a registry release, this provides additional metadata and signature information. - public var registryMetadata: RegistryReleaseMetadata? { - self.storage.registryMetadata - } + /// The default localization for resources. + public let defaultLocalization: String? - struct Storage: Hashable { - /// The underlying package reference. - public let underlying: Package - - /// The targets contained in the package. - public let targets: [ResolvedTarget] - - /// The products produced by the package. - public let products: [ResolvedProduct] - - /// The dependencies of the package. - public let dependencies: [ResolvedPackage] - - /// The default localization for resources. - public let defaultLocalization: String? - - /// The list of platforms that are supported by this target. - public let platforms: [SupportedPlatform] - - /// If the given package's source is a registry release, this provides additional metadata and signature information. - public let registryMetadata: RegistryReleaseMetadata? - } + /// The list of platforms that are supported by this target. + public let platforms: [SupportedPlatform] + + /// If the given package's source is a registry release, this provides additional metadata and signature information. + public let registryMetadata: RegistryReleaseMetadata? - private let storage: Storage private let platformVersionProvider: PlatformVersionProvider - public convenience init( + public init( package: Package, defaultLocalization: String?, platforms: [SupportedPlatform], @@ -92,44 +64,25 @@ public final class ResolvedPackage { registryMetadata: RegistryReleaseMetadata?, platformVersionProvider: PlatformVersionProvider ) { - self.init( - storage: .init( - underlying: package, - targets: targets, - products: products, - dependencies: dependencies, - defaultLocalization: defaultLocalization, - platforms: platforms, - registryMetadata: registryMetadata - ), - platformVersionProvider: platformVersionProvider - ) - } - - init(storage: Storage, platformVersionProvider: PlatformVersionProvider) { - self.storage = storage + self.underlying = package + self.targets = targets + self.products = products + self.dependencies = dependencies + self.defaultLocalization = defaultLocalization + self.platforms = platforms + self.registryMetadata = registryMetadata self.platformVersionProvider = platformVersionProvider } public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.storage.platforms, + declared: self.platforms, for: platform, usingXCTest: usingXCTest ) } } -extension ResolvedPackage: Hashable { - public func hash(into hasher: inout Hasher) { - hasher.combine(self.storage) - } - - public static func == (lhs: ResolvedPackage, rhs: ResolvedPackage) -> Bool { - lhs.storage == rhs.storage - } -} - extension ResolvedPackage: CustomStringConvertible { public var description: String { return "" diff --git a/Sources/PackageGraph/Resolution/ResolvedProduct.swift b/Sources/PackageGraph/Resolution/ResolvedProduct.swift index 7c11e5d2f65..bcad4c9f313 100644 --- a/Sources/PackageGraph/Resolution/ResolvedProduct.swift +++ b/Sources/PackageGraph/Resolution/ResolvedProduct.swift @@ -13,63 +13,34 @@ import Basics import PackageModel -public final class ResolvedProduct { +public struct ResolvedProduct: Hashable { /// The name of this product. public var name: String { - self.storage.underlying.name + self.underlying.name } /// The type of this product. public var type: ProductType { - self.storage.underlying.type + self.underlying.type } - /// The underlying product model. - public var underlying: Product { - self.storage.underlying - } + /// The underlying product. + public let underlying: Product /// The top level targets contained in this product. - public var targets: [ResolvedTarget] { - self.storage.targets - } + public let targets: [ResolvedTarget] /// Executable target for test entry point file. - public var testEntryPointTarget: ResolvedTarget? { - self.storage.testEntryPointTarget - } + public let testEntryPointTarget: ResolvedTarget? /// The default localization for resources. - public var defaultLocalization: String? { - self.storage.defaultLocalization - } + public let defaultLocalization: String? /// The list of platforms that are supported by this product. - public var platforms: [SupportedPlatform] { - self.storage.platforms - } + public let platforms: [SupportedPlatform] public let platformVersionProvider: PlatformVersionProvider - struct Storage: Hashable { - /// The underlying product. - let underlying: Product - - /// The top level targets contained in this product. - let targets: [ResolvedTarget] - - /// Executable target for test entry point file. - let testEntryPointTarget: ResolvedTarget? - - /// The default localization for resources. - let defaultLocalization: String? - - /// The list of platforms that are supported by this product. - let platforms: [SupportedPlatform] - } - - private let storage: Storage - /// The main executable target of product. /// /// Note: This property is only valid for executable products. @@ -87,47 +58,37 @@ public final class ResolvedProduct { } } - public convenience init(product: Product, targets: [ResolvedTarget]) { + public init(product: Product, targets: [ResolvedTarget]) { assert(product.targets.count == targets.count && product.targets.map(\.name) == targets.map(\.name)) let (platforms, platformVersionProvider) = Self.computePlatforms(targets: targets) let defaultLocalization = targets.first?.defaultLocalization - self.init( - storage: .init( - underlying: product, - targets: targets, - testEntryPointTarget: product.testEntryPointPath.map { testEntryPointPath in - // Create an executable resolved target with the entry point file, adding product's targets as dependencies. - let dependencies: [Target.Dependency] = product.targets.map { .target($0, conditions: []) } - let swiftTarget = SwiftTarget( - name: product.name, - dependencies: dependencies, - packageAccess: true, // entry point target so treated as a part of the package - testEntryPointPath: testEntryPointPath - ) - return ResolvedTarget( - storage: .init( - underlying: swiftTarget, - dependencies: targets.map { - .target($0, conditions: []) - }, - defaultLocalization: defaultLocalization, - supportedPlatforms: platforms - ), - platformVersionProvider: platformVersionProvider - ) + self.underlying = product + self.targets = targets + self.testEntryPointTarget = product.testEntryPointPath.map { testEntryPointPath in + // Create an executable resolved target with the entry point file, adding product's targets as dependencies. + let dependencies: [Target.Dependency] = product.targets.map { .target($0, conditions: []) } + let swiftTarget = SwiftTarget( + name: product.name, + dependencies: dependencies, + packageAccess: true, // entry point target so treated as a part of the package + testEntryPointPath: testEntryPointPath + ) + return ResolvedTarget( + underlying: swiftTarget, + dependencies: targets.map { + .target($0, conditions: []) }, - // defaultLocalization is currently shared across the entire package - // this may need to be enhanced if / when we support localization per target or product defaultLocalization: defaultLocalization, - platforms: platforms - ), - platformVersionProvider: platformVersionProvider - ) - } + supportedPlatforms: platforms, + platformVersionProvider: platformVersionProvider + ) + } - init(storage: Storage, platformVersionProvider: PlatformVersionProvider) { - self.storage = storage + // defaultLocalization is currently shared across the entire package + // this may need to be enhanced if / when we support localization per target or product + self.defaultLocalization = defaultLocalization + self.platforms = platforms self.platformVersionProvider = platformVersionProvider } @@ -150,32 +111,13 @@ public final class ResolvedProduct { } private static func computePlatforms(targets: [ResolvedTarget]) -> ([SupportedPlatform], PlatformVersionProvider) { - // merging two sets of supported platforms, preferring the max constraint - func merge(into partial: inout [SupportedPlatform], platforms: [SupportedPlatform]) { - for platformSupport in platforms { - if let existing = partial.firstIndex(where: { $0.platform == platformSupport.platform }) { - if partial[existing].version < platformSupport.version { - partial.remove(at: existing) - partial.append(platformSupport) - } - } else { - partial.append(platformSupport) - } - } - } - - let declared = targets.reduce(into: [SupportedPlatform]()) { partial, item in - merge(into: &partial, platforms: item.platforms) + let declaredPlatforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in + merge(into: &partial, platforms: item.supportedPlatforms) } return ( - declared.sorted(by: { $0.platform.name < $1.platform.name }), - .init(derivedXCTestPlatformProvider: { declared in - let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in - merge(into: &partial, platforms: [item.getDerived(for: declared, usingXCTest: item.type == .test)]) - } - return platforms.first!.version - }) + declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }), + PlatformVersionProvider(implementation: .mergingFromTargets(targets)) ) } @@ -197,16 +139,6 @@ public final class ResolvedProduct { } } -extension ResolvedProduct: Hashable { - public func hash(into hasher: inout Hasher) { - hasher.combine(self.storage) - } - - public static func == (lhs: ResolvedProduct, rhs: ResolvedProduct) -> Bool { - lhs.storage == rhs.storage - } -} - extension ResolvedProduct: CustomStringConvertible { public var description: String { "" diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 3984bf80064..14bc7ce15ac 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -15,7 +15,7 @@ import PackageModel import func TSCBasic.topologicalSort /// Represents a fully resolved target. All the dependencies for the target are resolved. -public final class ResolvedTarget { +public struct ResolvedTarget: Hashable { /// Represents dependency of a resolved target. public enum Dependency: Hashable { /// Direct dependency of the target. This target is in the same package and should be statically linked. @@ -129,90 +129,44 @@ public final class ResolvedTarget { return underlying.sources } - /// The dependencies of this target. - public var dependencies: [Dependency] { - self.storage.dependencies - } + /// The underlying target represented in this resolved target. + public let underlying: Target - /// The underlying target model represented in this resolved target. - public var underlying: Target { - self.storage.underlying - } + /// The dependencies of this target. + public let dependencies: [Dependency] /// The default localization for resources. - public var defaultLocalization: String? { - self.storage.defaultLocalization - } + public let defaultLocalization: String? /// The list of platforms that are supported by this target. - public var platforms: [SupportedPlatform] { - self.storage.supportedPlatforms - } - - struct Storage: Hashable { - /// The underlying target represented in this resolved target. - let underlying: Target - - /// The dependencies of this target. - let dependencies: [Dependency] - - /// The default localization for resources. - let defaultLocalization: String? - - /// The list of platforms that are supported by this target. - let supportedPlatforms: [SupportedPlatform] - } - - private let storage: Storage + public let supportedPlatforms: [SupportedPlatform] private let platformVersionProvider: PlatformVersionProvider - /// Create a target instance. - public convenience init( - target: Target, - dependencies: [Dependency], - defaultLocalization: String?, - platforms: [SupportedPlatform], + public init( + underlying: Target, + dependencies: [ResolvedTarget.Dependency], + defaultLocalization: String? = nil, + supportedPlatforms: [SupportedPlatform], platformVersionProvider: PlatformVersionProvider ) { - self.init( - storage: .init( - underlying: target, - dependencies: dependencies, - defaultLocalization: defaultLocalization, - supportedPlatforms: platforms - ), - platformVersionProvider: platformVersionProvider - ) - } - - init( - storage: Storage, - platformVersionProvider: PlatformVersionProvider - ) { - self.storage = storage + self.underlying = underlying + self.dependencies = dependencies + self.defaultLocalization = defaultLocalization + self.supportedPlatforms = supportedPlatforms self.platformVersionProvider = platformVersionProvider } + public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.storage.supportedPlatforms, + declared: self.supportedPlatforms, for: platform, usingXCTest: usingXCTest ) } } -extension ResolvedTarget: Hashable { - public func hash(into hasher: inout Hasher) { - hasher.combine(self.storage) - } - - public static func == (lhs: ResolvedTarget, rhs: ResolvedTarget) -> Bool { - lhs.storage == rhs.storage - } -} - extension ResolvedTarget: CustomStringConvertible { public var description: String { return "" diff --git a/Tests/PackageGraphTests/TargetTests.swift b/Tests/PackageGraphTests/TargetTests.swift index f09d5f119e6..b66e07794e5 100644 --- a/Tests/PackageGraphTests/TargetTests.swift +++ b/Tests/PackageGraphTests/TargetTests.swift @@ -16,9 +16,9 @@ import PackageGraph import PackageModel private extension ResolvedTarget { - convenience init(name: String, deps: ResolvedTarget...) { + init(name: String, deps: ResolvedTarget...) { self.init( - target: SwiftTarget( + underlying: SwiftTarget( name: name, type: .library, path: .root, From 27d4f68eac9644c88717b9c4f16805b7f4c6ea3b Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sat, 2 Dec 2023 22:56:57 +0000 Subject: [PATCH 13/24] Minor cleanups --- .../PackageGraph/PackageGraph+Loading.swift | 3 +-- .../Resolution/ResolvedTarget.swift | 2 +- Sources/PackageModel/PlatformRegistry.swift | 18 +++++++++++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index c20f0080e34..86921dab766 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -381,6 +381,7 @@ private func createResolvedPackages( targetBuilder.dependencies += try targetBuilder.target.dependencies.compactMap { dependency in switch dependency { case .target(let target, let conditions): + try targetBuilder.target.validateDependency(target: target) guard let targetBuilder = targetMap[target] else { throw InternalError("unknown target \(target.name)") } @@ -908,7 +909,6 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { let dependencies = try self.dependencies.map { dependency -> ResolvedTarget.Dependency in switch dependency { case .target(let targetBuilder, let conditions): - try self.target.validateDependency(target: targetBuilder.target) return .target(try targetBuilder.construct(), conditions: conditions) case .product(let productBuilder, let conditions): try self.target.validateDependency( @@ -948,7 +948,6 @@ extension Target { } /// Builder for resolved package. private final class ResolvedPackageBuilder: ResolvedBuilder { - /// The package reference. let package: Package diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 14bc7ce15ac..d34a2874bcd 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -14,7 +14,7 @@ import PackageModel import func TSCBasic.topologicalSort -/// Represents a fully resolved target. All the dependencies for the target are resolved. +/// Represents a fully resolved target. All the dependencies for this target are also stored as resolved. public struct ResolvedTarget: Hashable { /// Represents dependency of a resolved target. public enum Dependency: Hashable { diff --git a/Sources/PackageModel/PlatformRegistry.swift b/Sources/PackageModel/PlatformRegistry.swift index c56864b292b..eec668ddb92 100644 --- a/Sources/PackageModel/PlatformRegistry.swift +++ b/Sources/PackageModel/PlatformRegistry.swift @@ -11,8 +11,7 @@ //===----------------------------------------------------------------------===// /// A registry for available platforms. -public final class PlatformRegistry { - +public struct PlatformRegistry { /// The current registry is hardcoded and static so we can just use /// a singleton for now. public static let `default`: PlatformRegistry = .init() @@ -31,6 +30,19 @@ public final class PlatformRegistry { /// The static list of known platforms. private static var _knownPlatforms: [Platform] { - return [.macOS, .macCatalyst, .iOS, .tvOS, .watchOS, .visionOS, .linux, .windows, .android, .wasi, .driverKit, .openbsd] + [ + .macOS, + .macCatalyst, + .iOS, + .tvOS, + .watchOS, + .visionOS, + .linux, + .windows, + .android, + .wasi, + .driverKit, + .openbsd + ] } } From b6c2698420ec5d56d932be4faba803ce87588ec7 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Sun, 3 Dec 2023 10:26:53 +0000 Subject: [PATCH 14/24] Fix build errors --- .../PackageGraph/PackageGraph+Loading.swift | 32 ++++++++++++------- .../Resolution/ResolvedPackage.swift | 12 +++---- .../Resolution/ResolvedProduct.swift | 10 +++--- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index 86921dab766..29936bd7eb6 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -927,24 +927,34 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { underlying: self.target, dependencies: dependencies, defaultLocalization: self.defaultLocalization, - supportedPlatforms: self.platforms, + supportedPlatforms: self.supportedPlatforms, platformVersionProvider: self.platformVersionProvider ) } } extension Target { - - func validateDependency(target: Target) throws { - if self.type == .plugin && target.type == .library { - throw PackageGraphError.unsupportedPluginDependency(targetName: self.name, dependencyName: target.name, dependencyType: target.type.rawValue, dependencyPackage: nil) + func validateDependency(target: Target) throws { + if self.type == .plugin && target.type == .library { + throw PackageGraphError.unsupportedPluginDependency( + targetName: self.name, + dependencyName: target.name, + dependencyType: target.type.rawValue, + dependencyPackage: nil + ) + } } - } - func validateDependency(product: Product, productPackage: PackageIdentity) throws { - if self.type == .plugin && product.type.isLibrary { - throw PackageGraphError.unsupportedPluginDependency(targetName: self.name, dependencyName: product.name, dependencyType: product.type.description, dependencyPackage: productPackage.description) + + func validateDependency(product: Product, productPackage: PackageIdentity) throws { + if self.type == .plugin && product.type.isLibrary { + throw PackageGraphError.unsupportedPluginDependency( + targetName: self.name, + dependencyName: product.name, + dependencyType: product.type.description, + dependencyPackage: productPackage.description + ) + } } - } } /// Builder for resolved package. private final class ResolvedPackageBuilder: ResolvedBuilder { @@ -999,7 +1009,7 @@ private final class ResolvedPackageBuilder: ResolvedBuilder { override func constructImpl() throws -> ResolvedPackage { return ResolvedPackage( - package: self.package, + underlying: self.package, defaultLocalization: self.defaultLocalization, supportedPlatforms: self.supportedPlatforms, dependencies: try self.dependencies.map{ try $0.construct() }, diff --git a/Sources/PackageGraph/Resolution/ResolvedPackage.swift b/Sources/PackageGraph/Resolution/ResolvedPackage.swift index 83ecd2d7c21..96a9876af19 100644 --- a/Sources/PackageGraph/Resolution/ResolvedPackage.swift +++ b/Sources/PackageGraph/Resolution/ResolvedPackage.swift @@ -46,7 +46,7 @@ public struct ResolvedPackage: Hashable { public let defaultLocalization: String? /// The list of platforms that are supported by this target. - public let platforms: [SupportedPlatform] + public let supportedPlatforms: [SupportedPlatform] /// If the given package's source is a registry release, this provides additional metadata and signature information. public let registryMetadata: RegistryReleaseMetadata? @@ -55,28 +55,28 @@ public struct ResolvedPackage: Hashable { private let platformVersionProvider: PlatformVersionProvider public init( - package: Package, + underlying: Package, defaultLocalization: String?, - platforms: [SupportedPlatform], + supportedPlatforms: [SupportedPlatform], dependencies: [ResolvedPackage], targets: [ResolvedTarget], products: [ResolvedProduct], registryMetadata: RegistryReleaseMetadata?, platformVersionProvider: PlatformVersionProvider ) { - self.underlying = package + self.underlying = underlying self.targets = targets self.products = products self.dependencies = dependencies self.defaultLocalization = defaultLocalization - self.platforms = platforms + self.supportedPlatforms = supportedPlatforms self.registryMetadata = registryMetadata self.platformVersionProvider = platformVersionProvider } public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.platforms, + declared: self.supportedPlatforms, for: platform, usingXCTest: usingXCTest ) diff --git a/Sources/PackageGraph/Resolution/ResolvedProduct.swift b/Sources/PackageGraph/Resolution/ResolvedProduct.swift index bcad4c9f313..18755b038c7 100644 --- a/Sources/PackageGraph/Resolution/ResolvedProduct.swift +++ b/Sources/PackageGraph/Resolution/ResolvedProduct.swift @@ -37,7 +37,7 @@ public struct ResolvedProduct: Hashable { public let defaultLocalization: String? /// The list of platforms that are supported by this product. - public let platforms: [SupportedPlatform] + public let supportedPlatforms: [SupportedPlatform] public let platformVersionProvider: PlatformVersionProvider @@ -60,7 +60,7 @@ public struct ResolvedProduct: Hashable { public init(product: Product, targets: [ResolvedTarget]) { assert(product.targets.count == targets.count && product.targets.map(\.name) == targets.map(\.name)) - let (platforms, platformVersionProvider) = Self.computePlatforms(targets: targets) + let (supportedPlatforms, platformVersionProvider) = Self.computePlatforms(targets: targets) let defaultLocalization = targets.first?.defaultLocalization self.underlying = product @@ -80,7 +80,7 @@ public struct ResolvedProduct: Hashable { .target($0, conditions: []) }, defaultLocalization: defaultLocalization, - supportedPlatforms: platforms, + supportedPlatforms: supportedPlatforms, platformVersionProvider: platformVersionProvider ) } @@ -88,7 +88,7 @@ public struct ResolvedProduct: Hashable { // defaultLocalization is currently shared across the entire package // this may need to be enhanced if / when we support localization per target or product self.defaultLocalization = defaultLocalization - self.platforms = platforms + self.supportedPlatforms = supportedPlatforms self.platformVersionProvider = platformVersionProvider } @@ -123,7 +123,7 @@ public struct ResolvedProduct: Hashable { public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( - declared: self.platforms, + declared: self.supportedPlatforms, for: platform, usingXCTest: usingXCTest ) From b777e126c5a514e88017a2286ccb6490d2cc6d2e Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 5 Dec 2023 17:26:12 +0000 Subject: [PATCH 15/24] Rename `getDerived` to `getSupportedPlatform` --- .../Build/BuildDescription/ProductBuildDescription.swift | 2 +- Sources/Build/BuildPlan/BuildPlan.swift | 6 +++--- .../PackageGraph/Resolution/PlatformVersionProvider.swift | 2 +- Sources/PackageGraph/Resolution/ResolvedPackage.swift | 2 +- Sources/PackageGraph/Resolution/ResolvedProduct.swift | 2 +- Sources/PackageGraph/Resolution/ResolvedTarget.swift | 2 +- Sources/SPMTestSupport/PackageGraphTester.swift | 8 ++++---- Sources/XCBuildSupport/PIFBuilder.swift | 6 +++--- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index a779f548070..161dba44b1a 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -284,7 +284,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription // When deploying to macOS prior to macOS 12, add an rpath to the // back-deployed concurrency libraries. if useStdlibRpath, self.buildParameters.targetTriple.isMacOSX { - let macOSSupportedPlatform = self.package.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest) + let macOSSupportedPlatform = self.package.getSupportedPlatform(for: .macOS, usingXCTest: product.isLinkingXCTest) if macOSSupportedPlatform.version.major < 12 { let backDeployedStdlib = try buildParameters.toolchain.macosSwiftStdlib .parentDirectory diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index a076bddb7b8..9d6d739c0c8 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -133,7 +133,7 @@ extension BuildParameters { // Compute the triple string for Darwin platform using the platform version. if targetTriple.isDarwin() { let platform = buildEnvironment.platform - let supportedPlatform = target.getDerived(for: platform, usingXCTest: target.type == .test) + let supportedPlatform = target.getSupportedPlatform(for: platform, usingXCTest: target.type == .test) args += [targetTriple.tripleString(forPlatformVersion: supportedPlatform.version.versionString)] } else { args += [targetTriple.tripleString] @@ -409,8 +409,8 @@ public class BuildPlan: SPMBuildCore.BuildPlan { ) throws { // Supported platforms are defined at the package level. // This will need to become a bit complicated once we have target-level or product-level platform support. - let productPlatform = product.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest) - let targetPlatform = target.getDerived(for: .macOS, usingXCTest: target.type == .test) + let productPlatform = product.getSupportedPlatform(for: .macOS, usingXCTest: product.isLinkingXCTest) + let targetPlatform = target.getSupportedPlatform(for: .macOS, usingXCTest: target.type == .test) // Check if the version requirement is satisfied. // diff --git a/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift b/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift index eac22178a1d..461f61ec377 100644 --- a/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift +++ b/Sources/PackageGraph/Resolution/PlatformVersionProvider.swift @@ -46,7 +46,7 @@ public struct PlatformVersionProvider: Hashable { switch implementation { case .mergingFromTargets(let targets): let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in - merge(into: &partial, platforms: [item.getDerived(for: declared, usingXCTest: item.type == .test)]) + merge(into: &partial, platforms: [item.getSupportedPlatform(for: declared, usingXCTest: item.type == .test)]) } return platforms.first!.version diff --git a/Sources/PackageGraph/Resolution/ResolvedPackage.swift b/Sources/PackageGraph/Resolution/ResolvedPackage.swift index 72916172972..481f5c7cda5 100644 --- a/Sources/PackageGraph/Resolution/ResolvedPackage.swift +++ b/Sources/PackageGraph/Resolution/ResolvedPackage.swift @@ -73,7 +73,7 @@ public final class ResolvedPackage { self.platformVersionProvider = platformVersionProvider } - public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + public func getSupportedPlatform(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( declared: self.supportedPlatforms, for: platform, diff --git a/Sources/PackageGraph/Resolution/ResolvedProduct.swift b/Sources/PackageGraph/Resolution/ResolvedProduct.swift index e677c5cc1a1..5c107b867e0 100644 --- a/Sources/PackageGraph/Resolution/ResolvedProduct.swift +++ b/Sources/PackageGraph/Resolution/ResolvedProduct.swift @@ -115,7 +115,7 @@ public final class ResolvedProduct { ) } - public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + public func getSupportedPlatform(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( declared: self.supportedPlatforms, for: platform, diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index d67e6335729..e79f1577818 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -158,7 +158,7 @@ public final class ResolvedTarget { self.platformVersionProvider = platformVersionProvider } - public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { + public func getSupportedPlatform(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform { self.platformVersionProvider.getDerived( declared: self.supportedPlatforms, for: platform, diff --git a/Sources/SPMTestSupport/PackageGraphTester.swift b/Sources/SPMTestSupport/PackageGraphTester.swift index 5af4a88eb99..0d480a6ea96 100644 --- a/Sources/SPMTestSupport/PackageGraphTester.swift +++ b/Sources/SPMTestSupport/PackageGraphTester.swift @@ -182,7 +182,7 @@ public final class ResolvedTargetResult { let derived = platforms.map { let platform = PlatformRegistry.default.platformByName[$0.key] ?? PackageModel.Platform .custom(name: $0.key, oldestSupportedVersion: $0.value) - return self.target.getDerived(for: platform, usingXCTest: self.target.type == .test) + return self.target.getSupportedPlatform(for: platform, usingXCTest: self.target.type == .test) } let targetPlatforms = Dictionary( uniqueKeysWithValues: derived @@ -192,7 +192,7 @@ public final class ResolvedTargetResult { } public func checkDerivedPlatformOptions(_ platform: PackageModel.Platform, options: [String], file: StaticString = #file, line: UInt = #line) { - let platform = target.getDerived(for: platform, usingXCTest: target.type == .test) + let platform = target.getSupportedPlatform(for: platform, usingXCTest: target.type == .test) XCTAssertEqual(platform.options, options, file: file, line: line) } } @@ -240,14 +240,14 @@ public final class ResolvedProductResult { public func checkDerivedPlatforms(_ platforms: [String: String], file: StaticString = #file, line: UInt = #line) { let derived = platforms.map { let platform = PlatformRegistry.default.platformByName[$0.key] ?? PackageModel.Platform.custom(name: $0.key, oldestSupportedVersion: $0.value) - return product.getDerived(for: platform, usingXCTest: product.isLinkingXCTest) + return product.getSupportedPlatform(for: platform, usingXCTest: product.isLinkingXCTest) } let targetPlatforms = Dictionary(uniqueKeysWithValues: derived.map({ ($0.platform.name, $0.version.versionString) })) XCTAssertEqual(platforms, targetPlatforms, file: file, line: line) } public func checkDerivedPlatformOptions(_ platform: PackageModel.Platform, options: [String], file: StaticString = #file, line: UInt = #line) { - let platform = product.getDerived(for: platform, usingXCTest: product.isLinkingXCTest) + let platform = product.getSupportedPlatform(for: platform, usingXCTest: product.isLinkingXCTest) XCTAssertEqual(platform.options, options, file: file, line: line) } } diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift index b984499c105..58beb74531d 100644 --- a/Sources/XCBuildSupport/PIFBuilder.swift +++ b/Sources/XCBuildSupport/PIFBuilder.swift @@ -301,7 +301,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder { PlatformRegistry.default.knownPlatforms.forEach { guard let platform = PIF.BuildSettings.Platform.from(platform: $0) else { return } - let supportedPlatform = package.getDerived(for: $0, usingXCTest: false) + let supportedPlatform = package.getSupportedPlatform(for: $0, usingXCTest: false) if !supportedPlatform.options.isEmpty { settings[.SPECIALIZATION_SDK_OPTIONS, for: platform] = supportedPlatform.options } @@ -1419,13 +1419,13 @@ extension Array where Element == ResolvedTarget.Dependency { extension ResolvedPackage { func deploymentTarget(for platform: PackageModel.Platform, usingXCTest: Bool = false) -> String? { - return self.getDerived(for: platform, usingXCTest: usingXCTest).version.versionString + return self.getSupportedPlatform(for: platform, usingXCTest: usingXCTest).version.versionString } } extension ResolvedTarget { func deploymentTarget(for platform: PackageModel.Platform, usingXCTest: Bool = false) -> String? { - return self.getDerived(for: platform, usingXCTest: usingXCTest).version.versionString + return self.getSupportedPlatform(for: platform, usingXCTest: usingXCTest).version.versionString } } From cce1f120e812b9b2ae186f340b32ffef37fee0fd Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 5 Dec 2023 21:51:46 +0000 Subject: [PATCH 16/24] Fix build errors after merge --- Sources/Build/BuildPlan/BuildPlan+Test.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index c7ab58b3075..d38f52eca40 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -122,7 +122,7 @@ extension BuildPlan { testEntryPointSources: entryPointSources ) let entryPointResolvedTarget = ResolvedTarget( - target: entryPointTarget, + underlying: entryPointTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) } + resolvedTargetDependencies, defaultLocalization: testProduct.defaultLocalization, supportedPlatforms: testProduct.supportedPlatforms, @@ -150,7 +150,7 @@ extension BuildPlan { resolvedTargetDependencies = [.target(discoveryTargets!.resolved, conditions: [])] case .swiftTesting: discoveryTargets = nil - swiftTargetDependencies = testProduct.targets.map { .target($0.underlyingTarget, conditions: []) } + swiftTargetDependencies = testProduct.targets.map { .target($0.underlying, conditions: []) } resolvedTargetDependencies = testProduct.targets.map { .target($0, conditions: []) } } @@ -165,7 +165,7 @@ extension BuildPlan { testEntryPointSources: entryPointResolvedTarget.underlying.sources ) let entryPointResolvedTarget = ResolvedTarget( - target: entryPointTarget, + underlying: entryPointTarget, dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies, defaultLocalization: testProduct.defaultLocalization, supportedPlatforms: testProduct.supportedPlatforms, From afee8e6446cd2a5b778288a8d7452b6320f21f96 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 5 Dec 2023 21:55:34 +0000 Subject: [PATCH 17/24] Address PR feedback --- .../BuildDescription/ClangTargetBuildDescription.swift | 7 +++---- .../BuildDescription/SwiftTargetBuildDescription.swift | 5 ++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift index 5baded0ef17..d14add20938 100644 --- a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift @@ -26,9 +26,7 @@ public final class ClangTargetBuildDescription { public let target: ResolvedTarget /// The underlying clang target. - public var clangTarget: ClangTarget { - target.underlying as! ClangTarget - } + public let clangTarget: ClangTarget /// The tools version of the package that declared the target. This can /// can be used to conditionalize semantically significant changes in how @@ -119,10 +117,11 @@ public final class ClangTargetBuildDescription { fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws { - guard target.underlying is ClangTarget else { + guard let clangTarget = target.underlying as? ClangTarget else { throw InternalError("underlying target type mismatch \(target)") } + self.clangTarget = clangTarget self.fileSystem = fileSystem self.target = target self.toolsVersion = toolsVersion diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index 3a53b1004e4..bc6c647da1e 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -33,6 +33,8 @@ public final class SwiftTargetBuildDescription { /// The target described by this target. public let target: ResolvedTarget + private let swiftTarget: SwiftTarget + /// The tools version of the package that declared the target. This can /// can be used to conditionalize semantically significant changes in how /// a target is built. @@ -250,10 +252,11 @@ public final class SwiftTargetBuildDescription { fileSystem: FileSystem, observabilityScope: ObservabilityScope ) throws { - guard target.underlying is SwiftTarget else { + guard let swiftTarget = target.underlying as? SwiftTarget else { throw InternalError("underlying target type mismatch \(target)") } + self.swiftTarget = swiftTarget self.package = package self.target = target self.toolsVersion = toolsVersion From 6dab954585ed2fb999c887fe188f242b3fced48d Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Tue, 5 Dec 2023 21:57:07 +0000 Subject: [PATCH 18/24] Address PR feedback --- .../Build/BuildDescription/SwiftTargetBuildDescription.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index bc6c647da1e..95b8a9c314a 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -138,7 +138,7 @@ public final class SwiftTargetBuildDescription { /// The swift version for this target. var swiftVersion: SwiftLanguageVersion { - (self.target.underlying as! SwiftTarget).swiftVersion + self.swiftTarget.swiftVersion } /// Describes the purpose of a test target, including any special roles such as containing a list of discovered From 477935a3029888d8aecd5a79141c6cb65c3b0e3c Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Wed, 6 Dec 2023 10:09:15 +0000 Subject: [PATCH 19/24] Sort cases in `PlatformRegistry`, use `init` in `PackageBuilder` --- Sources/PackageLoading/PackageBuilder.swift | 9 +++------ Sources/PackageModel/PlatformRegistry.swift | 14 +++++++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/Sources/PackageLoading/PackageBuilder.swift b/Sources/PackageLoading/PackageBuilder.swift index 4c62d15e657..9379a7d099d 100644 --- a/Sources/PackageLoading/PackageBuilder.swift +++ b/Sources/PackageLoading/PackageBuilder.swift @@ -1149,8 +1149,7 @@ public final class PackageBuilder { var conditions: [PackageCondition] = [] if let config = condition?.config.flatMap({ BuildConfiguration(rawValue: $0) }) { - let condition = PackageCondition(configuration: config) - conditions.append(condition) + conditions.append(.init(configuration: config)) } if let platforms = condition?.platformNames.map({ @@ -1159,11 +1158,9 @@ public final class PackageBuilder { } else { return PackageModel.Platform.custom(name: $0, oldestSupportedVersion: .unknown) } - }), - !platforms.isEmpty + }), !platforms.isEmpty { - let condition = PackageCondition(platforms: platforms) - conditions.append(condition) + conditions.append(.init(platforms: platforms)) } return conditions diff --git a/Sources/PackageModel/PlatformRegistry.swift b/Sources/PackageModel/PlatformRegistry.swift index eec668ddb92..aec3e04fcc6 100644 --- a/Sources/PackageModel/PlatformRegistry.swift +++ b/Sources/PackageModel/PlatformRegistry.swift @@ -31,18 +31,18 @@ public struct PlatformRegistry { /// The static list of known platforms. private static var _knownPlatforms: [Platform] { [ + .android, + .driverKit, + .iOS, + .linux, .macOS, .macCatalyst, - .iOS, + .openbsd, .tvOS, - .watchOS, .visionOS, - .linux, - .windows, - .android, .wasi, - .driverKit, - .openbsd + .watchOS, + .windows, ] } } From 5ecf03df952f574d8a6f940ac7b4397c9f6b2b40 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Thu, 14 Dec 2023 12:26:48 +0000 Subject: [PATCH 20/24] clean up formatting --- .../PackageGraphPerfTests.swift | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift index 9c7c6856094..390eed8663c 100644 --- a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift +++ b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift @@ -22,8 +22,7 @@ import class TSCBasic.InMemoryFileSystem import class TSCTestSupport.XCTestCasePerf -class PackageGraphPerfTests: XCTestCasePerf { - +final class PackageGraphPerfTests: XCTestCasePerf { func testLoading100Packages() throws { #if !os(macOS) try XCTSkipIf(true, "test is only supported on macOS") @@ -49,8 +48,16 @@ class PackageGraphPerfTests: XCTestCasePerf { } else { let depName = "Foo\(pkg + 1)" let depUrl = "/\(depName)" - dependencies = [.localSourceControl(deprecatedName: depName, path: try .init(validating: depUrl), requirement: .upToNextMajor(from: "1.0.0"))] - targets = [try TargetDescription(name: name, dependencies: [.byName(name: depName, condition: nil)], path: ".")] + dependencies = [.localSourceControl( + deprecatedName: depName, + path: try .init(validating: depUrl), + requirement: .upToNextMajor(from: "1.0.0") + )] + targets = [try TargetDescription( + name: name, + dependencies: [.byName(name: depName, condition: nil)], + path: "." + )] } // Create manifest. let isRoot = pkg == 1 @@ -79,7 +86,11 @@ class PackageGraphPerfTests: XCTestCasePerf { measure { let observability = ObservabilitySystem.makeForTesting() let g = try! PackageGraph.load( - root: PackageGraphRoot(input: PackageGraphRootInput(packages: [rootManifest.path]), manifests: [rootManifest.path: rootManifest], observabilityScope: observability.topScope), + root: PackageGraphRoot( + input: PackageGraphRootInput(packages: [rootManifest.path]), + manifests: [rootManifest.path: rootManifest], + observabilityScope: observability.topScope + ), identityResolver: identityResolver, externalManifests: externalManifests, binaryArtifacts: [:], @@ -95,7 +106,10 @@ class PackageGraphPerfTests: XCTestCasePerf { let lastPackageNumber = 20 let packageNumberSequence = (1...lastPackageNumber) - let fs = InMemoryFileSystem(emptyFiles: packageNumberSequence.map({ "/Package\($0)/Sources/Target\($0)/s.swift" }) + ["/PackageA/Sources/TargetA/s.swift"]) + let fs = InMemoryFileSystem( + emptyFiles: packageNumberSequence.map({ "/Package\($0)/Sources/Target\($0)/s.swift" }) + + ["/PackageA/Sources/TargetA/s.swift"] + ) let packageSequence: [Manifest] = try packageNumberSequence.map { (sequenceNumber: Int) -> Manifest in let dependencySequence = sequenceNumber < lastPackageNumber ? Array((sequenceNumber + 1)...lastPackageNumber) : [] @@ -105,10 +119,17 @@ class PackageGraphPerfTests: XCTestCasePerf { toolsVersion: .v5_7, dependencies: try dependencySequence.map({ .fileSystem(path: try .init(validating: "/Package\($0)")) }), products: [ - try .init(name: "Package\(sequenceNumber)", type: .library(.dynamic), targets: ["Target\(sequenceNumber)"]) + try .init( + name: "Package\(sequenceNumber)", + type: .library(.dynamic), + targets: ["Target\(sequenceNumber)"] + ) ], targets: [ - try .init(name: "Target\(sequenceNumber)", dependencies: dependencySequence.map { .product(name: "Target\($0)", package: "Package\($0)") }) + try .init(name: "Target\(sequenceNumber)", + dependencies: dependencySequence.map { + .product(name: "Target\($0)", package: "Package\($0)") + }) ] ) } @@ -127,7 +148,11 @@ class PackageGraphPerfTests: XCTestCasePerf { measure { do { for _ in 0.. Date: Thu, 14 Dec 2023 16:26:18 +0000 Subject: [PATCH 21/24] Add dependency equality test in `TargetTests` --- Tests/PackageGraphTests/TargetTests.swift | 31 +++++++---------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/Tests/PackageGraphTests/TargetTests.swift b/Tests/PackageGraphTests/TargetTests.swift index ab4eac8415d..51a6d8df361 100644 --- a/Tests/PackageGraphTests/TargetTests.swift +++ b/Tests/PackageGraphTests/TargetTests.swift @@ -16,7 +16,7 @@ import PackageGraph import PackageModel private extension ResolvedTarget { - init(name: String, deps: ResolvedTarget...) { + init(name: String, deps: ResolvedTarget..., conditions: [PackageCondition] = []) { self.init( underlying: SwiftTarget( name: name, @@ -36,29 +36,17 @@ private extension ResolvedTarget { } } -func testTargets(file: StaticString = #file, line: UInt = #line, body: () throws -> Void) { - do { - try body() - } catch { - XCTFail("\(error)", file: file, line: line) - } -} - class TargetDependencyTests: XCTestCase { - func test1() throws { - testTargets { let t1 = ResolvedTarget(name: "t1") let t2 = ResolvedTarget(name: "t2", deps: t1) let t3 = ResolvedTarget(name: "t3", deps: t2) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) - } } func test2() throws { - testTargets { let t1 = ResolvedTarget(name: "t1") let t2 = ResolvedTarget(name: "t2", deps: t1) let t3 = ResolvedTarget(name: "t3", deps: t2, t1) @@ -67,11 +55,9 @@ class TargetDependencyTests: XCTestCase { XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) - } } func test3() throws { - testTargets { let t1 = ResolvedTarget(name: "t1") let t2 = ResolvedTarget(name: "t2", deps: t1) let t3 = ResolvedTarget(name: "t3", deps: t2, t1) @@ -80,11 +66,9 @@ class TargetDependencyTests: XCTestCase { XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) - } } func test4() throws { - testTargets { let t1 = ResolvedTarget(name: "t1") let t2 = ResolvedTarget(name: "t2", deps: t1) let t3 = ResolvedTarget(name: "t3", deps: t2) @@ -93,11 +77,9 @@ class TargetDependencyTests: XCTestCase { XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) - } } func test5() throws { - testTargets { let t1 = ResolvedTarget(name: "t1") let t2 = ResolvedTarget(name: "t2", deps: t1) let t3 = ResolvedTarget(name: "t3", deps: t2) @@ -117,11 +99,9 @@ class TargetDependencyTests: XCTestCase { XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) - } } func test6() throws { - testTargets { let t1 = ResolvedTarget(name: "t1") let t2 = ResolvedTarget(name: "t2", deps: t1) let t3 = ResolvedTarget(name: "t3", deps: t2) @@ -141,6 +121,13 @@ class TargetDependencyTests: XCTestCase { XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) - } + } + + func testConditions() throws { + let t1 = ResolvedTarget(name: "t1") + let t2 = ResolvedTarget(name: "t2", deps: t1) + let t2Conditions = ResolvedTarget(name: "t2", deps: t1, conditions: [.init(platforms: [.linux])]) + + XCTAssertEqual(t2, t2Conditions) } } From ad5792c06346fa6701dd2b24d8d13e03198627bd Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Thu, 14 Dec 2023 16:54:58 +0000 Subject: [PATCH 22/24] Fix target dependencies equality --- Sources/Build/BuildPlan/BuildPlan+Test.swift | 3 + .../PackageGraph/PackageGraph+Loading.swift | 8 ++ .../Resolution/ResolvedProduct.swift | 6 +- .../Resolution/ResolvedTarget.swift | 52 ++++++++++++- Tests/PackageGraphTests/TargetTests.swift | 78 +++++++++++-------- 5 files changed, 111 insertions(+), 36 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index e2eddadab4c..93fd9da29c7 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -85,6 +85,7 @@ extension BuildPlan { testDiscoverySrc: Sources(paths: discoveryPaths, root: discoveryDerivedDir) ) let discoveryResolvedTarget = ResolvedTarget( + packageIdentity: testProduct.packageIdentity, underlying: discoveryTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) }, defaultLocalization: testProduct.defaultLocalization, @@ -124,6 +125,7 @@ extension BuildPlan { testEntryPointSources: entryPointSources ) let entryPointResolvedTarget = ResolvedTarget( + packageIdentity: testProduct.packageIdentity, underlying: entryPointTarget, dependencies: testProduct.targets.map { .target($0, conditions: []) } + resolvedTargetDependencies, defaultLocalization: testProduct.defaultLocalization, @@ -168,6 +170,7 @@ extension BuildPlan { testEntryPointSources: entryPointResolvedTarget.underlying.sources ) let entryPointResolvedTarget = ResolvedTarget( + packageIdentity: testProduct.packageIdentity, underlying: entryPointTarget, dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies, defaultLocalization: testProduct.defaultLocalization, diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift index 7a84802091a..1e82171692f 100644 --- a/Sources/PackageGraph/PackageGraph+Loading.swift +++ b/Sources/PackageGraph/PackageGraph+Loading.swift @@ -373,6 +373,7 @@ private func createResolvedPackages( // Create target builders for each target in the package. let targetBuilders = package.targets.map { ResolvedTargetBuilder( + packageIdentity: package.identity, target: $0, observabilityScope: packageObservabilityScope, platformVersionProvider: platformVersionProvider @@ -861,6 +862,7 @@ private final class ResolvedProductBuilder: ResolvedBuilder { override func constructImpl() throws -> ResolvedProduct { return ResolvedProduct( + packageIdentity: packageBuilder.package.identity, product: product, targets: try targets.map{ try $0.construct() } ) @@ -879,6 +881,9 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { case product(_ product: ResolvedProductBuilder, conditions: [PackageCondition]) } + /// The reference to its package. + let packageIdentity: PackageIdentity + /// The target reference. let target: Target @@ -895,10 +900,12 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { let platformVersionProvider: PlatformVersionProvider init( + packageIdentity: PackageIdentity, target: Target, observabilityScope: ObservabilityScope, platformVersionProvider: PlatformVersionProvider ) { + self.packageIdentity = packageIdentity self.target = target self.observabilityScope = observabilityScope self.platformVersionProvider = platformVersionProvider @@ -929,6 +936,7 @@ private final class ResolvedTargetBuilder: ResolvedBuilder { } return ResolvedTarget( + packageIdentity: self.packageIdentity, underlying: self.target, dependencies: dependencies, defaultLocalization: self.defaultLocalization, diff --git a/Sources/PackageGraph/Resolution/ResolvedProduct.swift b/Sources/PackageGraph/Resolution/ResolvedProduct.swift index 3872d0d1b5b..8800c473031 100644 --- a/Sources/PackageGraph/Resolution/ResolvedProduct.swift +++ b/Sources/PackageGraph/Resolution/ResolvedProduct.swift @@ -24,6 +24,8 @@ public struct ResolvedProduct: Hashable { self.underlying.type } + public let packageIdentity: PackageIdentity + /// The underlying product. public let underlying: Product @@ -61,8 +63,9 @@ public struct ResolvedProduct: Hashable { } } - public init(product: Product, targets: [ResolvedTarget]) { + public init(packageIdentity: PackageIdentity, product: Product, targets: [ResolvedTarget]) { assert(product.targets.count == targets.count && product.targets.map(\.name) == targets.map(\.name)) + self.packageIdentity = packageIdentity self.underlying = product self.targets = targets @@ -85,6 +88,7 @@ public struct ResolvedProduct: Hashable { testEntryPointPath: testEntryPointPath ) return ResolvedTarget( + packageIdentity: packageIdentity, underlying: swiftTarget, dependencies: targets.map { .target($0, conditions: []) }, defaultLocalization: defaultLocalization ?? .none, // safe since this is a derived product diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index 809e40beed7..b8c747f5e8d 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -17,7 +17,7 @@ import func TSCBasic.topologicalSort /// Represents a fully resolved target. All the dependencies for this target are also stored as resolved. public struct ResolvedTarget: Hashable { /// Represents dependency of a resolved target. - public enum Dependency: Hashable { + public enum Dependency { /// Direct dependency of the target. This target is in the same package and should be statically linked. case target(_ target: ResolvedTarget, conditions: [PackageCondition]) @@ -129,6 +129,8 @@ public struct ResolvedTarget: Hashable { return underlying.sources } + let packageIdentity: PackageIdentity + /// The underlying target represented in this resolved target. public let underlying: Target @@ -148,12 +150,14 @@ public struct ResolvedTarget: Hashable { /// Create a resolved target instance. public init( + packageIdentity: PackageIdentity, underlying: Target, dependencies: [ResolvedTarget.Dependency], defaultLocalization: String? = nil, supportedPlatforms: [SupportedPlatform], platformVersionProvider: PlatformVersionProvider ) { + self.packageIdentity = packageIdentity self.underlying = underlying self.dependencies = dependencies self.defaultLocalization = defaultLocalization @@ -190,3 +194,49 @@ extension ResolvedTarget.Dependency: CustomStringConvertible { return str } } + +extension ResolvedTarget.Dependency: Identifiable { + public struct ID: Hashable { + enum Kind: Hashable { + case target + case product + } + + let kind: Kind + let packageIdentity: PackageIdentity + let name: String + } + + public var id: ID { + switch self { + case .target(let target, _): + return .init(kind: .target, packageIdentity: target.packageIdentity, name: target.name) + case .product(let product, _): + return .init(kind: .product, packageIdentity: product.packageIdentity, name: product.name) + } + } +} + +extension ResolvedTarget.Dependency: Equatable { + public static func == (lhs: ResolvedTarget.Dependency, rhs: ResolvedTarget.Dependency) -> Bool { + switch (lhs, rhs) { + case (.target(let lhsTarget, _), .target(let rhsTarget, _)): + return lhsTarget == rhsTarget + case (.product(let lhsProduct, _), .product(let rhsProduct, _)): + return lhsProduct == rhsProduct + case (.product, .target), (.target, .product): + return false + } + } +} + +extension ResolvedTarget.Dependency: Hashable { + public func hash(into hasher: inout Hasher) { + switch self { + case .target(let target, _): + hasher.combine(target) + case .product(let product, _): + hasher.combine(product) + } + } +} diff --git a/Tests/PackageGraphTests/TargetTests.swift b/Tests/PackageGraphTests/TargetTests.swift index 51a6d8df361..6a24dacbe27 100644 --- a/Tests/PackageGraphTests/TargetTests.swift +++ b/Tests/PackageGraphTests/TargetTests.swift @@ -13,11 +13,12 @@ import XCTest import PackageGraph -import PackageModel +@testable import PackageModel private extension ResolvedTarget { - init(name: String, deps: ResolvedTarget..., conditions: [PackageCondition] = []) { + init(packageIdentity: String, name: String, deps: ResolvedTarget..., conditions: [PackageCondition] = []) { self.init( + packageIdentity: .init(packageIdentity), underlying: SwiftTarget( name: name, type: .library, @@ -28,7 +29,7 @@ private extension ResolvedTarget { swiftVersion: .v4, usesUnsafeFlags: false ), - dependencies: deps.map { .target($0, conditions: []) }, + dependencies: deps.map { .target($0, conditions: conditions) }, defaultLocalization: nil, supportedPlatforms: [], platformVersionProvider: .init(implementation: .minimumDeploymentTargetDefault) @@ -38,19 +39,19 @@ private extension ResolvedTarget { class TargetDependencyTests: XCTestCase { func test1() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t3 = ResolvedTarget(name: "t3", deps: t2) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func test2() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t3 = ResolvedTarget(name: "t3", deps: t2, t1) - let t4 = ResolvedTarget(name: "t4", deps: t2, t3, t1) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t2, t3, t1) XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) @@ -58,10 +59,10 @@ class TargetDependencyTests: XCTestCase { } func test3() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t3 = ResolvedTarget(name: "t3", deps: t2, t1) - let t4 = ResolvedTarget(name: "t4", deps: t1, t2, t3) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t1, t2, t3) XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) @@ -69,10 +70,10 @@ class TargetDependencyTests: XCTestCase { } func test4() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t3 = ResolvedTarget(name: "t3", deps: t2) - let t4 = ResolvedTarget(name: "t4", deps: t3) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) @@ -80,12 +81,12 @@ class TargetDependencyTests: XCTestCase { } func test5() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t3 = ResolvedTarget(name: "t3", deps: t2) - let t4 = ResolvedTarget(name: "t4", deps: t3) - let t5 = ResolvedTarget(name: "t5", deps: t2) - let t6 = ResolvedTarget(name: "t6", deps: t5, t4) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) + let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2) + let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t5, t4) // precise order is not important, but it is important that the following are true let t6rd = try t6.recursiveTargetDependencies() @@ -102,12 +103,12 @@ class TargetDependencyTests: XCTestCase { } func test6() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t3 = ResolvedTarget(name: "t3", deps: t2) - let t4 = ResolvedTarget(name: "t4", deps: t3) - let t5 = ResolvedTarget(name: "t5", deps: t2) - let t6 = ResolvedTarget(name: "t6", deps: t4, t5) // same as above, but these two swapped + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) + let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2) + let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t4, t5) // same as above, but these two swapped // precise order is not important, but it is important that the following are true let t6rd = try t6.recursiveTargetDependencies() @@ -124,10 +125,19 @@ class TargetDependencyTests: XCTestCase { } func testConditions() throws { - let t1 = ResolvedTarget(name: "t1") - let t2 = ResolvedTarget(name: "t2", deps: t1) - let t2Conditions = ResolvedTarget(name: "t2", deps: t1, conditions: [.init(platforms: [.linux])]) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t2NoConditions = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t2WithConditions = ResolvedTarget( + packageIdentity: "pkg", + name: "t2", + deps: t1, + conditions: [.init(platforms: [.linux])] + ) - XCTAssertEqual(t2, t2Conditions) + // FIXME: we should test for actual `t2` and `t2NoConditions` equality, but `SwiftTarget` is a reference type, + // which currently breaks this test, and it shouldn't + XCTAssertEqual(t2.dependencies, t2NoConditions.dependencies) + XCTAssertEqual(t2.dependencies, t2WithConditions.dependencies) } } From 02d5dfb2bc43bf282e4f83f0ace2aed86f2109f0 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Thu, 14 Dec 2023 17:19:31 +0000 Subject: [PATCH 23/24] Use `topologicalSort` on `Identifiable`, fix `PackageGraphPerfTests` --- Sources/PackageGraph/PackageGraph.swift | 69 ++++++++++++++++++- .../Resolution/ResolvedPackage.swift | 4 ++ .../Resolution/ResolvedTarget.swift | 6 +- .../PackageGraphPerfTests.swift | 10 +-- .../TopologicalSortTests.swift | 64 +++++++++++++++++ 5 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 Tests/PackageGraphTests/TopologicalSortTests.swift diff --git a/Sources/PackageGraph/PackageGraph.swift b/Sources/PackageGraph/PackageGraph.swift index 1263d09b039..63c0503ccbe 100644 --- a/Sources/PackageGraph/PackageGraph.swift +++ b/Sources/PackageGraph/PackageGraph.swift @@ -10,11 +10,10 @@ // //===----------------------------------------------------------------------===// +import OrderedCollections import PackageLoading import PackageModel -import func TSCBasic.topologicalSort - enum PackageGraphError: Swift.Error { /// Indicates a non-root package with no targets. case noModules(Package) @@ -276,3 +275,69 @@ extension PackageGraphError: CustomStringConvertible { } } } + +enum GraphError: Error { + /// A cycle was detected in the input. + case unexpectedCycle +} + +/// Perform a topological sort of an graph. +/// +/// This function is optimized for use cases where cycles are unexpected, and +/// does not attempt to retain information on the exact nodes in the cycle. +/// +/// - Parameters: +/// - nodes: The list of input nodes to sort. +/// - successors: A closure for fetching the successors of a particular node. +/// +/// - Returns: A list of the transitive closure of nodes reachable from the +/// inputs, ordered such that every node in the list follows all of its +/// predecessors. +/// +/// - Throws: GraphError.unexpectedCycle +/// +/// - Complexity: O(v + e) where (v, e) are the number of vertices and edges +/// reachable from the input nodes via the relation. +func topologicalSort( + _ nodes: [T], successors: (T) throws -> [T] +) throws -> [T] { + // Implements a topological sort via recursion and reverse postorder DFS. + func visit(_ node: T, + _ stack: inout OrderedSet, _ visited: inout Set, _ result: inout [T], + _ successors: (T) throws -> [T]) throws { + // Mark this node as visited -- we are done if it already was. + if !visited.insert(node.id).inserted { + return + } + + // Otherwise, visit each adjacent node. + for succ in try successors(node) { + guard stack.append(succ.id).inserted else { + // If the successor is already in this current stack, we have found a cycle. + // + // FIXME: We could easily include information on the cycle we found here. + throw GraphError.unexpectedCycle + } + try visit(succ, &stack, &visited, &result, successors) + let popped = stack.removeLast() + assert(popped == succ.id) + } + + // Add to the result. + result.append(node) + } + + // FIXME: This should use a stack not recursion. + var visited = Set() + var result = [T]() + var stack = OrderedSet() + for node in nodes { + precondition(stack.isEmpty) + stack.append(node.id) + try visit(node, &stack, &visited, &result, successors) + let popped = stack.removeLast() + assert(popped == node.id) + } + + return result.reversed() +} diff --git a/Sources/PackageGraph/Resolution/ResolvedPackage.swift b/Sources/PackageGraph/Resolution/ResolvedPackage.swift index 35c144945f9..9ecd70e5bbb 100644 --- a/Sources/PackageGraph/Resolution/ResolvedPackage.swift +++ b/Sources/PackageGraph/Resolution/ResolvedPackage.swift @@ -87,3 +87,7 @@ extension ResolvedPackage: CustomStringConvertible { return "" } } + +extension ResolvedPackage: Identifiable { + public var id: PackageIdentity { self.underlying.identity } +} diff --git a/Sources/PackageGraph/Resolution/ResolvedTarget.swift b/Sources/PackageGraph/Resolution/ResolvedTarget.swift index b8c747f5e8d..63e53badfd0 100644 --- a/Sources/PackageGraph/Resolution/ResolvedTarget.swift +++ b/Sources/PackageGraph/Resolution/ResolvedTarget.swift @@ -84,12 +84,12 @@ public struct ResolvedTarget: Hashable { /// Returns the recursive dependencies, across the whole package-graph. public func recursiveDependencies() throws -> [Dependency] { - return try topologicalSort(self.dependencies) { $0.dependencies } + return try TSCBasic.topologicalSort(self.dependencies) { $0.dependencies } } /// Returns the recursive target dependencies, across the whole package-graph. public func recursiveTargetDependencies() throws -> [ResolvedTarget] { - return try topologicalSort(self.dependencies) { $0.dependencies }.compactMap { $0.target } + return try TSCBasic.topologicalSort(self.dependencies) { $0.dependencies }.compactMap { $0.target } } /// Returns the recursive dependencies, across the whole package-graph, which satisfy the input build environment, @@ -97,7 +97,7 @@ public struct ResolvedTarget: Hashable { /// - Parameters: /// - environment: The build environment to use to filter dependencies on. public func recursiveDependencies(satisfying environment: BuildEnvironment) throws -> [Dependency] { - return try topologicalSort(dependencies(satisfying: environment)) { dependency in + return try TSCBasic.topologicalSort(dependencies(satisfying: environment)) { dependency in return dependency.dependencies.filter { $0.satisfies(environment) } } } diff --git a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift index 390eed8663c..1b1c20ff25b 100644 --- a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift +++ b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift @@ -126,10 +126,12 @@ final class PackageGraphPerfTests: XCTestCasePerf { ) ], targets: [ - try .init(name: "Target\(sequenceNumber)", - dependencies: dependencySequence.map { - .product(name: "Target\($0)", package: "Package\($0)") - }) + try .init( + name: "Target\(sequenceNumber)", + dependencies: dependencySequence.map { + .product(name: "Target\($0)", package: "Package\($0)") + } + ) ] ) } diff --git a/Tests/PackageGraphTests/TopologicalSortTests.swift b/Tests/PackageGraphTests/TopologicalSortTests.swift new file mode 100644 index 00000000000..5877110fd18 --- /dev/null +++ b/Tests/PackageGraphTests/TopologicalSortTests.swift @@ -0,0 +1,64 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2016-2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See http://swift.org/LICENSE.txt for license information +// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + + +@testable import PackageGraph +import XCTest + +private func XCTAssertThrows( + _ expectedError: T, + file: StaticString = #file, + line: UInt = #line, + _ body: () throws -> Void +) where T: Equatable { + do { + try body() + XCTFail("body completed successfully", file: file, line: line) + } catch let error as T { + XCTAssertEqual(error, expectedError, file: file, line: line) + } catch { + XCTFail("unexpected error thrown: \(error)", file: file, line: line) + } +} + +extension Int: Identifiable { + public var id: Self { self } +} + +private func topologicalSort(_ nodes: [Int], _ successors: [Int: [Int]]) throws -> [Int] { + return try topologicalSort(nodes, successors: { successors[$0] ?? [] }) +} +private func topologicalSort(_ node: Int, _ successors: [Int: [Int]]) throws -> [Int] { + return try topologicalSort([node], successors) +} + +final class TopologicalSortTests: XCTestCase { + func testTopologicalSort() throws { + // A trivial graph. + XCTAssertEqual([1, 2], try topologicalSort(1, [1: [2]])) + XCTAssertEqual([1, 2], try topologicalSort([2, 1], [1: [2]])) + + // A diamond. + let diamond: [Int: [Int]] = [ + 1: [3, 2], + 2: [4], + 3: [4] + ] + XCTAssertEqual([1, 2, 3, 4], try topologicalSort(1, diamond)) + XCTAssertEqual([2, 3, 4], try topologicalSort([3, 2], diamond)) + XCTAssertEqual([1, 2, 3, 4], try topologicalSort([4, 3, 2, 1], diamond)) + + // Test cycle detection. + XCTAssertThrows(GraphError.unexpectedCycle) { _ = try topologicalSort(1, [1: [1]]) } + XCTAssertThrows(GraphError.unexpectedCycle) { _ = try topologicalSort(1, [1: [2], 2: [1]]) } + } +} From c3ab4f5266d230c4e6e3e30708212cfd29580822 Mon Sep 17 00:00:00 2001 From: Max Desiatov Date: Thu, 14 Dec 2023 17:41:57 +0000 Subject: [PATCH 24/24] Fix formatting --- Tests/PackageGraphTests/TargetTests.swift | 148 ++++++++++++---------- 1 file changed, 79 insertions(+), 69 deletions(-) diff --git a/Tests/PackageGraphTests/TargetTests.swift b/Tests/PackageGraphTests/TargetTests.swift index 6a24dacbe27..f8b33488149 100644 --- a/Tests/PackageGraphTests/TargetTests.swift +++ b/Tests/PackageGraphTests/TargetTests.swift @@ -15,8 +15,13 @@ import XCTest import PackageGraph @testable import PackageModel -private extension ResolvedTarget { - init(packageIdentity: String, name: String, deps: ResolvedTarget..., conditions: [PackageCondition] = []) { +extension ResolvedTarget { + fileprivate init( + packageIdentity: String, + name: String, + deps: ResolvedTarget..., + conditions: [PackageCondition] = [] + ) { self.init( packageIdentity: .init(packageIdentity), underlying: SwiftTarget( @@ -39,89 +44,94 @@ private extension ResolvedTarget { class TargetDependencyTests: XCTestCase { func test1() throws { - let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") - let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) - let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) - XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) + XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func test2() throws { - let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") - let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) - let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1) - let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t2, t3, t1) - - XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) - XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t2, t3, t1) + + XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) + XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func test3() throws { - let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") - let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) - let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1) - let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t1, t2, t3) - - XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) - XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2, t1) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t1, t2, t3) + + XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) + XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func test4() throws { - let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") - let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) - let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) - let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) - - XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) - XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) + + XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) + XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func test5() throws { - let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") - let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) - let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) - let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) - let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2) - let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t5, t4) - - // precise order is not important, but it is important that the following are true - let t6rd = try t6.recursiveTargetDependencies() - XCTAssertEqual(t6rd.firstIndex(of: t3)!, t6rd.index(after: t6rd.firstIndex(of: t4)!)) - XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t2)!) - XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t1)!) - XCTAssert(t6rd.firstIndex(of: t2)! < t6rd.firstIndex(of: t1)!) - XCTAssert(t6rd.firstIndex(of: t3)! < t6rd.firstIndex(of: t2)!) - - XCTAssertEqual(try t5.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) - XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) + let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2) + let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t5, t4) + + // precise order is not important, but it is important that the following are true + let t6rd = try t6.recursiveTargetDependencies() + XCTAssertEqual(t6rd.firstIndex(of: t3)!, t6rd.index(after: t6rd.firstIndex(of: t4)!)) + XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t2)!) + XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t1)!) + XCTAssert(t6rd.firstIndex(of: t2)! < t6rd.firstIndex(of: t1)!) + XCTAssert(t6rd.firstIndex(of: t3)! < t6rd.firstIndex(of: t2)!) + + XCTAssertEqual(try t5.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) + XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func test6() throws { - let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") - let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) - let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) - let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) - let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2) - let t6 = ResolvedTarget(packageIdentity: "pkg", name: "t6", deps: t4, t5) // same as above, but these two swapped - - // precise order is not important, but it is important that the following are true - let t6rd = try t6.recursiveTargetDependencies() - XCTAssertEqual(t6rd.firstIndex(of: t3)!, t6rd.index(after: t6rd.firstIndex(of: t4)!)) - XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t2)!) - XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t1)!) - XCTAssert(t6rd.firstIndex(of: t2)! < t6rd.firstIndex(of: t1)!) - XCTAssert(t6rd.firstIndex(of: t3)! < t6rd.firstIndex(of: t2)!) - - XCTAssertEqual(try t5.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) - XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) - XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) + let t1 = ResolvedTarget(packageIdentity: "pkg", name: "t1") + let t2 = ResolvedTarget(packageIdentity: "pkg", name: "t2", deps: t1) + let t3 = ResolvedTarget(packageIdentity: "pkg", name: "t3", deps: t2) + let t4 = ResolvedTarget(packageIdentity: "pkg", name: "t4", deps: t3) + let t5 = ResolvedTarget(packageIdentity: "pkg", name: "t5", deps: t2) + let t6 = ResolvedTarget( + packageIdentity: "pkg", + name: "t6", + deps: t4, + t5 + ) // same as above, but these two swapped + + // precise order is not important, but it is important that the following are true + let t6rd = try t6.recursiveTargetDependencies() + XCTAssertEqual(t6rd.firstIndex(of: t3)!, t6rd.index(after: t6rd.firstIndex(of: t4)!)) + XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t2)!) + XCTAssert(t6rd.firstIndex(of: t5)! < t6rd.firstIndex(of: t1)!) + XCTAssert(t6rd.firstIndex(of: t2)! < t6rd.firstIndex(of: t1)!) + XCTAssert(t6rd.firstIndex(of: t3)! < t6rd.firstIndex(of: t2)!) + + XCTAssertEqual(try t5.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t4.recursiveTargetDependencies(), [t3, t2, t1]) + XCTAssertEqual(try t3.recursiveTargetDependencies(), [t2, t1]) + XCTAssertEqual(try t2.recursiveTargetDependencies(), [t1]) } func testConditions() throws {