Skip to content

Commit 10be62b

Browse files
authored
[NFC] Remove SupportedPlatforms, add PlatformVersionProvider (#7161)
### Motivation: 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 marking `SupportedPlatforms` as `Hashable` and marking any of `Resolved*` types as value types also `Hashable`. In the future, when we need to make them `Sendable`, passing mutable state captured in a closure could become problematic. This also unblocks `PackageGraph` value types refactoring in #7160 and macros cross-compilation fix in #7118. ### Modifications: Add new `PlatformVersionProvider` type, which is a value type that can be easily made `Hashable` and `Sendable`. Instead of passing closures around, platform version computation code is now gathered in a single file (new `PlatformVersionProvider.swift`), with all possible context captured in its inner `enum Implementation`. This new type is adopted by `Resolved*` types in the `PackageGraph` module, which actively uses minimum platform version computations. Renamed `func getDerived` to `func getSupportedPlatform`, now that this function delegating to `PlatformVersionProvider` is declared on `Resolved*` types. ### Result: `SupportedPlatforms` type, which is nothing more than `[SupportedPlatform]` array with an additional closure, can be removed. Types that previously had properties of `SupportedPlatforms` can be easily converted to value types and made `Hashable` in a way that accounts for their content instead of class instance identity.
1 parent 64b7ca9 commit 10be62b

File tree

13 files changed

+256
-150
lines changed

13 files changed

+256
-150
lines changed

Sources/Build/BuildDescription/ProductBuildDescription.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,8 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
286286
// When deploying to macOS prior to macOS 12, add an rpath to the
287287
// back-deployed concurrency libraries.
288288
if useStdlibRpath, triple.isMacOSX {
289-
let macOSSupportedPlatform = self.package.platforms.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest)
289+
let macOSSupportedPlatform = self.package.getSupportedPlatform(for: .macOS, usingXCTest: product.isLinkingXCTest)
290+
290291
if macOSSupportedPlatform.version.major < 12 {
291292
let backDeployedStdlib = try buildParameters.toolchain.macosSwiftStdlib
292293
.parentDirectory

Sources/Build/BuildPlan/BuildPlan+Test.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ extension BuildPlan {
8888
target: discoveryTarget,
8989
dependencies: testProduct.targets.map { .target($0, conditions: []) },
9090
defaultLocalization: testProduct.defaultLocalization,
91-
platforms: testProduct.platforms
91+
supportedPlatforms: testProduct.supportedPlatforms,
92+
platformVersionProvider: testProduct.platformVersionProvider
9293
)
9394
let discoveryTargetBuildDescription = try SwiftTargetBuildDescription(
9495
package: package,
@@ -126,7 +127,8 @@ extension BuildPlan {
126127
target: entryPointTarget,
127128
dependencies: testProduct.targets.map { .target($0, conditions: []) } + resolvedTargetDependencies,
128129
defaultLocalization: testProduct.defaultLocalization,
129-
platforms: testProduct.platforms
130+
supportedPlatforms: testProduct.supportedPlatforms,
131+
platformVersionProvider: testProduct.platformVersionProvider
130132
)
131133
return try SwiftTargetBuildDescription(
132134
package: package,
@@ -169,7 +171,8 @@ extension BuildPlan {
169171
target: entryPointTarget,
170172
dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies,
171173
defaultLocalization: testProduct.defaultLocalization,
172-
platforms: testProduct.platforms
174+
supportedPlatforms: testProduct.supportedPlatforms,
175+
platformVersionProvider: testProduct.platformVersionProvider
173176
)
174177
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
175178
package: package,

Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ extension BuildParameters {
134134
// Compute the triple string for Darwin platform using the platform version.
135135
if self.triple.isDarwin() {
136136
let platform = buildEnvironment.platform
137-
let supportedPlatform = target.platforms.getDerived(for: platform, usingXCTest: target.type == .test)
137+
let supportedPlatform = target.getSupportedPlatform(for: platform, usingXCTest: target.type == .test)
138138
args += [self.triple.tripleString(forPlatformVersion: supportedPlatform.version.versionString)]
139139
} else {
140140
args += [self.triple.tripleString]
@@ -462,8 +462,8 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
462462
) throws {
463463
// Supported platforms are defined at the package level.
464464
// This will need to become a bit complicated once we have target-level or product-level platform support.
465-
let productPlatform = product.platforms.getDerived(for: .macOS, usingXCTest: product.isLinkingXCTest)
466-
let targetPlatform = target.platforms.getDerived(for: .macOS, usingXCTest: target.type == .test)
465+
let productPlatform = product.getSupportedPlatform(for: .macOS, usingXCTest: product.isLinkingXCTest)
466+
let targetPlatform = target.getSupportedPlatform(for: .macOS, usingXCTest: target.type == .test)
467467

468468
// Check if the version requirement is satisfied.
469469
//

Sources/PackageGraph/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ add_library(PackageGraph
3232
Resolution/DependencyResolverBinding.swift
3333
Resolution/DependencyResolverDelegate.swift
3434
Resolution/DependencyResolverError.swift
35+
Resolution/PlatformVersionProvider.swift
3536
Resolution/ResolvedPackage.swift
3637
Resolution/ResolvedProduct.swift
3738
Resolution/ResolvedTarget.swift

Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ extension PackageGraph {
145145
}
146146
}
147147

148+
let platformVersionProvider: PlatformVersionProvider
149+
if let customXCTestMinimumDeploymentTargets {
150+
platformVersionProvider = .init(implementation: .customXCTestMinimumDeploymentTargets(customXCTestMinimumDeploymentTargets))
151+
} else {
152+
platformVersionProvider = .init(implementation: .minimumDeploymentTargetDefault)
153+
}
154+
148155
// Resolve dependencies and create resolved packages.
149156
let resolvedPackages = try createResolvedPackages(
150157
nodes: allNodes,
@@ -153,13 +160,7 @@ extension PackageGraph {
153160
rootManifests: root.manifests,
154161
unsafeAllowedPackages: unsafeAllowedPackages,
155162
platformRegistry: customPlatformsRegistry ?? .default,
156-
derivedXCTestPlatformProvider: { declared in
157-
if let customXCTestMinimumDeploymentTargets {
158-
return customXCTestMinimumDeploymentTargets[declared]
159-
} else {
160-
return MinimumDeploymentTarget.default.computeXCTestMinimumDeploymentTarget(for: declared)
161-
}
162-
},
163+
platformVersionProvider: platformVersionProvider,
163164
fileSystem: fileSystem,
164165
observabilityScope: observabilityScope
165166
)
@@ -240,7 +241,7 @@ private func createResolvedPackages(
240241
rootManifests: [PackageIdentity: Manifest],
241242
unsafeAllowedPackages: Set<PackageReference>,
242243
platformRegistry: PlatformRegistry,
243-
derivedXCTestPlatformProvider: @escaping (_ declared: PackageModel.Platform) -> PlatformVersion?,
244+
platformVersionProvider: PlatformVersionProvider,
244245
fileSystem: FileSystem,
245246
observabilityScope: ObservabilityScope
246247
) throws -> [ResolvedPackage] {
@@ -257,7 +258,8 @@ private func createResolvedPackages(
257258
package,
258259
productFilter: node.productFilter,
259260
isAllowedToVendUnsafeProducts: isAllowedToVendUnsafeProducts,
260-
allowedToOverride: allowedToOverride
261+
allowedToOverride: allowedToOverride,
262+
platformVersionProvider: platformVersionProvider
261263
)
262264
}
263265

@@ -361,14 +363,19 @@ private func createResolvedPackages(
361363

362364
packageBuilder.defaultLocalization = package.manifest.defaultLocalization
363365

364-
packageBuilder.platforms = computePlatforms(
366+
packageBuilder.supportedPlatforms = computePlatforms(
365367
package: package,
366-
platformRegistry: platformRegistry,
367-
derivedXCTestPlatformProvider: derivedXCTestPlatformProvider
368+
platformRegistry: platformRegistry
368369
)
369370

370371
// Create target builders for each target in the package.
371-
let targetBuilders = package.targets.map{ ResolvedTargetBuilder(target: $0, observabilityScope: packageObservabilityScope) }
372+
let targetBuilders = package.targets.map {
373+
ResolvedTargetBuilder(
374+
target: $0,
375+
observabilityScope: packageObservabilityScope,
376+
platformVersionProvider: platformVersionProvider
377+
)
378+
}
372379
packageBuilder.targets = targetBuilders
373380

374381
// Establish dependencies between the targets. A target can only depend on another target present in the same package.
@@ -386,7 +393,7 @@ private func createResolvedPackages(
386393
}
387394
}
388395
targetBuilder.defaultLocalization = packageBuilder.defaultLocalization
389-
targetBuilder.platforms = packageBuilder.platforms
396+
targetBuilder.supportedPlatforms = packageBuilder.supportedPlatforms
390397
}
391398

392399
// Create product builders for each product in the package. A product can only contain a target present in the same package.
@@ -743,10 +750,8 @@ private class DuplicateProductsChecker {
743750

744751
private func computePlatforms(
745752
package: Package,
746-
platformRegistry: PlatformRegistry,
747-
derivedXCTestPlatformProvider: @escaping (_ declared: PackageModel.Platform) -> PlatformVersion?
748-
) -> SupportedPlatforms {
749-
753+
platformRegistry: PlatformRegistry
754+
) -> [SupportedPlatform] {
750755
// the supported platforms as declared in the manifest
751756
let declaredPlatforms: [SupportedPlatform] = package.manifest.platforms.map { platform in
752757
let declaredPlatform = platformRegistry.platformByName[platform.platformName]
@@ -758,10 +763,7 @@ private func computePlatforms(
758763
)
759764
}
760765

761-
return SupportedPlatforms(
762-
declared: declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }),
763-
derivedXCTestPlatformProvider: derivedXCTestPlatformProvider
764-
)
766+
return declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name })
765767
}
766768

767769
// Track and override module aliases specified for targets in a package graph
@@ -888,18 +890,22 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
888890
var defaultLocalization: String? = nil
889891

890892
/// The platforms supported by this package.
891-
var platforms: SupportedPlatforms = .init(declared: [], derivedXCTestPlatformProvider: .none)
893+
var supportedPlatforms: [SupportedPlatform] = []
894+
895+
let platformVersionProvider: PlatformVersionProvider
892896

893897
init(
894898
target: Target,
895-
observabilityScope: ObservabilityScope
899+
observabilityScope: ObservabilityScope,
900+
platformVersionProvider: PlatformVersionProvider
896901
) {
897902
self.target = target
898903
self.diagnosticsEmitter = observabilityScope.makeDiagnosticsEmitter() {
899904
var metadata = ObservabilityMetadata()
900905
metadata.targetName = target.name
901906
return metadata
902907
}
908+
self.platformVersionProvider = platformVersionProvider
903909
}
904910

905911
func diagnoseInvalidUseOfUnsafeFlags(_ product: ResolvedProduct) throws {
@@ -934,7 +940,8 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
934940
target: self.target,
935941
dependencies: dependencies,
936942
defaultLocalization: self.defaultLocalization,
937-
platforms: self.platforms
943+
supportedPlatforms: self.supportedPlatforms,
944+
platformVersionProvider: self.platformVersionProvider
938945
)
939946
}
940947
}
@@ -983,27 +990,37 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
983990
var defaultLocalization: String? = nil
984991

985992
/// The platforms supported by this package.
986-
var platforms: SupportedPlatforms = .init(declared: [], derivedXCTestPlatformProvider: .none)
993+
var supportedPlatforms: [SupportedPlatform] = []
987994

988995
/// If the given package's source is a registry release, this provides additional metadata and signature information.
989996
var registryMetadata: RegistryReleaseMetadata?
990997

991-
init(_ package: Package, productFilter: ProductFilter, isAllowedToVendUnsafeProducts: Bool, allowedToOverride: Bool) {
998+
let platformVersionProvider: PlatformVersionProvider
999+
1000+
init(
1001+
_ package: Package,
1002+
productFilter: ProductFilter,
1003+
isAllowedToVendUnsafeProducts: Bool,
1004+
allowedToOverride: Bool,
1005+
platformVersionProvider: PlatformVersionProvider
1006+
) {
9921007
self.package = package
9931008
self.productFilter = productFilter
9941009
self.isAllowedToVendUnsafeProducts = isAllowedToVendUnsafeProducts
9951010
self.allowedToOverride = allowedToOverride
1011+
self.platformVersionProvider = platformVersionProvider
9961012
}
9971013

9981014
override func constructImpl() throws -> ResolvedPackage {
9991015
return ResolvedPackage(
10001016
package: self.package,
10011017
defaultLocalization: self.defaultLocalization,
1002-
platforms: self.platforms,
1018+
supportedPlatforms: self.supportedPlatforms,
10031019
dependencies: try self.dependencies.map{ try $0.construct() },
10041020
targets: try self.targets.map{ try $0.construct() },
10051021
products: try self.products.map{ try $0.construct() },
1006-
registryMetadata: self.registryMetadata
1022+
registryMetadata: self.registryMetadata,
1023+
platformVersionProvider: self.platformVersionProvider
10071024
)
10081025
}
10091026
}
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See http://swift.org/LICENSE.txt for license information
9+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import struct PackageModel.MinimumDeploymentTarget
14+
import struct PackageModel.Platform
15+
import struct PackageModel.PlatformVersion
16+
import struct PackageModel.SupportedPlatform
17+
18+
/// Merging two sets of supported platforms, preferring the max constraint
19+
func merge(into partial: inout [SupportedPlatform], platforms: [SupportedPlatform]) {
20+
for platformSupport in platforms {
21+
if let existing = partial.firstIndex(where: { $0.platform == platformSupport.platform }) {
22+
if partial[existing].version < platformSupport.version {
23+
partial.remove(at: existing)
24+
partial.append(platformSupport)
25+
}
26+
} else {
27+
partial.append(platformSupport)
28+
}
29+
}
30+
}
31+
32+
public struct PlatformVersionProvider: Hashable {
33+
public enum Implementation: Hashable {
34+
case mergingFromTargets([ResolvedTarget])
35+
case customXCTestMinimumDeploymentTargets([PackageModel.Platform: PlatformVersion])
36+
case minimumDeploymentTargetDefault
37+
}
38+
39+
private let implementation: Implementation
40+
41+
public init(implementation: Implementation) {
42+
self.implementation = implementation
43+
}
44+
45+
func derivedXCTestPlatformProvider(_ declared: PackageModel.Platform) -> PlatformVersion? {
46+
switch self.implementation {
47+
case .mergingFromTargets(let targets):
48+
let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in
49+
merge(
50+
into: &partial,
51+
platforms: [item.getSupportedPlatform(for: declared, usingXCTest: item.type == .test)]
52+
)
53+
}
54+
return platforms.first!.version
55+
56+
case .customXCTestMinimumDeploymentTargets(let customXCTestMinimumDeploymentTargets):
57+
return customXCTestMinimumDeploymentTargets[declared]
58+
59+
case .minimumDeploymentTargetDefault:
60+
return MinimumDeploymentTarget.default.computeXCTestMinimumDeploymentTarget(for: declared)
61+
}
62+
}
63+
64+
/// Returns the supported platform instance for the given platform.
65+
func getDerived(declared: [SupportedPlatform], for platform: Platform, usingXCTest: Bool) -> SupportedPlatform {
66+
// derived platform based on known minimum deployment target logic
67+
if let declaredPlatform = declared.first(where: { $0.platform == platform }) {
68+
var version = declaredPlatform.version
69+
70+
if usingXCTest,
71+
let xcTestMinimumDeploymentTarget = self.derivedXCTestPlatformProvider(platform),
72+
version < xcTestMinimumDeploymentTarget
73+
{
74+
version = xcTestMinimumDeploymentTarget
75+
}
76+
77+
// If the declared version is smaller than the oldest supported one, we raise the derived version to that.
78+
if version < platform.oldestSupportedVersion {
79+
version = platform.oldestSupportedVersion
80+
}
81+
82+
return SupportedPlatform(
83+
platform: declaredPlatform.platform,
84+
version: version,
85+
options: declaredPlatform.options
86+
)
87+
} else {
88+
let minimumSupportedVersion: PlatformVersion
89+
if usingXCTest,
90+
let xcTestMinimumDeploymentTarget = self.derivedXCTestPlatformProvider(platform),
91+
xcTestMinimumDeploymentTarget > platform.oldestSupportedVersion
92+
{
93+
minimumSupportedVersion = xcTestMinimumDeploymentTarget
94+
} else {
95+
minimumSupportedVersion = platform.oldestSupportedVersion
96+
}
97+
98+
let oldestSupportedVersion: PlatformVersion
99+
if platform == .macCatalyst {
100+
let iOS = self.getDerived(declared: declared, for: .iOS, usingXCTest: usingXCTest)
101+
// If there was no deployment target specified for Mac Catalyst, fall back to the iOS deployment target.
102+
oldestSupportedVersion = max(minimumSupportedVersion, iOS.version)
103+
} else {
104+
oldestSupportedVersion = minimumSupportedVersion
105+
}
106+
107+
return SupportedPlatform(
108+
platform: platform,
109+
version: oldestSupportedVersion,
110+
options: []
111+
)
112+
}
113+
}
114+
}

0 commit comments

Comments
 (0)