Skip to content

Commit 08c7ce3

Browse files
authored
Drop unknown platforms when generating PIF (#9021)
Some existing packages use `.when(platforms: [.custom("DoesNotExist")])` to unconditionally deactivate settings. Stop asserting on these unknown platforms
1 parent c68610c commit 08c7ce3

File tree

9 files changed

+215
-25
lines changed

9 files changed

+215
-25
lines changed
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// swift-tools-version: 6.2
2+
3+
import PackageDescription
4+
5+
let package = Package(
6+
name: "UnknownPlatforms",
7+
targets: [
8+
.executableTarget(
9+
name: "UnknownPlatforms",
10+
swiftSettings: [
11+
.define("FOO", .when(platforms: [.custom("DoesNotExist")])),
12+
.define("BAR", .when(platforms: [.linux])),
13+
.define("BAZ", .when(platforms: [.macOS])),
14+
],
15+
),
16+
]
17+
)
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// The Swift Programming Language
2+
// https://docs.swift.org/swift-book
3+
4+
@main
5+
struct UnknownPlatforms {
6+
static func main() {
7+
print("Hello, world!")
8+
}
9+
}

Package.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,10 @@ let package = Package(
985985
"_InternalTestSupport",
986986
]
987987
),
988+
.testTarget(
989+
name: "SwiftBuildSupportTests",
990+
dependencies: ["SwiftBuildSupport", "_InternalTestSupport", "_InternalBuildTestSupport"]
991+
),
988992
// Examples (These are built to ensure they stay up to date with the API.)
989993
.executableTarget(
990994
name: "package-info",

Sources/SwiftBuildSupport/PIFBuilder.swift

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,7 @@ extension ModulesGraph {
8383
}
8484

8585
/// The parameters required by `PIFBuilder`.
86-
struct PIFBuilderParameters {
87-
let triple: Basics.Triple
88-
86+
package struct PIFBuilderParameters {
8987
/// Whether the toolchain supports `-package-name` option.
9088
let isPackageAccessModifierSupported: Bool
9189

@@ -101,9 +99,6 @@ struct PIFBuilderParameters {
10199
/// An array of paths to search for pkg-config `.pc` files.
102100
let pkgConfigDirectories: [AbsolutePath]
103101

104-
/// The toolchain's SDK root path.
105-
let sdkRootPath: AbsolutePath?
106-
107102
/// The Swift language versions supported by the SwiftBuild being used for the build.
108103
let supportedSwiftVersions: [SwiftLanguageVersion]
109104

@@ -118,6 +113,19 @@ struct PIFBuilderParameters {
118113

119114
/// Additional rules for including a source or resource file in a target
120115
let additionalFileRules: [FileRuleDescription]
116+
117+
package init(isPackageAccessModifierSupported: Bool, enableTestability: Bool, shouldCreateDylibForDynamicProducts: Bool, toolchainLibDir: AbsolutePath, pkgConfigDirectories: [AbsolutePath], supportedSwiftVersions: [SwiftLanguageVersion], pluginScriptRunner: PluginScriptRunner, disableSandbox: Bool, pluginWorkingDirectory: AbsolutePath, additionalFileRules: [FileRuleDescription]) {
118+
self.isPackageAccessModifierSupported = isPackageAccessModifierSupported
119+
self.enableTestability = enableTestability
120+
self.shouldCreateDylibForDynamicProducts = shouldCreateDylibForDynamicProducts
121+
self.toolchainLibDir = toolchainLibDir
122+
self.pkgConfigDirectories = pkgConfigDirectories
123+
self.supportedSwiftVersions = supportedSwiftVersions
124+
self.pluginScriptRunner = pluginScriptRunner
125+
self.disableSandbox = disableSandbox
126+
self.pluginWorkingDirectory = pluginWorkingDirectory
127+
self.additionalFileRules = additionalFileRules
128+
}
121129
}
122130

123131
/// PIF object builder for a package graph.
@@ -146,7 +154,7 @@ public final class PIFBuilder {
146154
/// - parameters: The parameters used to configure the PIF.
147155
/// - fileSystem: The file system to read from.
148156
/// - observabilityScope: The ObservabilityScope to emit diagnostics to.
149-
init(
157+
package init(
150158
graph: ModulesGraph,
151159
parameters: PIFBuilderParameters,
152160
fileSystem: FileSystem,
@@ -163,7 +171,7 @@ public final class PIFBuilder {
163171
/// - prettyPrint: Whether to return a formatted JSON.
164172
/// - preservePIFModelStructure: Whether to preserve model structure.
165173
/// - Returns: The package graph in the JSON PIF format.
166-
func generatePIF(
174+
package func generatePIF(
167175
prettyPrint: Bool = true,
168176
preservePIFModelStructure: Bool = false,
169177
printPIFManifestGraphviz: Bool = false,
@@ -227,7 +235,7 @@ public final class PIFBuilder {
227235
}
228236

229237
/// Constructs a `PIF.TopLevelObject` representing the package graph.
230-
private func constructPIF(buildParameters: BuildParameters) async throws -> PIF.TopLevelObject {
238+
package func constructPIF(buildParameters: BuildParameters) async throws -> PIF.TopLevelObject {
231239
let pluginScriptRunner = self.parameters.pluginScriptRunner
232240
let outputDir = self.parameters.pluginWorkingDirectory.appending("outputs")
233241

@@ -727,13 +735,11 @@ extension PIFBuilderParameters {
727735
additionalFileRules: [FileRuleDescription]
728736
) {
729737
self.init(
730-
triple: buildParameters.triple,
731738
isPackageAccessModifierSupported: buildParameters.driverParameters.isPackageAccessModifierSupported,
732739
enableTestability: buildParameters.enableTestability,
733740
shouldCreateDylibForDynamicProducts: buildParameters.shouldCreateDylibForDynamicProducts,
734741
toolchainLibDir: (try? buildParameters.toolchain.toolchainLibDir) ?? .root,
735742
pkgConfigDirectories: buildParameters.pkgConfigDirectories,
736-
sdkRootPath: buildParameters.toolchain.sdkRootPath,
737743
supportedSwiftVersions: supportedSwiftVersions,
738744
pluginScriptRunner: pluginScriptRunner,
739745
disableSandbox: disableSandbox,

Sources/SwiftBuildSupport/PackagePIFBuilder+Helpers.swift

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ extension Sequence<PackageModel.PackageCondition> {
234234
}
235235

236236
var pifPlatformsForCondition: [ProjectModel.BuildSettings.Platform] = platforms
237-
.map { ProjectModel.BuildSettings.Platform(from: $0) }
237+
.compactMap { try? ProjectModel.BuildSettings.Platform(from: $0) }
238238

239239
// Treat catalyst like macOS for backwards compatibility with older tools versions.
240240
if pifPlatformsForCondition.contains(.macOS), toolsVersion < ToolsVersion.v5_5 {
@@ -537,7 +537,7 @@ extension PackageGraph.ResolvedModule {
537537
/// Collect the build settings defined in the package manifest.
538538
/// Some of them apply *only* to the target itself, while others are also imparted to clients.
539539
/// Note that the platform is *optional*; unconditional settings have no platform condition.
540-
var allBuildSettings: AllBuildSettings {
540+
func computeAllBuildSettings(observabilityScope: ObservabilityScope) -> AllBuildSettings {
541541
var allSettings = AllBuildSettings()
542542

543543
for (declaration, settingsAssigments) in self.underlying.buildSettings.assignments {
@@ -565,7 +565,16 @@ extension PackageGraph.ResolvedModule {
565565
let (platforms, configurations, _) = settingAssignment.conditions.splitIntoConcreteConditions
566566

567567
for platform in platforms {
568-
let pifPlatform = platform.map { ProjectModel.BuildSettings.Platform(from: $0) }
568+
let pifPlatform: ProjectModel.BuildSettings.Platform?
569+
if let platform {
570+
guard let computedPifPlatform = try? ProjectModel.BuildSettings.Platform(from: platform) else {
571+
observabilityScope.logPIF(.warning, "Ignoring settings assignments for unknown platform '\(platform.name)'")
572+
continue
573+
}
574+
pifPlatform = computedPifPlatform
575+
} else {
576+
pifPlatform = nil
577+
}
569578

570579
if pifDeclaration == .OTHER_LDFLAGS {
571580
var settingsByDeclaration: [ProjectModel.BuildSettings.Declaration: [String]]
@@ -962,7 +971,11 @@ extension ProjectModel.BuildSettings.MultipleValueSetting {
962971
}
963972

964973
extension ProjectModel.BuildSettings.Platform {
965-
init(from platform: PackageModel.Platform) {
974+
enum Error: Swift.Error {
975+
case unknownPlatform(String)
976+
}
977+
978+
init(from platform: PackageModel.Platform) throws {
966979
self = switch platform {
967980
case .macOS: .macOS
968981
case .macCatalyst: .macCatalyst
@@ -977,7 +990,7 @@ extension ProjectModel.BuildSettings.Platform {
977990
case .wasi: .wasi
978991
case .openbsd: .openbsd
979992
case .freebsd: .freebsd
980-
default: preconditionFailure("Unexpected platform: \(platform.name)")
993+
default: throw Error.unknownPlatform(platform.name)
981994
}
982995
}
983996
}

Sources/SwiftBuildSupport/PackagePIFBuilder.swift

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,10 @@ public final class PackagePIFBuilder {
562562
self.delegate.configureProjectBuildSettings(&settings)
563563

564564
for (platform, platformOptions) in self.package.sdkOptions(delegate: self.delegate) {
565-
let pifPlatform = ProjectModel.BuildSettings.Platform(from: platform)
565+
guard let pifPlatform = try? ProjectModel.BuildSettings.Platform(from: platform) else {
566+
log(.warning, "Ignoring options '\(platformOptions.joined(separator: " "))' specified for unknown platform \(platform.name)")
567+
continue
568+
}
566569
settings.platformSpecificSettings[pifPlatform]![.SPECIALIZATION_SDK_OPTIONS]!
567570
.append(contentsOf: platformOptions)
568571
}
@@ -584,11 +587,11 @@ public final class PackagePIFBuilder {
584587
let arm64ePlatforms: [PackageModel.Platform] = [.iOS, .macOS, .visionOS]
585588
for arm64ePlatform in arm64ePlatforms {
586589
if self.delegate.shouldPackagesBuildForARM64e(platform: arm64ePlatform) {
587-
let pifPlatform: ProjectModel.BuildSettings.Platform = switch arm64ePlatform {
588-
case .iOS:
589-
._iOSDevice
590-
default:
591-
.init(from: arm64ePlatform)
590+
let pifPlatform: ProjectModel.BuildSettings.Platform
591+
do {
592+
pifPlatform = try .init(from: arm64ePlatform)
593+
} catch {
594+
preconditionFailure("Unhandled arm64e platform: \(error)")
592595
}
593596
settings.platformSpecificSettings[pifPlatform]![.ARCHS, default: []].append(contentsOf: ["arm64e"])
594597
}

Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Modules.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ extension PackagePIFProjectBuilder {
738738
var debugSettings = settings
739739
var releaseSettings = settings
740740

741-
let allBuildSettings = sourceModule.allBuildSettings
741+
let allBuildSettings = sourceModule.computeAllBuildSettings(observabilityScope: pifBuilder.observabilityScope)
742742

743743
// Apply target-specific build settings defined in the manifest.
744744
for (buildConfig, declarationsByPlatform) in allBuildSettings.targetSettings {
@@ -756,7 +756,7 @@ extension PackagePIFProjectBuilder {
756756
}
757757

758758
// Impart the linker flags.
759-
for (platform, settingsByDeclaration) in sourceModule.allBuildSettings.impartedSettings {
759+
for (platform, settingsByDeclaration) in sourceModule.computeAllBuildSettings(observabilityScope: pifBuilder.observabilityScope).impartedSettings {
760760
// Note: A `nil` platform means that the declaration applies to *all* platforms.
761761
for (declaration, stringValues) in settingsByDeclaration {
762762
impartedSettings.append(values: stringValues, to: declaration, platform: platform)

Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Products.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ extension PackagePIFProjectBuilder {
470470
var releaseSettings: ProjectModel.BuildSettings = settings
471471

472472
// Apply target-specific build settings defined in the manifest.
473-
for (buildConfig, declarationsByPlatform) in mainModule.allBuildSettings.targetSettings {
473+
for (buildConfig, declarationsByPlatform) in mainModule.computeAllBuildSettings(observabilityScope: pifBuilder.observabilityScope).targetSettings {
474474
for (platform, declarations) in declarationsByPlatform {
475475
// A `nil` platform means that the declaration applies to *all* platforms.
476476
for (declaration, stringValues) in declarations {
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2025 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 Basics
14+
import Testing
15+
import PackageGraph
16+
import PackageLoading
17+
import PackageModel
18+
import SPMBuildCore
19+
import SwiftBuild
20+
import SwiftBuildSupport
21+
import _InternalTestSupport
22+
import Workspace
23+
24+
extension PIFBuilderParameters {
25+
fileprivate static func constructDefaultParametersForTesting(temporaryDirectory: Basics.AbsolutePath) throws -> Self {
26+
self.init(
27+
isPackageAccessModifierSupported: true,
28+
enableTestability: false,
29+
shouldCreateDylibForDynamicProducts: false,
30+
toolchainLibDir: temporaryDirectory.appending(component: "toolchain-lib-dir"),
31+
pkgConfigDirectories: [],
32+
supportedSwiftVersions: [.v4, .v4_2, .v5, .v6],
33+
pluginScriptRunner: DefaultPluginScriptRunner(
34+
fileSystem: localFileSystem,
35+
cacheDir: temporaryDirectory.appending(component: "plugin-cache-dir"),
36+
toolchain: try UserToolchain.default
37+
),
38+
disableSandbox: false,
39+
pluginWorkingDirectory: temporaryDirectory.appending(component: "plugin-working-dir"),
40+
additionalFileRules: []
41+
)
42+
}
43+
}
44+
45+
fileprivate func withGeneratedPIF(fromFixture fixtureName: String, do doIt: (SwiftBuildSupport.PIF.TopLevelObject, TestingObservability) async throws -> ()) async throws {
46+
try await fixture(name: fixtureName) { fixturePath in
47+
let observabilitySystem = ObservabilitySystem.makeForTesting()
48+
let workspace = try Workspace(
49+
fileSystem: localFileSystem,
50+
forRootPackage: fixturePath,
51+
customManifestLoader: ManifestLoader(toolchain: UserToolchain.default),
52+
delegate: MockWorkspaceDelegate()
53+
)
54+
let rootInput = PackageGraphRootInput(packages: [fixturePath], dependencies: [])
55+
let graph = try await workspace.loadPackageGraph(
56+
rootInput: rootInput,
57+
observabilityScope: observabilitySystem.topScope
58+
)
59+
let builder = PIFBuilder(
60+
graph: graph,
61+
parameters: try PIFBuilderParameters.constructDefaultParametersForTesting(temporaryDirectory: fixturePath),
62+
fileSystem: localFileSystem,
63+
observabilityScope: observabilitySystem.topScope
64+
)
65+
let pif = try await builder.constructPIF(
66+
buildParameters: mockBuildParameters(destination: .host)
67+
)
68+
try await doIt(pif, observabilitySystem)
69+
}
70+
}
71+
72+
extension SwiftBuildSupport.PIF.Workspace {
73+
fileprivate func project(named name: String) throws -> SwiftBuildSupport.PIF.Project {
74+
let matchingProjects = projects.filter {
75+
$0.underlying.name == name
76+
}
77+
if matchingProjects.isEmpty {
78+
throw StringError("No project named \(name) in PIF workspace")
79+
} else if matchingProjects.count > 1 {
80+
throw StringError("Multiple projects named \(name) in PIF workspace")
81+
} else {
82+
return matchingProjects[0]
83+
}
84+
}
85+
}
86+
87+
extension SwiftBuildSupport.PIF.Project {
88+
fileprivate func target(named name: String) throws -> ProjectModel.BaseTarget {
89+
let matchingTargets = underlying.targets.filter {
90+
$0.common.name == name
91+
}
92+
if matchingTargets.isEmpty {
93+
throw StringError("No target named \(name) in PIF project")
94+
} else if matchingTargets.count > 1 {
95+
throw StringError("Multiple target named \(name) in PIF project")
96+
} else {
97+
return matchingTargets[0]
98+
}
99+
}
100+
}
101+
102+
extension SwiftBuild.ProjectModel.BaseTarget {
103+
fileprivate func buildConfig(named name: String) throws -> SwiftBuild.ProjectModel.BuildConfig {
104+
let matchingConfigs = common.buildConfigs.filter {
105+
$0.name == name
106+
}
107+
if matchingConfigs.isEmpty {
108+
throw StringError("No config named \(name) in PIF target")
109+
} else if matchingConfigs.count > 1 {
110+
throw StringError("Multiple configs named \(name) in PIF target")
111+
} else {
112+
return matchingConfigs[0]
113+
}
114+
}
115+
}
116+
117+
@Suite
118+
struct PIFBuilderTests {
119+
@Test func platformConditionBasics() async throws {
120+
try await withGeneratedPIF(fromFixture: "PIFBuilder/UnknownPlatforms") { pif, observabilitySystem in
121+
// We should emit a warning to the PIF log about the unknown platform
122+
#expect(observabilitySystem.diagnostics.filter {
123+
$0.severity == .warning && $0.message.contains("Ignoring settings assignments for unknown platform 'DoesNotExist'")
124+
}.count > 0)
125+
126+
let releaseConfig = try pif.workspace
127+
.project(named: "UnknownPlatforms")
128+
.target(named: "UnknownPlatforms")
129+
.buildConfig(named: "Release")
130+
131+
// The platforms with conditional settings should have those propagated to the PIF.
132+
#expect(releaseConfig.settings.platformSpecificSettings[.linux]?[.SWIFT_ACTIVE_COMPILATION_CONDITIONS] == ["$(inherited)", "BAR"])
133+
#expect(releaseConfig.settings.platformSpecificSettings[.macOS]?[.SWIFT_ACTIVE_COMPILATION_CONDITIONS] == ["$(inherited)", "BAZ"])
134+
// Platforms without conditional settings should get the default.
135+
#expect(releaseConfig.settings.platformSpecificSettings[.windows]?[.SWIFT_ACTIVE_COMPILATION_CONDITIONS] == ["$(inherited)"])
136+
}
137+
}
138+
}

0 commit comments

Comments
 (0)