Skip to content

Commit 6e3f087

Browse files
authored
Assure trait-guarded dependencies are not included in resolution; precompute enabled traits before resolution (#8852)
Trait-guarded dependencies were still being considered during dependency resolution, when they should be excluded if they aren't being used in any other scenario. Additionally, enabled traits should be pre-computed before entering the resolution stage in order to avoid possible race conditions when navigating through the dependency graph nodes. ### Modifications: Since we have the `--experimental-prune-unused-dependencies` feature behind an experimental flag, we'll now consider an alternate path that will prune trait-guarded package dependencies from the dependency graph **_if and only if_** said dependency is not used in any other unguarded context. A dictionary wrapper titled `EnabledTraitsMap` is used to store the enabled traits per package for the package graph and is stored as a property within the `Workspace`, with some additional behaviour to return a `["default"]` set of traits if the package has not yet been stored in the dictionary, rather than returning `nil`. Following this behaviour, when passing a set of traits to methods that require them (e.g. for dependency computation, enabled trait computation, etc.) we now require that it is not Optional, since the checks done on a `nil` set of traits were redundant. SwiftCommandState now also stores a `TraitConfiguration`, since we'll want access to this across multiple `SwiftCommand`s, and it is essentially a part of the state anyhow. `TraitOptions` is now included in the `GlobalOptions` for `SwiftCommand`s to supplement this, so that when a `SwiftCommandState` is created we will have access to the user-passed enabled traits. These options, as entitled, are available globally across all the swift package commands, so previous properties that declared `TraitOptions` in these commands has been removed in favour of using it straight from the `GlobalOptions`. ### Result: Trait-guarded dependencies are excluded from dependency resolution, and since traits are pre-computed there should no longer be an issue with race conditions for traits in resolution as well.
1 parent b10658b commit 6e3f087

File tree

50 files changed

+752
-564
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+752
-564
lines changed

Sources/Commands/PackageCommands/APIDiff.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ struct APIDiff: AsyncSwiftCommand {
7272
help: "One or more targets to include in the API comparison. If present, only the specified targets (and any products specified using `--products`) will be compared.")
7373
var targets: [String] = []
7474

75-
@OptionGroup(visibility: .hidden)
76-
package var traits: TraitOptions
77-
7875
@Option(name: .customLong("baseline-dir"),
7976
help: "The path to a directory used to store API baseline files. If unspecified, a temporary directory will be used.")
8077
var overrideBaselineDir: Basics.AbsolutePath?
@@ -106,7 +103,6 @@ struct APIDiff: AsyncSwiftCommand {
106103
)
107104
} else {
108105
let buildSystem = try await swiftCommandState.createBuildSystem(
109-
traitConfiguration: .init(traitOptions: self.traits),
110106
cacheBuildManifest: false,
111107
)
112108
try await runWithSwiftPMCoordinatedDiffing(
@@ -210,7 +206,6 @@ struct APIDiff: AsyncSwiftCommand {
210206
)
211207
let delegate = DiagnosticsCapturingBuildSystemDelegate()
212208
let buildSystem = try await swiftCommandState.createBuildSystem(
213-
traitConfiguration: .init(traitOptions: self.traits),
214209
cacheBuildManifest: false,
215210
productsBuildParameters: productsBuildParameters,
216211
delegate: delegate
@@ -319,7 +314,6 @@ struct APIDiff: AsyncSwiftCommand {
319314
// Build the baseline module.
320315
// FIXME: We need to implement the build tool invocation closure here so that build tool plugins work with the APIDigester. rdar://86112934
321316
let buildSystem = try await swiftCommandState.createBuildSystem(
322-
traitConfiguration: .init(),
323317
cacheBuildManifest: false,
324318
productsBuildParameters: productsBuildParameters,
325319
packageGraphLoader: { graph }

Sources/Commands/PackageCommands/DumpCommands.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ struct DumpSymbolGraph: AsyncSwiftCommand {
5353
// We turn build manifest caching off because we need the build plan.
5454
let buildSystem = try await swiftCommandState.createBuildSystem(
5555
// We are enabling all traits for dumping the symbol graph.
56-
traitConfiguration: .init(enableAllTraits: true),
56+
enableAllTraits: true,
5757
cacheBuildManifest: false
5858
)
5959
// TODO pass along the various flags as associated values to the symbol graph build output (e.g. includeSPISymbols)

Sources/Commands/PackageCommands/Install.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ extension SwiftPackageCommand {
8888
commandState.preferredBuildConfiguration = .release
8989
}
9090

91-
try await commandState.createBuildSystem(explicitProduct: productToInstall.name, traitConfiguration: .init())
91+
try await commandState.createBuildSystem(explicitProduct: productToInstall.name)
9292
.build(subset: .product(productToInstall.name), buildOutputs: [])
9393

9494
let binPath = try commandState.productsBuildParameters.buildPath.appending(component: productToInstall.name)

Sources/Commands/PackageCommands/Migrate.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ extension SwiftPackageCommand {
196196
}
197197

198198
return try await swiftCommandState.createBuildSystem(
199-
traitConfiguration: .init(),
200199
// Don't attempt to cache manifests with temporary
201200
// feature flags added just for migration purposes.
202201
cacheBuildManifest: false,

Sources/Commands/PackageCommands/PluginCommand.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,6 @@ struct PluginCommand: AsyncSwiftCommand {
338338
// Build or bring up-to-date any executable host-side tools on which this plugin depends. Add them and any binary dependencies to the tool-names-to-path map.
339339
let buildSystem = try await swiftCommandState.createBuildSystem(
340340
explicitBuildSystem: buildSystemKind,
341-
traitConfiguration: .init(),
342341
cacheBuildManifest: false,
343342
productsBuildParameters: swiftCommandState.productsBuildParameters,
344343
toolsBuildParameters: buildParameters,

Sources/Commands/PackageCommands/Resolve.swift

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ extension SwiftPackageCommand {
3131

3232
@Argument(help: "The name of the package to resolve.")
3333
var packageName: String?
34-
35-
/// Specifies the traits to build.
36-
@OptionGroup(visibility: .hidden)
37-
package var traits: TraitOptions
3834
}
3935

4036
struct Resolve: AsyncSwiftCommand {
@@ -50,10 +46,10 @@ extension SwiftPackageCommand {
5046
func run(_ swiftCommandState: SwiftCommandState) async throws {
5147
// If a package is provided, use that to resolve the dependencies.
5248
if let packageName = resolveOptions.packageName {
53-
let workspace = try swiftCommandState.getActiveWorkspace(traitConfiguration: .init(traitOptions: resolveOptions.traits))
49+
let workspace = try swiftCommandState.getActiveWorkspace()
5450
try await workspace.resolve(
5551
packageName: packageName,
56-
root: swiftCommandState.getWorkspaceRoot(traitConfiguration: .init(traitOptions: resolveOptions.traits)),
52+
root: swiftCommandState.getWorkspaceRoot(),
5753
version: resolveOptions.version,
5854
branch: resolveOptions.branch,
5955
revision: resolveOptions.revision,
@@ -64,7 +60,7 @@ extension SwiftPackageCommand {
6460
}
6561
} else {
6662
// Otherwise, run a normal resolve.
67-
try await swiftCommandState.resolve(.init(traitOptions: resolveOptions.traits))
63+
try await swiftCommandState.resolve()
6864
}
6965
}
7066
}

Sources/Commands/Snippets/Cards/SnippetCard.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ struct SnippetCard: Card {
112112

113113
func runExample() async throws {
114114
print("Building '\(snippet.path)'\n")
115-
let buildSystem = try await swiftCommandState.createBuildSystem(explicitProduct: snippet.name, traitConfiguration: .init())
115+
let buildSystem = try await swiftCommandState.createBuildSystem(explicitProduct: snippet.name)
116116
try await buildSystem.build(subset: .product(snippet.name), buildOutputs: [])
117117
let executablePath = try swiftCommandState.productsBuildParameters.buildPath.appending(component: snippet.name)
118118
if let exampleTarget = try await buildSystem.getPackageGraph().module(for: snippet.name) {

Sources/Commands/SwiftBuildCommand.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,6 @@ struct BuildCommandOptions: ParsableArguments {
110110
@OptionGroup(visibility: .private)
111111
var testLibraryOptions: TestLibraryOptions
112112

113-
/// Specifies the traits to build.
114-
@OptionGroup(visibility: .hidden)
115-
package var traits: TraitOptions
116-
117113
/// If should link the Swift stdlib statically.
118114
@Flag(name: .customLong("static-swift-stdlib"), inversion: .prefixedNo, help: "Link Swift stdlib statically.")
119115
public var shouldLinkStaticSwiftStdlib: Bool = false
@@ -144,7 +140,6 @@ public struct SwiftBuildCommand: AsyncSwiftCommand {
144140
// FIXME: Doesn't seem ideal that we need an explicit build operation, but this concretely uses the `LLBuildManifest`.
145141
guard let buildOperation = try await swiftCommandState.createBuildSystem(
146142
explicitBuildSystem: .native,
147-
traitConfiguration: .init(traitOptions: self.options.traits)
148143
) as? BuildOperation else {
149144
throw StringError("asked for native build system but did not get it")
150145
}
@@ -194,7 +189,6 @@ public struct SwiftBuildCommand: AsyncSwiftCommand {
194189
) async throws {
195190
let buildSystem = try await swiftCommandState.createBuildSystem(
196191
explicitProduct: options.product,
197-
traitConfiguration: .init(traitOptions: self.options.traits),
198192
shouldLinkStaticSwiftStdlib: options.shouldLinkStaticSwiftStdlib,
199193
productsBuildParameters: productsBuildParameters,
200194
toolsBuildParameters: toolsBuildParameters,

Sources/Commands/SwiftRunCommand.swift

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,6 @@ struct RunCommandOptions: ParsableArguments {
8888
@Argument(help: "The executable to run.", completion: .shellCommand("swift package completion-tool list-executables"))
8989
var executable: String?
9090

91-
/// Specifies the traits to build the product with.
92-
@OptionGroup(visibility: .hidden)
93-
package var traits: TraitOptions
94-
9591
/// The arguments to pass to the executable.
9692
@Argument(parsing: .captureForPassthrough,
9793
help: "The arguments to pass to the executable.")
@@ -139,7 +135,6 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
139135
// FIXME: We need to implement the build tool invocation closure here so that build tool plugins work with the REPL. rdar://86112934
140136
let buildSystem = try await swiftCommandState.createBuildSystem(
141137
explicitBuildSystem: .native,
142-
traitConfiguration: .init(traitOptions: self.options.traits),
143138
cacheBuildManifest: false,
144139
packageGraphLoader: asyncUnsafeGraphLoader
145140
)
@@ -164,7 +159,6 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
164159
do {
165160
let buildSystem = try await swiftCommandState.createBuildSystem(
166161
explicitProduct: options.executable,
167-
traitConfiguration: .init(traitOptions: self.options.traits)
168162
)
169163
let productName = try await findProductName(in: buildSystem.getPackageGraph())
170164
if options.shouldBuildTests {
@@ -220,9 +214,9 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
220214
do {
221215
let buildSystem = try await swiftCommandState.createBuildSystem(
222216
explicitProduct: options.executable,
223-
traitConfiguration: .init(traitOptions: self.options.traits)
224217
)
225-
let productName = try await findProductName(in: buildSystem.getPackageGraph())
218+
let modulesGraph = try await buildSystem.getPackageGraph()
219+
let productName = try findProductName(in: modulesGraph)
226220
if options.shouldBuildTests {
227221
try await buildSystem.build(subset: .allIncludingTests, buildOutputs: [])
228222
} else if options.shouldBuild {

Sources/Commands/SwiftTestCommand.swift

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,6 @@ struct TestCommandOptions: ParsableArguments {
220220
var enableExperimentalTestOutput: Bool {
221221
return testOutput == .experimentalSummary
222222
}
223-
224-
@OptionGroup(visibility: .hidden)
225-
package var traits: TraitOptions
226223
}
227224

228225
/// Tests filtering specifier, which is used to filter tests to run.
@@ -659,7 +656,7 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
659656
productsBuildParameters: productsBuildParameters,
660657
toolsBuildParameters: toolsBuildParameters,
661658
testProduct: self.options.sharedOptions.testProduct,
662-
traitConfiguration: .init(traitOptions: self.options.traits)
659+
traitConfiguration: .init(traitOptions: self.globalOptions.traits)
663660
)
664661
}
665662

@@ -741,9 +738,6 @@ extension SwiftTestCommand {
741738
@OptionGroup()
742739
var testEventStreamOptions: TestEventStreamOptions
743740

744-
@OptionGroup(visibility: .hidden)
745-
package var traits: TraitOptions
746-
747741
// for deprecated passthrough from SwiftTestTool (parse will fail otherwise)
748742
@Flag(name: [.customLong("list-tests"), .customShort("l")], help: .hidden)
749743
var _deprecated_passthrough: Bool = false
@@ -850,7 +844,7 @@ extension SwiftTestCommand {
850844
productsBuildParameters: productsBuildParameters,
851845
toolsBuildParameters: toolsBuildParameters,
852846
testProduct: self.sharedOptions.testProduct,
853-
traitConfiguration: .init(traitOptions: self.traits)
847+
traitConfiguration: .init(traitOptions: self.globalOptions.traits)
854848
)
855849
}
856850
}
@@ -1561,7 +1555,6 @@ private func buildTestsIfNeeded(
15611555
traitConfiguration: TraitConfiguration
15621556
) async throws -> [BuiltTestProduct] {
15631557
let buildSystem = try await swiftCommandState.createBuildSystem(
1564-
traitConfiguration: traitConfiguration,
15651558
productsBuildParameters: productsBuildParameters,
15661559
toolsBuildParameters: toolsBuildParameters
15671560
)

0 commit comments

Comments
 (0)