Skip to content

Commit c1b84da

Browse files
neonichuabertelrud
neonichu
and
abertelrud
committed
Improve version-specific manifest selection (#6801)
Currently, we only pick a non-exact match for version-specific manifests if its tools-version is *higher* than the regular one. That seems like an edge case only and we should *also* pick it if the regular manifest is incompatible. This both matches intuition as well as our current documentation on the topic. It should also be backwards-compatible with the current behavior since the new behavior only comes into play if we would have previously failed to load a manifest at all. Co-authored-by: Anders Bertelrud <[email protected]>
1 parent da66bb1 commit c1b84da

File tree

12 files changed

+120
-146
lines changed

12 files changed

+120
-146
lines changed

‎Sources/Build/BuildDescription/ProductBuildDescription.swift‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,16 +267,17 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
267267

268268
// When deploying to macOS prior to macOS 12, add an rpath to the
269269
// back-deployed concurrency libraries.
270-
if useStdlibRpath, self.buildParameters.targetTriple.isMacOSX,
271-
let macOSSupportedPlatform = self.package.platforms.getDerived(for: .macOS),
272-
macOSSupportedPlatform.version.major < 12
270+
if useStdlibRpath, self.buildParameters.targetTriple.isMacOSX
273271
{
274-
let backDeployedStdlib = try buildParameters.toolchain.macosSwiftStdlib
275-
.parentDirectory
276-
.parentDirectory
277-
.appending("swift-5.5")
278-
.appending("macosx")
279-
args += ["-Xlinker", "-rpath", "-Xlinker", backDeployedStdlib.pathString]
272+
let macOSSupportedPlatform = self.package.platforms.getDerived(for: .macOS, usingXCTest: product.type == .test)
273+
if macOSSupportedPlatform.version.major < 12 {
274+
let backDeployedStdlib = try buildParameters.toolchain.macosSwiftStdlib
275+
.parentDirectory
276+
.parentDirectory
277+
.appending("swift-5.5")
278+
.appending("macosx")
279+
args += ["-Xlinker", "-rpath", "-Xlinker", backDeployedStdlib.pathString]
280+
}
280281
}
281282
}
282283

‎Sources/Build/BuildPlan.swift‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,7 @@ extension BuildParameters {
129129
// Compute the triple string for Darwin platform using the platform version.
130130
if targetTriple.isDarwin() {
131131
let platform = buildEnvironment.platform
132-
guard let supportedPlatform = target.platforms.getDerived(for: platform) else {
133-
throw StringError("the target \(target) doesn't support building for the \(platform.name) platform")
134-
}
132+
let supportedPlatform = target.platforms.getDerived(for: platform, usingXCTest: target.type == .test)
135133
args += [targetTriple.tripleString(forPlatformVersion: supportedPlatform.version.versionString)]
136134
} else {
137135
args += [targetTriple.tripleString]
@@ -294,7 +292,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
294292
target: discoveryTarget,
295293
dependencies: testProduct.targets.map { .target($0, conditions: []) },
296294
defaultLocalization: .none, // safe since this is a derived target
297-
platforms: .init(declared: [], derived: []) // safe since this is a derived target
295+
platforms: .init(declared: [], deriveXCTestPlatform: { _ in nil }) // safe since this is a derived target
298296
)
299297
let discoveryTargetBuildDescription = try SwiftTargetBuildDescription(
300298
package: package,
@@ -327,7 +325,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
327325
target: entryPointTarget,
328326
dependencies: testProduct.targets.map { .target($0, conditions: []) } + [.target(discoveryResolvedTarget, conditions: [])],
329327
defaultLocalization: .none, // safe since this is a derived target
330-
platforms: .init(declared: [], derived: []) // safe since this is a derived target
328+
platforms: .init(declared: [], deriveXCTestPlatform: { _ in nil }) // safe since this is a derived target
331329
)
332330
return try SwiftTargetBuildDescription(
333331
package: package,
@@ -356,7 +354,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
356354
target: entryPointTarget,
357355
dependencies: entryPointResolvedTarget.dependencies + [.target(discoveryTargets.resolved, conditions: [])],
358356
defaultLocalization: .none, // safe since this is a derived target
359-
platforms: .init(declared: [], derived: []) // safe since this is a derived target
357+
platforms: .init(declared: [], deriveXCTestPlatform: { _ in nil }) // safe since this is a derived target
360358
)
361359
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
362360
package: package,
@@ -563,12 +561,8 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
563561
) throws {
564562
// Supported platforms are defined at the package level.
565563
// This will need to become a bit complicated once we have target-level or product-level platform support.
566-
guard let productPlatform = product.platforms.getDerived(for: .macOS) else {
567-
throw StringError("Expected supported platform macOS in product \(product)")
568-
}
569-
guard let targetPlatform = target.platforms.getDerived(for: .macOS) else {
570-
throw StringError("Expected supported platform macOS in target \(target)")
571-
}
564+
let productPlatform = product.platforms.getDerived(for: .macOS, usingXCTest: product.type == .test)
565+
let targetPlatform = target.platforms.getDerived(for: .macOS, usingXCTest: product.type == .test)
572566

573567
// Check if the version requirement is satisfied.
574568
//

‎Sources/Commands/PackageTools/Update.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ extension SwiftPackageTool {
3434

3535
func run(_ swiftTool: SwiftTool) throws {
3636
let workspace = try swiftTool.getActiveWorkspace()
37-
37+
3838
let changes = try workspace.updateDependencies(
3939
root: swiftTool.getWorkspaceRoot(),
4040
packages: packages,

‎Sources/PackageGraph/PackageGraph+Loading.swift‎

Lines changed: 14 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,13 @@ extension PackageGraph {
145145
rootManifests: root.manifests,
146146
unsafeAllowedPackages: unsafeAllowedPackages,
147147
platformRegistry: customPlatformsRegistry ?? .default,
148-
xcTestMinimumDeploymentTargets: customXCTestMinimumDeploymentTargets ?? MinimumDeploymentTarget.default.xcTestMinimumDeploymentTargets,
148+
deriveXCTestPlatform: { declared in
149+
if let customXCTestMinimumDeploymentTargets = customXCTestMinimumDeploymentTargets {
150+
return customXCTestMinimumDeploymentTargets[declared]
151+
} else {
152+
return MinimumDeploymentTarget.default.computeXCTestMinimumDeploymentTarget(for: declared)
153+
}
154+
},
149155
fileSystem: fileSystem,
150156
observabilityScope: observabilityScope
151157
)
@@ -226,7 +232,7 @@ private func createResolvedPackages(
226232
rootManifests: [PackageIdentity: Manifest],
227233
unsafeAllowedPackages: Set<PackageReference>,
228234
platformRegistry: PlatformRegistry,
229-
xcTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion],
235+
deriveXCTestPlatform: @escaping (_ declared: PackageModel.Platform) -> PlatformVersion?,
230236
fileSystem: FileSystem,
231237
observabilityScope: ObservabilityScope
232238
) throws -> [ResolvedPackage] {
@@ -349,16 +355,8 @@ private func createResolvedPackages(
349355

350356
packageBuilder.platforms = computePlatforms(
351357
package: package,
352-
usingXCTest: false,
353-
platformRegistry: platformRegistry,
354-
xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets
355-
)
356-
357-
let testPlatforms = computePlatforms(
358-
package: package,
359-
usingXCTest: true,
360358
platformRegistry: platformRegistry,
361-
xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets
359+
deriveXCTestPlatform: deriveXCTestPlatform
362360
)
363361

364362
// Create target builders for each target in the package.
@@ -380,7 +378,7 @@ private func createResolvedPackages(
380378
}
381379
}
382380
targetBuilder.defaultLocalization = packageBuilder.defaultLocalization
383-
targetBuilder.platforms = targetBuilder.target.type == .test ? testPlatforms : packageBuilder.platforms
381+
targetBuilder.platforms = packageBuilder.platforms
384382
}
385383

386384
// Create product builders for each product in the package. A product can only contain a target present in the same package.
@@ -734,9 +732,8 @@ private class DuplicateProductsChecker {
734732

735733
private func computePlatforms(
736734
package: Package,
737-
usingXCTest: Bool,
738735
platformRegistry: PlatformRegistry,
739-
xcTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]
736+
deriveXCTestPlatform: @escaping (_ declared: PackageModel.Platform) -> PlatformVersion?
740737
) -> SupportedPlatforms {
741738

742739
// the supported platforms as declared in the manifest
@@ -750,67 +747,9 @@ private func computePlatforms(
750747
)
751748
}
752749

753-
// the derived platforms based on known minimum deployment target logic
754-
var derivedPlatforms = [SupportedPlatform]()
755-
756-
/// Add each declared platform to the supported platforms list.
757-
for platform in package.manifest.platforms {
758-
let declaredPlatform = platformRegistry.platformByName[platform.platformName]
759-
?? PackageModel.Platform.custom(name: platform.platformName, oldestSupportedVersion: platform.version)
760-
var version = PlatformVersion(platform.version)
761-
762-
if usingXCTest, let xcTestMinimumDeploymentTarget = xcTestMinimumDeploymentTargets[declaredPlatform], version < xcTestMinimumDeploymentTarget {
763-
version = xcTestMinimumDeploymentTarget
764-
}
765-
766-
// If the declared version is smaller than the oldest supported one, we raise the derived version to that.
767-
if version < declaredPlatform.oldestSupportedVersion {
768-
version = declaredPlatform.oldestSupportedVersion
769-
}
770-
771-
let supportedPlatform = SupportedPlatform(
772-
platform: declaredPlatform,
773-
version: version,
774-
options: platform.options
775-
)
776-
777-
derivedPlatforms.append(supportedPlatform)
778-
}
779-
780-
// Find the undeclared platforms.
781-
let remainingPlatforms = Set(platformRegistry.platformByName.keys).subtracting(derivedPlatforms.map({ $0.platform.name }))
782-
783-
/// Start synthesizing for each undeclared platform.
784-
for platformName in remainingPlatforms.sorted() {
785-
let platform = platformRegistry.platformByName[platformName]!
786-
787-
let minimumSupportedVersion: PlatformVersion
788-
if usingXCTest, let xcTestMinimumDeploymentTarget = xcTestMinimumDeploymentTargets[platform], xcTestMinimumDeploymentTarget > platform.oldestSupportedVersion {
789-
minimumSupportedVersion = xcTestMinimumDeploymentTarget
790-
} else {
791-
minimumSupportedVersion = platform.oldestSupportedVersion
792-
}
793-
794-
let oldestSupportedVersion: PlatformVersion
795-
if platform == .macCatalyst, let iOS = derivedPlatforms.first(where: { $0.platform == .iOS }) {
796-
// If there was no deployment target specified for Mac Catalyst, fall back to the iOS deployment target.
797-
oldestSupportedVersion = max(minimumSupportedVersion, iOS.version)
798-
} else {
799-
oldestSupportedVersion = minimumSupportedVersion
800-
}
801-
802-
let supportedPlatform = SupportedPlatform(
803-
platform: platform,
804-
version: oldestSupportedVersion,
805-
options: []
806-
)
807-
808-
derivedPlatforms.append(supportedPlatform)
809-
}
810-
811750
return SupportedPlatforms(
812751
declared: declaredPlatforms.sorted(by: { $0.platform.name < $1.platform.name }),
813-
derived: derivedPlatforms.sorted(by: { $0.platform.name < $1.platform.name })
752+
deriveXCTestPlatform: deriveXCTestPlatform
814753
)
815754
}
816755

@@ -934,7 +873,7 @@ private final class ResolvedTargetBuilder: ResolvedBuilder<ResolvedTarget> {
934873
var defaultLocalization: String? = nil
935874

936875
/// The platforms supported by this package.
937-
var platforms: SupportedPlatforms = .init(declared: [], derived: [])
876+
var platforms: SupportedPlatforms = .init(declared: [], deriveXCTestPlatform: { _ in nil })
938877

939878
init(
940879
target: Target,
@@ -1026,7 +965,7 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
1026965
var defaultLocalization: String? = nil
1027966

1028967
/// The platforms supported by this package.
1029-
var platforms: SupportedPlatforms = .init(declared: [], derived: [])
968+
var platforms: SupportedPlatforms = .init(declared: [], deriveXCTestPlatform: { _ in nil })
1030969

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

‎Sources/PackageGraph/ResolvedProduct.swift‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,15 @@ public final class ResolvedProduct {
7070
target: swiftTarget,
7171
dependencies: targets.map { .target($0, conditions: []) },
7272
defaultLocalization: .none, // safe since this is a derived product
73-
platforms: .init(declared: [], derived: []) // safe since this is a derived product
73+
platforms: .init(declared: [], deriveXCTestPlatform: { _ in nil }) // safe since this is a derived product
7474
)
7575
}
7676

7777
// defaultLocalization is currently shared across the entire package
7878
// this may need to be enhanced if / when we support localization per target or product
7979
self.defaultLocalization = self.targets.first?.defaultLocalization
8080

81-
self.platforms = Self.computePlatforms(targets: targets)
81+
self.platforms = Self.computePlatforms(targets: targets, usingXCTest: product.type == .test)
8282
}
8383

8484
/// True if this product contains Swift targets.
@@ -99,7 +99,7 @@ public final class ResolvedProduct {
9999
return Array(Set(targets).union(recursiveDependencies))
100100
}
101101

102-
private static func computePlatforms(targets: [ResolvedTarget]) -> SupportedPlatforms {
102+
private static func computePlatforms(targets: [ResolvedTarget], usingXCTest: Bool) -> SupportedPlatforms {
103103
// merging two sets of supported platforms, preferring the max constraint
104104
func merge(into partial: inout [SupportedPlatform], platforms: [SupportedPlatform]) {
105105
for platformSupport in platforms {
@@ -118,16 +118,13 @@ public final class ResolvedProduct {
118118
merge(into: &partial, platforms: item.platforms.declared)
119119
}
120120

121-
let derived = targets.reduce(into: [SupportedPlatform]()) { partial, item in
122-
merge(into: &partial, platforms: item.platforms.derived)
123-
}
124-
125121
return SupportedPlatforms(
126-
declared: declared.sorted(by: { $0.platform.name < $1.platform.name }),
127-
derived: derived.sorted(by: { $0.platform.name < $1.platform.name })
128-
)
129-
130-
122+
declared: declared.sorted(by: { $0.platform.name < $1.platform.name })) { declared in
123+
let platforms = targets.reduce(into: [SupportedPlatform]()) { partial, item in
124+
merge(into: &partial, platforms: [item.platforms.getDerived(for: declared, usingXCTest: item.type == .test)])
125+
}
126+
return platforms.first!.version
127+
}
131128
}
132129
}
133130

‎Sources/PackageModel/MinimumDeploymentTarget.swift‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,20 @@ import struct TSCBasic.ProcessResult
1717

1818

1919
public struct MinimumDeploymentTarget {
20-
public let xcTestMinimumDeploymentTargets: [PackageModel.Platform:PlatformVersion]
20+
public let xcTestMinimumDeploymentTargets = ThreadSafeKeyValueStore<PackageModel.Platform,PlatformVersion>()
2121

2222
public static let `default`: MinimumDeploymentTarget = .init()
2323

24-
public init() {
25-
xcTestMinimumDeploymentTargets = PlatformRegistry.default.knownPlatforms.reduce([PackageModel.Platform:PlatformVersion]()) {
26-
var dict = $0
27-
dict[$1] = Self.computeXCTestMinimumDeploymentTarget(for: $1)
28-
return dict
24+
private init() {
25+
}
26+
27+
public func computeXCTestMinimumDeploymentTarget(for platform: PackageModel.Platform) -> PlatformVersion {
28+
if let result = xcTestMinimumDeploymentTargets[platform] {
29+
return result
30+
} else {
31+
let result = Self.computeXCTestMinimumDeploymentTarget(for: platform)
32+
xcTestMinimumDeploymentTargets[platform] = result
33+
return result
2934
}
3035
}
3136

‎Sources/PackageModel/Platform.swift‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,56 @@ public struct Platform: Equatable, Hashable, Codable {
5454

5555
public struct SupportedPlatforms {
5656
public let declared: [SupportedPlatform]
57-
public let derived: [SupportedPlatform]
57+
private let deriveXCTestPlatform: (Platform) -> PlatformVersion?
5858

59-
public init(declared: [SupportedPlatform], derived: [SupportedPlatform]) {
59+
public init(declared: [SupportedPlatform], deriveXCTestPlatform: @escaping (_ declared: Platform) -> PlatformVersion?) {
6060
self.declared = declared
61-
self.derived = derived
61+
self.deriveXCTestPlatform = deriveXCTestPlatform
6262
}
6363

6464
/// Returns the supported platform instance for the given platform.
65-
public func getDerived(for platform: Platform) -> SupportedPlatform? {
66-
return self.derived.first(where: { $0.platform == platform })
65+
public func getDerived(for platform: Platform, usingXCTest: Bool) -> SupportedPlatform {
66+
// derived platform based on known minimum deployment target logic
67+
if let declaredPlatform = self.declared.first(where: { $0.platform == platform }) {
68+
var version = declaredPlatform.version
69+
70+
if usingXCTest, let xcTestMinimumDeploymentTarget = deriveXCTestPlatform(platform), version < xcTestMinimumDeploymentTarget {
71+
version = xcTestMinimumDeploymentTarget
72+
}
73+
74+
// If the declared version is smaller than the oldest supported one, we raise the derived version to that.
75+
if version < platform.oldestSupportedVersion {
76+
version = platform.oldestSupportedVersion
77+
}
78+
79+
return SupportedPlatform(
80+
platform: declaredPlatform.platform,
81+
version: version,
82+
options: declaredPlatform.options
83+
)
84+
} else {
85+
let minimumSupportedVersion: PlatformVersion
86+
if usingXCTest, let xcTestMinimumDeploymentTarget = deriveXCTestPlatform(platform), xcTestMinimumDeploymentTarget > platform.oldestSupportedVersion {
87+
minimumSupportedVersion = xcTestMinimumDeploymentTarget
88+
} else {
89+
minimumSupportedVersion = platform.oldestSupportedVersion
90+
}
91+
92+
let oldestSupportedVersion: PlatformVersion
93+
if platform == .macCatalyst {
94+
let iOS = getDerived(for: .iOS, usingXCTest: usingXCTest)
95+
// If there was no deployment target specified for Mac Catalyst, fall back to the iOS deployment target.
96+
oldestSupportedVersion = max(minimumSupportedVersion, iOS.version)
97+
} else {
98+
oldestSupportedVersion = minimumSupportedVersion
99+
}
100+
101+
return SupportedPlatform(
102+
platform: platform,
103+
version: oldestSupportedVersion,
104+
options: []
105+
)
106+
}
67107
}
68108
}
69109

0 commit comments

Comments
 (0)