Skip to content

Commit c1ddf0c

Browse files
MaxDesiatovMaxDesiatov
committed
Allow module graph loading to become async
Making `packageGraphLoader` async unnecessarily colors all of the code using this graph as `async`. We should compute the graph earlier, which means we pass the already computed graph around than an `async` loader closure. This unblocks the conversion of PubGrub code to Structured Concurrency.
1 parent 4b9f96f commit c1ddf0c

File tree

13 files changed

+46
-75
lines changed

13 files changed

+46
-75
lines changed

‎Sources/Build/BuildOperation.swift

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
4646
/// Build parameters for build tools: plugins and macros.
4747
let toolsBuildParameters: BuildParameters
4848

49-
/// The closure for loading the package graph.
50-
let packageGraphLoader: () throws -> ModulesGraph
51-
5249
/// the plugin configuration for build plugins
5350
let pluginConfiguration: PluginConfiguration?
5451

@@ -79,7 +76,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
7976
private let buildDescription = ThreadSafeBox<BuildDescription>()
8077

8178
/// The loaded package graph.
82-
private let packageGraph = ThreadSafeBox<ModulesGraph>()
79+
public let modulesGraph: ModulesGraph
8380

8481
/// The output stream for the build delegate.
8582
private let outputStream: OutputByteStream
@@ -113,7 +110,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
113110
productsBuildParameters: BuildParameters,
114111
toolsBuildParameters: BuildParameters,
115112
cacheBuildManifest: Bool,
116-
packageGraphLoader: @escaping () throws -> ModulesGraph,
113+
modulesGraph: ModulesGraph,
117114
pluginConfiguration: PluginConfiguration? = .none,
118115
scratchDirectory: AbsolutePath,
119116
additionalFileRules: [FileRuleDescription],
@@ -135,7 +132,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
135132
self.productsBuildParameters = productsBuildParameters
136133
self.toolsBuildParameters = toolsBuildParameters
137134
self.cacheBuildManifest = cacheBuildManifest
138-
self.packageGraphLoader = packageGraphLoader
135+
self.modulesGraph = modulesGraph
139136
self.additionalFileRules = additionalFileRules
140137
self.pluginConfiguration = pluginConfiguration
141138
self.scratchDirectory = scratchDirectory
@@ -148,12 +145,6 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
148145
self.observabilityScope = observabilityScope.makeChildScope(description: "Build Operation")
149146
}
150147

151-
public func getPackageGraph() throws -> ModulesGraph {
152-
try self.packageGraph.memoize {
153-
try self.packageGraphLoader()
154-
}
155-
}
156-
157148
/// Compute and return the latest build description.
158149
///
159150
/// This will try skip build planning if build manifest caching is enabled
@@ -525,8 +516,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
525516
case .allIncludingTests:
526517
return LLBuildManifestBuilder.TargetKind.test.targetName
527518
case .product(let productName, let destination):
528-
// FIXME: This is super unfortunate that we might need to load the package graph.
529-
let graph = try getPackageGraph()
519+
let graph = self.modulesGraph
530520

531521
let buildTriple: BuildTriple? = if let destination {
532522
destination == .host ? .tools : .destination
@@ -561,8 +551,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
561551
}
562552
return try product.getLLBuildTargetName(buildParameters: buildParameters)
563553
case .target(let targetName, let destination):
564-
// FIXME: This is super unfortunate that we might need to load the package graph.
565-
let graph = try getPackageGraph()
554+
let graph = self.modulesGraph
566555

567556
let buildTriple: BuildTriple? = if let destination {
568557
destination == .host ? .tools : .destination
@@ -593,7 +582,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
593582
/// Create the build plan and return the build description.
594583
private func plan(subset: BuildSubset? = nil) throws -> (description: BuildDescription, manifest: LLBuildManifest) {
595584
// Load the package graph.
596-
let graph = try getPackageGraph()
585+
let graph = self.modulesGraph
597586
let buildToolPluginInvocationResults: [ResolvedModule.ID: (target: ResolvedModule, results: [BuildToolPluginInvocationResult])]
598587
let prebuildCommandResults: [ResolvedModule.ID: [PrebuildCommandResult]]
599588
// Invoke any build tool plugins in the graph to generate prebuild commands and build commands.
@@ -612,7 +601,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
612601
productsBuildParameters: targetBuildParameters,
613602
toolsBuildParameters: pluginsBuildParameters,
614603
cacheBuildManifest: false,
615-
packageGraphLoader: { graph },
604+
modulesGraph: graph,
616605
scratchDirectory: pluginsBuildParameters.dataPath,
617606
additionalFileRules: self.additionalFileRules,
618607
pkgConfigDirectories: self.pkgConfigDirectories,

‎Sources/Commands/PackageCommands/APIDiff.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,8 @@ struct APIDiff: SwiftCommand {
8585
// We turn build manifest caching off because we need the build plan.
8686
let buildSystem = try swiftCommandState.createBuildSystem(explicitBuildSystem: .native, cacheBuildManifest: false)
8787

88-
let packageGraph = try buildSystem.getPackageGraph()
8988
let modulesToDiff = try determineModulesToDiff(
90-
packageGraph: packageGraph,
89+
packageGraph: buildSystem.modulesGraph,
9190
observabilityScope: swiftCommandState.observabilityScope
9291
)
9392

‎Sources/Commands/PackageCommands/DumpCommands.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ struct DumpSymbolGraph: SwiftCommand {
6666
// Run the tool once for every library and executable target in the root package.
6767
let buildPlan = try buildSystem.buildPlan
6868
let symbolGraphDirectory = buildPlan.destinationBuildParameters.dataPath.appending("symbolgraph")
69-
let targets = try buildSystem.getPackageGraph().rootPackages.flatMap{ $0.targets }.filter{ $0.type == .library }
69+
let targets = buildSystem.modulesGraph.rootPackages.flatMap { $0.targets }.filter { $0.type == .library }
7070
for target in targets {
7171
print("-- Emitting symbol graph for", target.name)
7272
let result = try symbolGraphExtractor.extractSymbolGraph(

‎Sources/Commands/PackageCommands/PluginCommand.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ struct PluginCommand: SwiftCommand {
324324
cacheBuildManifest: false,
325325
productsBuildParameters: swiftCommandState.productsBuildParameters,
326326
toolsBuildParameters: buildParameters,
327-
packageGraphLoader: { packageGraph }
327+
modulesGraph: packageGraph
328328
)
329329

330330
let accessibleTools = try plugin.processAccessibleTools(

‎Sources/Commands/Snippets/Cards/SnippetCard.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ struct SnippetCard: Card {
9696
let buildSystem = try swiftCommandState.createBuildSystem(explicitProduct: snippet.name)
9797
try buildSystem.build(subset: .product(snippet.name))
9898
let executablePath = try swiftCommandState.productsBuildParameters.buildPath.appending(component: snippet.name)
99-
if let exampleTarget = try buildSystem.getPackageGraph().target(for: snippet.name, destination: .destination) {
99+
if let exampleTarget = buildSystem.modulesGraph.target(for: snippet.name, destination: .destination) {
100100
try ProcessEnv.chdir(exampleTarget.sources.paths[0].parentDirectory)
101101
}
102102
try exec(path: executablePath.pathString, args: [])

‎Sources/Commands/SwiftRunCommand.swift

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,18 +120,16 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
120120
switch options.mode {
121121
case .repl:
122122
// Load a custom package graph which has a special product for REPL.
123-
let graphLoader = {
124-
try swiftCommandState.loadPackageGraph(
125-
explicitProduct: self.options.executable
126-
)
127-
}
123+
let modulesGraph = try swiftCommandState.loadPackageGraph(
124+
explicitProduct: self.options.executable
125+
)
128126

129127
// Construct the build operation.
130128
// FIXME: We need to implement the build tool invocation closure here so that build tool plugins work with the REPL. rdar://86112934
131129
let buildSystem = try swiftCommandState.createBuildSystem(
132130
explicitBuildSystem: .native,
133131
cacheBuildManifest: false,
134-
packageGraphLoader: graphLoader
132+
modulesGraph: modulesGraph
135133
)
136134

137135
// Perform build.
@@ -150,7 +148,7 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
150148
case .debugger:
151149
do {
152150
let buildSystem = try swiftCommandState.createBuildSystem(explicitProduct: options.executable)
153-
let productName = try findProductName(in: buildSystem.getPackageGraph())
151+
let productName = try findProductName(in: buildSystem.modulesGraph)
154152
if options.shouldBuildTests {
155153
try buildSystem.build(subset: .allIncludingTests)
156154
} else if options.shouldBuild {
@@ -192,7 +190,7 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
192190

193191
do {
194192
let buildSystem = try swiftCommandState.createBuildSystem(explicitProduct: options.executable)
195-
let productName = try findProductName(in: buildSystem.getPackageGraph())
193+
let productName = try findProductName(in: buildSystem.modulesGraph)
196194
if options.shouldBuildTests {
197195
try buildSystem.build(subset: .allIncludingTests)
198196
} else if options.shouldBuild {

‎Sources/Commands/Utilities/APIDigester.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ struct APIDigesterBaselineDumper {
142142
cacheBuildManifest: false,
143143
productsBuildParameters: productsBuildParameters,
144144
toolsBuildParameters: toolsBuildParameters,
145-
packageGraphLoader: { graph }
145+
modulesGraph: graph
146146
)
147147
try buildSystem.build()
148148

‎Sources/Commands/Utilities/PluginDelegate.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,7 @@ final class PluginDelegate: PluginInvocationDelegate {
384384
let buildSystem = try swiftCommandState.createBuildSystem(explicitBuildSystem: .native, cacheBuildManifest: false)
385385

386386
// Find the target in the build operation's package graph; it's an error if we don't find it.
387-
let packageGraph = try buildSystem.getPackageGraph()
388-
guard let target = packageGraph.target(for: targetName) else {
387+
guard let target = buildSystem.modulesGraph.target(for: targetName) else {
389388
throw StringError("could not find a target named “\(targetName)”")
390389
}
391390

@@ -427,10 +426,10 @@ final class PluginDelegate: PluginInvocationDelegate {
427426
symbolGraphExtractor.emitExtensionBlockSymbols = options.emitExtensionBlocks
428427

429428
// Determine the output directory, and remove any old version if it already exists.
430-
guard let package = packageGraph.package(for: target) else {
429+
guard let package = buildSystem.modulesGraph.package(for: target) else {
431430
throw StringError("could not determine the package for target “\(target.name)”")
432431
}
433-
let outputDir = try buildParameters.dataPath.appending(
432+
let outputDir = buildParameters.dataPath.appending(
434433
components: "extracted-symbols",
435434
package.identity.description,
436435
target.name

‎Sources/CoreCommands/BuildSystemSupport.swift

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,22 @@ private struct NativeBuildSystemFactory: BuildSystemFactory {
2727
cacheBuildManifest: Bool,
2828
productsBuildParameters: BuildParameters?,
2929
toolsBuildParameters: BuildParameters?,
30-
packageGraphLoader: (() throws -> ModulesGraph)?,
30+
modulesGraph: ModulesGraph?,
3131
outputStream: OutputByteStream?,
3232
logLevel: Diagnostic.Severity?,
3333
observabilityScope: ObservabilityScope?
3434
) throws -> any BuildSystem {
3535
let rootPackageInfo = try swiftCommandState.getRootPackageInformation()
3636
let testEntryPointPath = productsBuildParameters?.testingParameters.testProductStyle.explicitlySpecifiedEntryPointPath
37+
let graph = try modulesGraph ?? self.swiftCommandState.loadPackageGraph(
38+
explicitProduct: explicitProduct,
39+
testEntryPointPath: testEntryPointPath
40+
)
3741
return try BuildOperation(
3842
productsBuildParameters: try productsBuildParameters ?? self.swiftCommandState.productsBuildParameters,
3943
toolsBuildParameters: try toolsBuildParameters ?? self.swiftCommandState.toolsBuildParameters,
4044
cacheBuildManifest: cacheBuildManifest && self.swiftCommandState.canUseCachedBuildManifest(),
41-
packageGraphLoader: packageGraphLoader ?? {
42-
try self.swiftCommandState.loadPackageGraph(
43-
explicitProduct: explicitProduct,
44-
testEntryPointPath: testEntryPointPath
45-
)
46-
},
45+
modulesGraph: graph,
4746
pluginConfiguration: .init(
4847
scriptRunner: self.swiftCommandState.getPluginScriptRunner(),
4948
workDirectory: try self.swiftCommandState.getActiveWorkspace().location.pluginWorkingDirectory,
@@ -69,18 +68,16 @@ private struct XcodeBuildSystemFactory: BuildSystemFactory {
6968
cacheBuildManifest: Bool,
7069
productsBuildParameters: BuildParameters?,
7170
toolsBuildParameters: BuildParameters?,
72-
packageGraphLoader: (() throws -> ModulesGraph)?,
71+
modulesGraph: ModulesGraph?,
7372
outputStream: OutputByteStream?,
7473
logLevel: Diagnostic.Severity?,
7574
observabilityScope: ObservabilityScope?
7675
) throws -> any BuildSystem {
7776
return try XcodeBuildSystem(
7877
buildParameters: productsBuildParameters ?? self.swiftCommandState.productsBuildParameters,
79-
packageGraphLoader: packageGraphLoader ?? {
80-
try self.swiftCommandState.loadPackageGraph(
81-
explicitProduct: explicitProduct
82-
)
83-
},
78+
modulesGraph: try modulesGraph ?? self.swiftCommandState.loadPackageGraph(
79+
explicitProduct: explicitProduct
80+
),
8481
outputStream: outputStream ?? self.swiftCommandState.outputStream,
8582
logLevel: logLevel ?? self.swiftCommandState.logLevel,
8683
fileSystem: self.swiftCommandState.fileSystem,

‎Sources/CoreCommands/SwiftCommandState.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,7 @@ public final class SwiftCommandState {
701701
shouldLinkStaticSwiftStdlib: Bool = false,
702702
productsBuildParameters: BuildParameters? = .none,
703703
toolsBuildParameters: BuildParameters? = .none,
704-
packageGraphLoader: (() throws -> ModulesGraph)? = .none,
704+
modulesGraph: ModulesGraph? = .none,
705705
outputStream: OutputByteStream? = .none,
706706
logLevel: Basics.Diagnostic.Severity? = .none,
707707
observabilityScope: ObservabilityScope? = .none
@@ -719,7 +719,7 @@ public final class SwiftCommandState {
719719
cacheBuildManifest: cacheBuildManifest,
720720
productsBuildParameters: productsParameters,
721721
toolsBuildParameters: toolsBuildParameters,
722-
packageGraphLoader: packageGraphLoader,
722+
modulesGraph: modulesGraph,
723723
outputStream: outputStream,
724724
logLevel: logLevel,
725725
observabilityScope: observabilityScope

‎Sources/SPMBuildCore/BuildSystem/BuildSystem.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public protocol BuildSystem: Cancellable {
4444
var builtTestProducts: [BuiltTestProduct] { get }
4545

4646
/// Returns the package graph used by the build system.
47-
func getPackageGraph() throws -> ModulesGraph
47+
var modulesGraph: ModulesGraph { get }
4848

4949
/// Builds a subset of the package graph.
5050
/// - Parameters:
@@ -104,7 +104,7 @@ public protocol BuildSystemFactory {
104104
cacheBuildManifest: Bool,
105105
productsBuildParameters: BuildParameters?,
106106
toolsBuildParameters: BuildParameters?,
107-
packageGraphLoader: (() throws -> ModulesGraph)?,
107+
modulesGraph: ModulesGraph?,
108108
outputStream: OutputByteStream?,
109109
logLevel: Diagnostic.Severity?,
110110
observabilityScope: ObservabilityScope?
@@ -130,7 +130,7 @@ public struct BuildSystemProvider {
130130
cacheBuildManifest: Bool = true,
131131
productsBuildParameters: BuildParameters? = .none,
132132
toolsBuildParameters: BuildParameters? = .none,
133-
packageGraphLoader: (() throws -> ModulesGraph)? = .none,
133+
modulesGraph: ModulesGraph? = .none,
134134
outputStream: OutputByteStream? = .none,
135135
logLevel: Diagnostic.Severity? = .none,
136136
observabilityScope: ObservabilityScope? = .none
@@ -143,7 +143,7 @@ public struct BuildSystemProvider {
143143
cacheBuildManifest: cacheBuildManifest,
144144
productsBuildParameters: productsBuildParameters,
145145
toolsBuildParameters: toolsBuildParameters,
146-
packageGraphLoader: packageGraphLoader,
146+
modulesGraph: modulesGraph,
147147
outputStream: outputStream,
148148
logLevel: logLevel,
149149
observabilityScope: observabilityScope

‎Sources/XCBuildSupport/XcodeBuildSystem.swift

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ import enum TSCUtility.Diagnostics
3232

3333
public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
3434
private let buildParameters: BuildParameters
35-
private let packageGraphLoader: () throws -> ModulesGraph
3635
private let logLevel: Basics.Diagnostic.Severity
3736
private let xcbuildPath: AbsolutePath
38-
private var packageGraph: ModulesGraph?
3937
private var pifBuilder: PIFBuilder?
4038
private let fileSystem: FileSystem
4139
private let observabilityScope: ObservabilityScope
@@ -46,9 +44,11 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
4644
/// The delegate used by the build system.
4745
public weak var delegate: SPMBuildCore.BuildSystemDelegate?
4846

47+
public let modulesGraph: ModulesGraph
48+
4949
public var builtTestProducts: [BuiltTestProduct] {
5050
do {
51-
let graph = try getPackageGraph()
51+
let graph = self.modulesGraph
5252

5353
var builtProducts: [BuiltTestProduct] = []
5454

@@ -81,14 +81,14 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
8181

8282
public init(
8383
buildParameters: BuildParameters,
84-
packageGraphLoader: @escaping () throws -> ModulesGraph,
84+
modulesGraph: ModulesGraph,
8585
outputStream: OutputByteStream,
8686
logLevel: Basics.Diagnostic.Severity,
8787
fileSystem: FileSystem,
8888
observabilityScope: ObservabilityScope
8989
) throws {
9090
self.buildParameters = buildParameters
91-
self.packageGraphLoader = packageGraphLoader
91+
self.modulesGraph = modulesGraph
9292
self.outputStream = outputStream
9393
self.logLevel = logLevel
9494
self.fileSystem = fileSystem
@@ -312,7 +312,7 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
312312

313313
private func getPIFBuilder() throws -> PIFBuilder {
314314
try memoize(to: &pifBuilder) {
315-
let graph = try getPackageGraph()
315+
let graph = self.modulesGraph
316316
let pifBuilder = try PIFBuilder(
317317
graph: graph,
318318
parameters: .init(buildParameters, supportedSwiftVersions: supportedSwiftVersions()),
@@ -322,15 +322,6 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
322322
return pifBuilder
323323
}
324324
}
325-
326-
/// Returns the package graph using the graph loader closure.
327-
///
328-
/// First access will cache the graph.
329-
public func getPackageGraph() throws -> ModulesGraph {
330-
try memoize(to: &packageGraph) {
331-
try packageGraphLoader()
332-
}
333-
}
334325
}
335326

336327
struct XCBBuildParameters: Encodable {

0 commit comments

Comments
 (0)