Skip to content

Commit 9854e40

Browse files
MaxDesiatovMaxDesiatov
authored
[NFC] Group BuildParameters properties into separate structs (#6976)
### Motivation: `BuildParameters` initializer already takes about 30 different arguments. That makes it hard to work with even with auto-complete. Since these initializer arguments and properties weren't previously grouped, we should group them by purpose to reduce the amount of initializer arguments and properties we have to maintain in a single `BuildParameters` type. ### Modifications: Created separate structs to group these build parameters by purpose: `DriverParameters`, `LinkingParameters`, and `TestingParameters`. Deprecated the existing properties and made the proxies to new properties on the new types. These new types are now stored in separate files in the `BuildParameters` subdirectory. Also group a couple of files in the same `SPMBuildCore` target to separate subdirectories: `BuildSystem` and `Plugin` for easier navigation and visual search in a project navigator or when searching on GitHub or directly on the file system. ### Result: `BuildParameters` type is easier to work with. It's easier to find a build parameter property by its purpose, as some of them are now grouped in separate types.
1 parent d90d52e commit 9854e40

36 files changed

+765
-740
lines changed

‎Sources/Build/BuildDescription/ClangTargetBuildDescription.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ public final class ClangTargetBuildDescription {
292292
}
293293

294294
// Enable the correct lto mode if requested.
295-
switch self.buildParameters.linkTimeOptimizationMode {
295+
switch self.buildParameters.linkingParameters.linkTimeOptimizationMode {
296296
case nil:
297297
break
298298
case .full:

‎Sources/Build/BuildDescription/ProductBuildDescription.swift

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
107107
}
108108

109109
private var deadStripArguments: [String] {
110-
if !self.buildParameters.linkerDeadStrip {
110+
if !self.buildParameters.linkingParameters.linkerDeadStrip {
111111
return []
112112
}
113113

@@ -154,7 +154,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
154154
args += self.additionalFlags
155155

156156
// pass `-v` during verbose builds.
157-
if self.buildParameters.verboseOutput {
157+
if self.buildParameters.outputParameters.isVerbose {
158158
args += ["-v"]
159159
}
160160

@@ -169,7 +169,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
169169
args += self.dylibs.map { "-l" + $0.product.name }
170170

171171
// Add arguments needed for code coverage if it is enabled.
172-
if self.buildParameters.enableCodeCoverage {
172+
if self.buildParameters.testingParameters.enableCodeCoverage {
173173
args += ["-profile-coverage-mapping", "-profile-generate"]
174174
}
175175

@@ -198,7 +198,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
198198
return []
199199
case .test:
200200
// Test products are bundle when using objectiveC, executable when using test entry point.
201-
switch self.buildParameters.testProductStyle {
201+
switch self.buildParameters.testingParameters.testProductStyle {
202202
case .loadableBundle:
203203
args += ["-Xlinker", "-bundle"]
204204
case .entryPointExecutable:
@@ -215,7 +215,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
215215
case .executable, .snippet:
216216
// Link the Swift stdlib statically, if requested.
217217
// TODO: unify this logic with SwiftTargetBuildDescription.stdlibArguments
218-
if self.buildParameters.shouldLinkStaticSwiftStdlib {
218+
if self.buildParameters.linkingParameters.shouldLinkStaticSwiftStdlib {
219219
if self.buildParameters.targetTriple.isDarwin() {
220220
self.observabilityScope.emit(.swiftBackDeployError)
221221
} else if self.buildParameters.targetTriple.isSupportingStaticStdlib {
@@ -235,8 +235,10 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
235235
// Support for linking tests against executables is conditional on the tools
236236
// version of the package that defines the executable product.
237237
let executableTarget = try product.executableTarget
238-
if let target = executableTarget.underlyingTarget as? SwiftTarget, self.toolsVersion >= .v5_5,
239-
self.buildParameters.canRenameEntrypointFunctionName, target.supportsTestableExecutablesFeature
238+
if let target = executableTarget.underlyingTarget as? SwiftTarget,
239+
self.toolsVersion >= .v5_5,
240+
self.buildParameters.driverParameters.canRenameEntrypointFunctionName,
241+
target.supportsTestableExecutablesFeature
240242
{
241243
if let flags = buildParameters.linkerFlagsForRenamingMainFunction(of: executableTarget) {
242244
args += flags
@@ -258,7 +260,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
258260

259261
// Set rpath such that dynamic libraries are looked up
260262
// adjacent to the product, unless overridden.
261-
if !self.buildParameters.shouldDisableLocalRpath {
263+
if !self.buildParameters.linkingParameters.shouldDisableLocalRpath {
262264
if self.buildParameters.targetTriple.isLinux() {
263265
args += ["-Xlinker", "-rpath=$ORIGIN"]
264266
} else if self.buildParameters.targetTriple.isDarwin() {

‎Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public final class SwiftTargetBuildDescription {
9090
let relativeSources = self.target.sources.relativePaths
9191
+ self.derivedSources.relativePaths
9292
+ self.pluginDerivedSources.relativePaths
93-
let ltoEnabled = self.buildParameters.linkTimeOptimizationMode != nil
93+
let ltoEnabled = self.buildParameters.linkingParameters.linkTimeOptimizationMode != nil
9494
let objectFileExtension = ltoEnabled ? "bc" : "o"
9595
return try relativeSources.map {
9696
try AbsolutePath(
@@ -312,7 +312,7 @@ public final class SwiftTargetBuildDescription {
312312
return
313313
}
314314

315-
guard buildParameters.targetTriple.isDarwin(), buildParameters.experimentalTestOutput else {
315+
guard buildParameters.targetTriple.isDarwin(), buildParameters.testingParameters.experimentalTestOutput else {
316316
return
317317
}
318318

@@ -455,7 +455,7 @@ public final class SwiftTargetBuildDescription {
455455
args += ["-swift-version", self.swiftVersion.rawValue]
456456

457457
// pass `-v` during verbose builds.
458-
if self.buildParameters.verboseOutput {
458+
if self.buildParameters.outputParameters.isVerbose {
459459
args += ["-v"]
460460
}
461461

@@ -498,7 +498,7 @@ public final class SwiftTargetBuildDescription {
498498
// we can rename the symbol unconditionally.
499499
// No `-` for these flags because the set of Strings in driver.supportedFrontendFlags do
500500
// not have a leading `-`
501-
if self.buildParameters.canRenameEntrypointFunctionName,
501+
if self.buildParameters.driverParameters.canRenameEntrypointFunctionName,
502502
self.buildParameters.linkerFlagsForRenamingMainFunction(of: self.target) != nil
503503
{
504504
args += ["-Xfrontend", "-entry-point-function-name", "-Xfrontend", "\(self.target.c99name)_main"]
@@ -521,12 +521,12 @@ public final class SwiftTargetBuildDescription {
521521
}
522522

523523
// Add arguments needed for code coverage if it is enabled.
524-
if self.buildParameters.enableCodeCoverage {
524+
if self.buildParameters.testingParameters.enableCodeCoverage {
525525
args += ["-profile-coverage-mapping", "-profile-generate"]
526526
}
527527

528528
// Add arguments to colorize output if stdout is tty
529-
if self.buildParameters.colorizedOutput {
529+
if self.buildParameters.outputParameters.isColorized {
530530
args += ["-color-diagnostics"]
531531
}
532532

@@ -535,7 +535,7 @@ public final class SwiftTargetBuildDescription {
535535

536536
// Add the output for the `.swiftinterface`, if requested or if library evolution has been enabled some other
537537
// way.
538-
if self.buildParameters.enableParseableModuleInterfaces || args.contains("-enable-library-evolution") {
538+
if self.buildParameters.driverParameters.enableParseableModuleInterfaces || args.contains("-enable-library-evolution") {
539539
args += ["-emit-module-interface-path", self.parseableModuleInterfaceOutputPath.pathString]
540540
}
541541

@@ -554,7 +554,7 @@ public final class SwiftTargetBuildDescription {
554554
// args += self.buildParameters.flags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()
555555

556556
// Enable the correct LTO mode if requested.
557-
switch self.buildParameters.linkTimeOptimizationMode {
557+
switch self.buildParameters.linkingParameters.linkTimeOptimizationMode {
558558
case nil:
559559
break
560560
case .full:
@@ -675,7 +675,7 @@ public final class SwiftTargetBuildDescription {
675675
// Write out the entries for each source file.
676676
let sources = self.sources
677677
let objects = try self.objects
678-
let ltoEnabled = self.buildParameters.linkTimeOptimizationMode != nil
678+
let ltoEnabled = self.buildParameters.linkingParameters.linkTimeOptimizationMode != nil
679679
let objectKey = ltoEnabled ? "llvm-bc" : "object"
680680

681681
for idx in 0..<sources.count {
@@ -815,7 +815,7 @@ public final class SwiftTargetBuildDescription {
815815
// test targets must be built with -enable-testing
816816
// since its required for test discovery (the non objective-c reflection kind)
817817
return ["-enable-testing"]
818-
} else if self.buildParameters.enableTestability {
818+
} else if self.buildParameters.testingParameters.enableTestability {
819819
return ["-enable-testing"]
820820
} else {
821821
return []
@@ -832,7 +832,7 @@ public final class SwiftTargetBuildDescription {
832832
private var stdlibArguments: [String] {
833833
var arguments: [String] = []
834834

835-
let isLinkingStaticStdlib = self.buildParameters.shouldLinkStaticSwiftStdlib
835+
let isLinkingStaticStdlib = self.buildParameters.linkingParameters.shouldLinkStaticSwiftStdlib
836836
&& self.buildParameters.targetTriple.isSupportingStaticStdlib
837837
if isLinkingStaticStdlib {
838838
arguments += ["-static-stdlib"]

‎Sources/Build/BuildManifest/LLBuildManifestBuilder+Product.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ extension LLBuildManifestBuilder {
2121
let testInputs: [AbsolutePath]
2222
if buildProduct.product.type == .test
2323
&& buildProduct.buildParameters.targetTriple.isDarwin()
24-
&& buildProduct.buildParameters.experimentalTestOutput {
24+
&& buildProduct.buildParameters.testingParameters.experimentalTestOutput {
2525
let testBundleInfoPlistPath = try buildProduct.binaryPath.parentDirectory.parentDirectory.appending(component: "Info.plist")
2626
testInputs = [testBundleInfoPlistPath]
2727

‎Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ extension LLBuildManifestBuilder {
3939
let moduleNode = Node.file(target.moduleOutputPath)
4040
let cmdOutputs = objectNodes + [moduleNode]
4141

42-
if self.buildParameters.useIntegratedSwiftDriver {
42+
if self.buildParameters.driverParameters.useIntegratedSwiftDriver {
4343
try self.addSwiftCmdsViaIntegratedDriver(
4444
target,
4545
inputs: inputs,
@@ -124,7 +124,7 @@ extension LLBuildManifestBuilder {
124124
// common intermediate dependency modules, such dependencies can lead
125125
// to cycles in the resulting manifest.
126126
var manifestNodeInputs: [Node] = []
127-
if self.buildParameters.useExplicitModuleBuild && !isMainModule(job) {
127+
if self.buildParameters.driverParameters.useExplicitModuleBuild && !isMainModule(job) {
128128
manifestNodeInputs = jobInputs
129129
} else {
130130
manifestNodeInputs = (inputs + jobInputs).uniqued()

‎Sources/Build/BuildManifest/LLBuildManifestBuilder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public class LLBuildManifestBuilder {
8282

8383
addPackageStructureCommand()
8484
addBinaryDependencyCommands()
85-
if self.buildParameters.useExplicitModuleBuild {
85+
if self.buildParameters.driverParameters.useExplicitModuleBuild {
8686
// Explicit module builds use the integrated driver directly and
8787
// require that every target's build jobs specify its dependencies explicitly to plan
8888
// its build.

‎Sources/Build/BuildOperation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
112112
) {
113113
/// Checks if stdout stream is tty.
114114
var buildParameters = buildParameters
115-
buildParameters.colorizedOutput = outputStream.isTTY
115+
buildParameters.outputParameters.isColorized = outputStream.isTTY
116116

117117
self.buildParameters = buildParameters
118118
self.cacheBuildManifest = cacheBuildManifest

‎Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ final class TestEntryPointCommand: CustomLLBuildCommand, TestBuildCommand {
216216
}
217217

218218
let testObservabilitySetup: String
219-
if self.context.buildParameters.experimentalTestOutput
219+
if self.context.buildParameters.testingParameters.experimentalTestOutput
220220
&& self.context.buildParameters.targetTriple.supportsTestSummary {
221221
testObservabilitySetup = "_ = SwiftPMXCTestObserver()\n"
222222
} else {
@@ -352,7 +352,7 @@ public struct BuildDescription: Codable {
352352
self.testEntryPointCommands = testEntryPointCommands
353353
self.copyCommands = copyCommands
354354
self.writeCommands = writeCommands
355-
self.explicitTargetDependencyImportCheckingMode = plan.buildParameters
355+
self.explicitTargetDependencyImportCheckingMode = plan.buildParameters.driverParameters
356356
.explicitTargetDependencyImportCheckingMode
357357
self.targetDependencyMap = try plan.targets.reduce(into: [TargetName: [TargetName]]()) {
358358
let deps = try $1.target.recursiveDependencies(satisfying: plan.buildParameters.buildEnvironment)

‎Sources/Build/BuildPlan/BuildPlan+Product.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ extension BuildPlan {
244244
}
245245

246246
// Add derived test targets, if necessary
247-
if buildParameters.testProductStyle.requiresAdditionalDerivedTestTargets {
247+
if buildParameters.testingParameters.testProductStyle.requiresAdditionalDerivedTestTargets {
248248
if product.type == .test, let derivedTestTargets = derivedTestTargetsMap[product] {
249249
staticTargets.append(contentsOf: derivedTestTargets)
250250
}

‎Sources/Build/BuildPlan/BuildPlan+Test.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ extension BuildPlan {
3131
_ fileSystem: FileSystem,
3232
_ observabilityScope: ObservabilityScope
3333
) throws -> [(product: ResolvedProduct, discoveryTargetBuildDescription: SwiftTargetBuildDescription?, entryPointTargetBuildDescription: SwiftTargetBuildDescription)] {
34-
guard buildParameters.testProductStyle.requiresAdditionalDerivedTestTargets,
35-
case .entryPointExecutable(let explicitlyEnabledDiscovery, let explicitlySpecifiedPath) = buildParameters.testProductStyle
34+
guard buildParameters.testingParameters.testProductStyle.requiresAdditionalDerivedTestTargets,
35+
case .entryPointExecutable(let explicitlyEnabledDiscovery, let explicitlySpecifiedPath) =
36+
buildParameters.testingParameters.testProductStyle
3637
else {
3738
throw InternalError("makeTestManifestTargets should not be used for build plan which does not require additional derived test targets")
3839
}

‎Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
368368
}
369369

370370
// Plan the derived test targets, if necessary.
371-
if buildParameters.testProductStyle.requiresAdditionalDerivedTestTargets {
371+
if buildParameters.testingParameters.testProductStyle.requiresAdditionalDerivedTestTargets {
372372
let derivedTestTargets = try Self.makeDerivedTestTargets(buildParameters, graph, self.fileSystem, self.observabilityScope)
373373
for item in derivedTestTargets {
374374
var derivedTestTargets = [item.entryPointTargetBuildDescription.target]

‎Sources/Commands/SwiftTestTool.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ final class ParallelTestRunner {
879879

880880
// Print test results.
881881
for test in processedTests.get() {
882-
if (!test.success || shouldOutputSuccess) && !buildParameters.experimentalTestOutput {
882+
if (!test.success || shouldOutputSuccess) && !buildParameters.testingParameters.experimentalTestOutput {
883883
// command's result output goes on stdout
884884
// ie "swift test" should output to stdout
885885
print(test.output)

‎Sources/Commands/Utilities/PluginDelegate.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ final class PluginDelegate: PluginInvocationDelegate {
171171
// which ones they are until we've built them and can examine the binaries.
172172
let toolchain = try swiftTool.getTargetToolchain()
173173
var buildParameters = try swiftTool.buildParameters()
174-
buildParameters.enableTestability = true
175-
buildParameters.enableCodeCoverage = parameters.enableCodeCoverage
174+
buildParameters.testingParameters.enableTestability = true
175+
buildParameters.testingParameters.enableCodeCoverage = parameters.enableCodeCoverage
176176
let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)
177177
try buildSystem.build(subset: .allIncludingTests)
178178

‎Sources/Commands/Utilities/TestingSupport.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ enum TestingSupport {
161161
}
162162

163163
// Add the code coverage related variables.
164-
if buildParameters.enableCodeCoverage {
164+
if buildParameters.testingParameters.enableCodeCoverage {
165165
// Defines the path at which the profraw files will be written on test execution.
166166
//
167167
// `%m` will create a pool of profraw files and append the data from
@@ -209,12 +209,12 @@ extension SwiftTool {
209209
experimentalTestOutput: Bool = false
210210
) throws -> BuildParameters {
211211
var parameters = try self.buildParameters()
212-
parameters.enableCodeCoverage = enableCodeCoverage
212+
parameters.testingParameters.enableCodeCoverage = enableCodeCoverage
213213
// for test commands, we normally enable building with testability
214214
// but we let users override this with a flag
215-
parameters.enableTestability = enableTestability ?? true
215+
parameters.testingParameters.enableTestability = enableTestability ?? true
216216
parameters.shouldSkipBuilding = shouldSkipBuilding
217-
parameters.experimentalTestOutput = experimentalTestOutput
217+
parameters.testingParameters.experimentalTestOutput = experimentalTestOutput
218218
return parameters
219219
}
220220
}

‎Sources/CoreCommands/BuildSystemSupport.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ private struct NativeBuildSystemFactory: BuildSystemFactory {
3131
customLogLevel: Diagnostic.Severity?,
3232
customObservabilityScope: ObservabilityScope?
3333
) throws -> any BuildSystem {
34-
let testEntryPointPath = customBuildParameters?.testProductStyle.explicitlySpecifiedEntryPointPath
34+
let testEntryPointPath = customBuildParameters?.testingParameters.testProductStyle.explicitlySpecifiedEntryPointPath
3535
let graphLoader = { try self.swiftTool.loadPackageGraph(explicitProduct: explicitProduct, testEntryPointPath: testEntryPointPath) }
3636
return try BuildOperation(
3737
buildParameters: customBuildParameters ?? self.swiftTool.buildParameters(),

0 commit comments

Comments
 (0)