Skip to content

Commit 3251a55

Browse files
authored
[NFC] SPMBuildCore: make BuildSystemProvider.Provider a protocol (#6984)
### Motivation: When requirements are expressed as a closure type (`BuildSystemProvider.Provider` in this case), it confuses the type checker when working on any related code. Multi-line closure type inference has been known to be suboptimal in Swift, and converting it to a protocol localizes "Type of expression is ambiguous without a type annotation" to a specific line that causes the error instead of attaching it to the whole closure. ### Modifications: Created a new `protocol BuildSystemFactory` with a `makeBuildSystem` method that replaces the previous `BuildSystemProvider.Provider` closure typealias. Added new `private struct NativeBuildSystemFactory` and `private struct XcodeBuildSystemFactory` in `BuildSystemSupport.swift`, which conform to the new protocol and are used in the implementation of `defaultBuildSystemProvider` computed property. ### Result: It's much easier to work with `BuildSystemFactory` protocol: it no longer confuses the type checker when changes to the closure body are made or need to be made. Additionally, the new protocol method can use named function arguments instead of unnamed ones, since closures don't support the former.
1 parent 18ea3ea commit 3251a55

File tree

2 files changed

+85
-49
lines changed

2 files changed

+85
-49
lines changed

Sources/CoreCommands/BuildSystemSupport.swift

Lines changed: 60 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,41 +19,67 @@ import struct PackageGraph.PackageGraph
1919
import struct PackageLoading.FileRuleDescription
2020
import protocol TSCBasic.OutputByteStream
2121

22+
private struct NativeBuildSystemFactory: BuildSystemFactory {
23+
let swiftTool: SwiftTool
24+
25+
func makeBuildSystem(
26+
explicitProduct: String?,
27+
cacheBuildManifest: Bool,
28+
customBuildParameters: BuildParameters?,
29+
customPackageGraphLoader: (() throws -> PackageGraph)?,
30+
customOutputStream: OutputByteStream?,
31+
customLogLevel: Diagnostic.Severity?,
32+
customObservabilityScope: ObservabilityScope?
33+
) throws -> any BuildSystem {
34+
let testEntryPointPath = customBuildParameters?.testProductStyle.explicitlySpecifiedEntryPointPath
35+
let graphLoader = { try self.swiftTool.loadPackageGraph(explicitProduct: explicitProduct, testEntryPointPath: testEntryPointPath) }
36+
return try BuildOperation(
37+
buildParameters: customBuildParameters ?? self.swiftTool.buildParameters(),
38+
cacheBuildManifest: cacheBuildManifest && self.swiftTool.canUseCachedBuildManifest(),
39+
packageGraphLoader: customPackageGraphLoader ?? graphLoader,
40+
pluginConfiguration: .init(
41+
scriptRunner: self.swiftTool.getPluginScriptRunner(),
42+
workDirectory: try self.swiftTool.getActiveWorkspace().location.pluginWorkingDirectory,
43+
disableSandbox: self.swiftTool.shouldDisableSandbox
44+
),
45+
additionalFileRules: FileRuleDescription.swiftpmFileTypes,
46+
pkgConfigDirectories: self.swiftTool.options.locations.pkgConfigDirectories,
47+
outputStream: customOutputStream ?? self.swiftTool.outputStream,
48+
logLevel: customLogLevel ?? self.swiftTool.logLevel,
49+
fileSystem: self.swiftTool.fileSystem,
50+
observabilityScope: customObservabilityScope ?? self.swiftTool.observabilityScope)
51+
}
52+
}
53+
54+
private struct XcodeBuildSystemFactory: BuildSystemFactory {
55+
let swiftTool: SwiftTool
56+
57+
func makeBuildSystem(
58+
explicitProduct: String?,
59+
cacheBuildManifest: Bool,
60+
customBuildParameters: BuildParameters?,
61+
customPackageGraphLoader: (() throws -> PackageGraph)?,
62+
customOutputStream: OutputByteStream?,
63+
customLogLevel: Diagnostic.Severity?,
64+
customObservabilityScope: ObservabilityScope?
65+
) throws -> any BuildSystem {
66+
let graphLoader = { try self.swiftTool.loadPackageGraph(explicitProduct: explicitProduct) }
67+
return try XcodeBuildSystem(
68+
buildParameters: customBuildParameters ?? self.swiftTool.buildParameters(),
69+
packageGraphLoader: customPackageGraphLoader ?? graphLoader,
70+
outputStream: customOutputStream ?? self.swiftTool.outputStream,
71+
logLevel: customLogLevel ?? self.swiftTool.logLevel,
72+
fileSystem: self.swiftTool.fileSystem,
73+
observabilityScope: customObservabilityScope ?? self.swiftTool.observabilityScope
74+
)
75+
}
76+
}
77+
2278
extension SwiftTool {
2379
public var defaultBuildSystemProvider: BuildSystemProvider {
24-
get throws {
25-
return .init(providers: [
26-
.native: { (explicitProduct: String?, cacheBuildManifest: Bool, customBuildParameters: BuildParameters?, customPackageGraphLoader: (() throws -> PackageGraph)?, customOutputStream: OutputByteStream?, customLogLevel: Diagnostic.Severity?, customObservabilityScope: ObservabilityScope?) throws -> BuildSystem in
27-
let testEntryPointPath = customBuildParameters?.testProductStyle.explicitlySpecifiedEntryPointPath
28-
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct, testEntryPointPath: testEntryPointPath) }
29-
return try BuildOperation(
30-
buildParameters: customBuildParameters ?? self.buildParameters(),
31-
cacheBuildManifest: cacheBuildManifest && self.canUseCachedBuildManifest(),
32-
packageGraphLoader: customPackageGraphLoader ?? graphLoader,
33-
pluginConfiguration: .init(
34-
scriptRunner: self.getPluginScriptRunner(),
35-
workDirectory: try self.getActiveWorkspace().location.pluginWorkingDirectory,
36-
disableSandbox: self.shouldDisableSandbox
37-
),
38-
additionalFileRules: FileRuleDescription.swiftpmFileTypes,
39-
pkgConfigDirectories: self.options.locations.pkgConfigDirectories,
40-
outputStream: customOutputStream ?? self.outputStream,
41-
logLevel: customLogLevel ?? self.logLevel,
42-
fileSystem: self.fileSystem,
43-
observabilityScope: customObservabilityScope ?? self.observabilityScope)
44-
},
45-
.xcode: { (explicitProduct: String?, cacheBuildManifest: Bool, customBuildParameters: BuildParameters?, customPackageGraphLoader: (() throws -> PackageGraph)?, customOutputStream: OutputByteStream?, customLogLevel: Diagnostic.Severity?, customObservabilityScope: ObservabilityScope?) throws -> BuildSystem in
46-
let graphLoader = { try self.loadPackageGraph(explicitProduct: explicitProduct) }
47-
return try XcodeBuildSystem(
48-
buildParameters: customBuildParameters ?? self.buildParameters(),
49-
packageGraphLoader: customPackageGraphLoader ?? graphLoader,
50-
outputStream: customOutputStream ?? self.outputStream,
51-
logLevel: customLogLevel ?? self.logLevel,
52-
fileSystem: self.fileSystem,
53-
observabilityScope: customObservabilityScope ?? self.observabilityScope
54-
)
55-
},
56-
])
57-
}
80+
.init(providers: [
81+
.native: NativeBuildSystemFactory(swiftTool: self),
82+
.xcode: XcodeBuildSystemFactory(swiftTool: self)
83+
])
5884
}
5985
}

Sources/SPMBuildCore/BuildSystem.swift

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -87,26 +87,28 @@ public protocol BuildPlan {
8787
func createREPLArguments() throws -> [String]
8888
}
8989

90+
public protocol BuildSystemFactory {
91+
func makeBuildSystem(
92+
explicitProduct: String?,
93+
cacheBuildManifest: Bool,
94+
customBuildParameters: BuildParameters?,
95+
customPackageGraphLoader: (() throws -> PackageGraph)?,
96+
customOutputStream: OutputByteStream?,
97+
customLogLevel: Diagnostic.Severity?,
98+
customObservabilityScope: ObservabilityScope?
99+
) throws -> any BuildSystem
100+
}
101+
90102
public struct BuildSystemProvider {
91103
// TODO: In the future, we may want this to be about specific capabilities of a build system rather than choosing a concrete one.
92104
public enum Kind: String, CaseIterable {
93105
case native
94106
case xcode
95107
}
96108

97-
public typealias Provider = (
98-
_ explicitProduct: String?,
99-
_ cacheBuildManifest: Bool,
100-
_ customBuildParameters: BuildParameters?,
101-
_ customPackageGraphLoader: (() throws -> PackageGraph)?,
102-
_ customOutputStream: OutputByteStream?,
103-
_ customLogLevel: Diagnostic.Severity?,
104-
_ customObservabilityScope: ObservabilityScope?
105-
) throws -> BuildSystem
106-
107-
public let providers: [Kind:Provider]
109+
public let providers: [Kind: any BuildSystemFactory]
108110

109-
public init(providers: [Kind:Provider]) {
111+
public init(providers: [Kind: any BuildSystemFactory]) {
110112
self.providers = providers
111113
}
112114

@@ -119,11 +121,19 @@ public struct BuildSystemProvider {
119121
customOutputStream: OutputByteStream? = .none,
120122
customLogLevel: Diagnostic.Severity? = .none,
121123
customObservabilityScope: ObservabilityScope? = .none
122-
) throws -> BuildSystem {
123-
guard let provider = self.providers[kind] else {
124+
) throws -> any BuildSystem {
125+
guard let buildSystemFactory = self.providers[kind] else {
124126
throw Errors.buildSystemProviderNotRegistered(kind: kind)
125127
}
126-
return try provider(explicitProduct, cacheBuildManifest, customBuildParameters, customPackageGraphLoader, customOutputStream, customLogLevel, customObservabilityScope)
128+
return try buildSystemFactory.makeBuildSystem(
129+
explicitProduct: explicitProduct,
130+
cacheBuildManifest: cacheBuildManifest,
131+
customBuildParameters: customBuildParameters,
132+
customPackageGraphLoader: customPackageGraphLoader,
133+
customOutputStream: customOutputStream,
134+
customLogLevel: customLogLevel,
135+
customObservabilityScope: customObservabilityScope
136+
)
127137
}
128138
}
129139

0 commit comments

Comments
 (0)