Skip to content

Commit 41a559a

Browse files
xedinxedin
committed
[Build] BuildPlan: Start using id from module/product build descriptions for build plan
Instead of using `Resolved{Product, Module}.ID` to storage module and product collections in the build plan, switch to use newly added `Identifiable` conformation on `{Product, Module}BuildDescription`. This is yet another step (if not the last one) on the path to remove `buildTriple` from `Resolved{Product, Module}` identifiers.
1 parent 115c82d commit 41a559a

File tree

8 files changed

+82
-64
lines changed

8 files changed

+82
-64
lines changed

‎Sources/Basics/Collections/IdentifiableSet.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ public struct IdentifiableSet<Element: Identifiable>: Collection {
4242
Index(storageIndex: self.storage.elements.endIndex)
4343
}
4444

45+
public var values: some Sequence<Element> {
46+
self.storage.values
47+
}
48+
4549
public subscript(position: Index) -> Element {
4650
self.storage.elements[position.storageIndex].value
4751
}

‎Sources/Build/BuildManifest/LLBuildManifestBuilder.swift

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public class LLBuildManifestBuilder {
102102
try addTargetsToExplicitBuildManifest()
103103
} else {
104104
// Create commands for all target descriptions in the plan.
105-
for (_, description) in self.plan.targetMap {
105+
for description in self.plan.targetMap {
106106
switch description {
107107
case .swift(let desc):
108108
try self.createSwiftCompileCommand(desc)
@@ -116,7 +116,7 @@ public class LLBuildManifestBuilder {
116116
try self.addTestEntryPointGenerationCommand()
117117

118118
// Create command for all products in the plan.
119-
for (_, description) in self.plan.productMap {
119+
for description in self.plan.productMap {
120120
try self.createProductCommand(description)
121121
}
122122

@@ -133,7 +133,7 @@ public class LLBuildManifestBuilder {
133133

134134
addPackageStructureCommand()
135135

136-
for (_, description) in self.plan.targetMap {
136+
for description in self.plan.targetMap {
137137
switch description {
138138
case .swift(let desc):
139139
try self.createSwiftCompileCommand(desc)
@@ -148,7 +148,7 @@ public class LLBuildManifestBuilder {
148148
}
149149
}
150150

151-
for (_, description) in self.plan.productMap {
151+
for description in self.plan.productMap {
152152
// Need to generate macro products
153153
switch description.product.type {
154154
case .macro, .plugin:
@@ -271,7 +271,7 @@ extension LLBuildManifestBuilder {
271271
private func addTestDiscoveryGenerationCommand() throws {
272272
for testDiscoveryTarget in self.plan.targets.compactMap(\.testDiscoveryTargetBuildDescription) {
273273
let testTargets = testDiscoveryTarget.target.dependencies
274-
.compactMap(\.module).compactMap { self.plan.targetMap[$0.id] }
274+
.compactMap(\.module).compactMap { self.plan.description(for: $0, context: testDiscoveryTarget.destination) }
275275
let objectFiles = try testTargets.flatMap { try $0.objects }.sorted().map(Node.file)
276276
let outputs = testDiscoveryTarget.target.sources.paths
277277

@@ -288,18 +288,22 @@ extension LLBuildManifestBuilder {
288288
}
289289

290290
private func addTestEntryPointGenerationCommand() throws {
291-
for target in self.plan.targets {
292-
guard case .swift(let target) = target,
293-
case .entryPoint(let isSynthesized) = target.testTargetRole,
291+
for module in self.plan.targets {
292+
guard case .swift(let swiftModule) = module,
293+
case .entryPoint(let isSynthesized) = swiftModule.testTargetRole,
294294
isSynthesized else { continue }
295295

296-
let testEntryPointTarget = target
296+
let testEntryPointTarget = swiftModule
297297

298298
// Get the Swift target build descriptions of all discovery modules this synthesized entry point target
299299
// depends on.
300-
let discoveredTargetDependencyBuildDescriptions = testEntryPointTarget.target.dependencies
301-
.compactMap(\.module?.id)
302-
.compactMap { self.plan.targetMap[$0] }
300+
let discoveredTargetDependencyBuildDescriptions = module.dependencies(using: self.plan)
301+
.compactMap {
302+
if case .module(_, let description) = $0 {
303+
return description
304+
}
305+
return nil
306+
}
303307
.compactMap(\.testDiscoveryTargetBuildDescription)
304308

305309
// The module outputs of the discovery modules this synthesized entry point target depends on are

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ extension BuildPlan {
7777
}
7878
}
7979

80-
for target in dependencies.staticTargets {
81-
switch target.underlying {
80+
for module in dependencies.staticTargets {
81+
switch module.underlying {
8282
case is SwiftModule:
8383
// Swift targets are guaranteed to have a corresponding Swift description.
84-
guard case .swift(let description) = self.targetMap[target.id] else {
85-
throw InternalError("unknown target \(target)")
84+
guard case .swift(let description) = self.description(for: module, context: buildProduct.destination) else {
85+
throw InternalError("unknown module \(module)")
8686
}
8787

8888
// Based on the debugging strategy, we either need to pass swiftmodule paths to the
@@ -103,16 +103,16 @@ extension BuildPlan {
103103

104104
buildProduct.staticTargets = dependencies.staticTargets
105105
buildProduct.dylibs = try dependencies.dylibs.map {
106-
guard let product = self.productMap[$0.id] else {
106+
guard let product = self.description(for: $0, context: buildProduct.destination) else {
107107
throw InternalError("unknown product \($0)")
108108
}
109109
return product
110110
}
111-
buildProduct.objects += try dependencies.staticTargets.flatMap { targetName -> [AbsolutePath] in
112-
guard let target = self.targetMap[targetName.id] else {
113-
throw InternalError("unknown target \(targetName)")
111+
buildProduct.objects += try dependencies.staticTargets.flatMap { module -> [AbsolutePath] in
112+
guard let description = self.description(for: module, context: buildProduct.destination) else {
113+
throw InternalError("unknown module \(module)")
114114
}
115-
return try target.objects
115+
return try description.objects
116116
}
117117
buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths
118118

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import protocol TSCBasic.FileSystem
3434

3535
extension BuildPlan {
3636
static func makeDerivedTestTargets(
37-
testProducts: [(product: ResolvedProduct, buildDescription: ProductBuildDescription)],
37+
testProducts: [ProductBuildDescription],
3838
destinationBuildParameters: BuildParameters,
3939
toolsBuildParameters: BuildParameters,
4040
shouldDisableSandbox: Bool,
@@ -51,7 +51,8 @@ extension BuildPlan {
5151

5252
var isDiscoveryEnabledRedundantly = explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly
5353
var result: [(ResolvedProduct, SwiftModuleBuildDescription?, SwiftModuleBuildDescription)] = []
54-
for (testProduct, testBuildDescription) in testProducts {
54+
for testBuildDescription in testProducts {
55+
let testProduct = testBuildDescription.product
5556
let package = testBuildDescription.package
5657

5758
isDiscoveryEnabledRedundantly = isDiscoveryEnabledRedundantly && nil == testProduct.testEntryPointModule

‎Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,10 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
197197
public let graph: ModulesGraph
198198

199199
/// The target build description map.
200-
public let targetMap: [ResolvedModule.ID: ModuleBuildDescription]
200+
public let targetMap: IdentifiableSet<ModuleBuildDescription>
201201

202202
/// The product build description map.
203-
public let productMap: [ResolvedProduct.ID: ProductBuildDescription]
203+
public let productMap: IdentifiableSet<ProductBuildDescription>
204204

205205
/// The plugin descriptions. Plugins are represented in the package graph
206206
/// as targets, but they are not directly included in the build graph.
@@ -290,13 +290,12 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
290290
var prebuildCommandResults: [ResolvedModule.ID: [CommandPluginResult]] = [:]
291291

292292
// Create product description for each product we have in the package graph that is eligible.
293-
var productMap: [ResolvedProduct.ID: (product: ResolvedProduct, buildDescription: ProductBuildDescription)] =
294-
[:]
293+
var productMap = IdentifiableSet<ProductBuildDescription>()
295294
// Create build target description for each target which we need to plan.
296295
// Plugin targets are noted, since they need to be compiled, but they do
297296
// not get directly incorporated into the build description that will be
298297
// given to LLBuild.
299-
var targetMap = [ResolvedModule.ID: ModuleBuildDescription]()
298+
var targetMap = IdentifiableSet<ModuleBuildDescription>()
300299
var pluginDescriptions = [PluginBuildDescription]()
301300
var shouldGenerateTestObservation = true
302301

@@ -312,7 +311,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
312311
throw InternalError("Package not found for product: \(product.name)")
313312
}
314313

315-
productMap[product.id] = try (product, ProductBuildDescription(
314+
try productMap.insert(ProductBuildDescription(
316315
package: package,
317316
product: product,
318317
toolsVersion: package.manifest.toolsVersion,
@@ -384,7 +383,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
384383
shouldGenerateTestObservation = false // Only generate the code once.
385384
}
386385

387-
targetMap[module.id] = try .swift(
386+
try targetMap.insert(.swift(
388387
SwiftModuleBuildDescription(
389388
package: package,
390389
target: module,
@@ -399,9 +398,9 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
399398
fileSystem: fileSystem,
400399
observabilityScope: planningObservabilityScope
401400
)
402-
)
401+
))
403402
case is ClangModule:
404-
targetMap[module.id] = try .clang(
403+
try targetMap.insert(.clang(
405404
ClangModuleBuildDescription(
406405
package: package,
407406
target: module,
@@ -413,7 +412,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
413412
fileSystem: fileSystem,
414413
observabilityScope: planningObservabilityScope
415414
)
416-
)
415+
))
417416
case is PluginModule:
418417
try module.dependencies.compactMap {
419418
switch $0 {
@@ -429,7 +428,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
429428
return nil
430429
}
431430
}.forEach {
432-
productMap[$0.id] = try ($0, ProductBuildDescription(
431+
try productMap.insert(ProductBuildDescription(
433432
package: package,
434433
product: $0,
435434
toolsVersion: package.manifest.toolsVersion,
@@ -468,7 +467,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
468467

469468
// Plan the derived test targets, if necessary.
470469
let derivedTestTargets = try Self.makeDerivedTestTargets(
471-
testProducts: productMap.values.filter {
470+
testProducts: productMap.filter {
472471
$0.product.type == .test
473472
},
474473
destinationBuildParameters: destinationBuildParameters,
@@ -480,12 +479,12 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
480479
for item in derivedTestTargets {
481480
var derivedTestTargets = [item.entryPointTargetBuildDescription.target]
482481

483-
targetMap[item.entryPointTargetBuildDescription.target.id] = .swift(
482+
targetMap.insert(.swift(
484483
item.entryPointTargetBuildDescription
485-
)
484+
))
486485

487486
if let discoveryTargetBuildDescription = item.discoveryTargetBuildDescription {
488-
targetMap[discoveryTargetBuildDescription.target.id] = .swift(discoveryTargetBuildDescription)
487+
targetMap.insert(.swift(discoveryTargetBuildDescription))
489488
derivedTestTargets.append(discoveryTargetBuildDescription.target)
490489
}
491490

@@ -495,7 +494,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
495494
self.buildToolPluginInvocationResults = buildToolPluginInvocationResults
496495
self.prebuildCommandResults = prebuildCommandResults
497496

498-
self.productMap = productMap.mapValues(\.buildDescription)
497+
self.productMap = productMap
499498
self.targetMap = targetMap
500499
self.pluginDescriptions = pluginDescriptions
501500

@@ -711,14 +710,28 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
711710
for product: ResolvedProduct,
712711
context: BuildParameters.Destination
713712
) -> ProductBuildDescription? {
714-
return self.productMap[product.id]
713+
let destination: BuildParameters.Destination = switch product.type {
714+
case .macro, .plugin:
715+
.host
716+
default:
717+
context
718+
}
719+
720+
return self.productMap[.init(productID: product.id, destination: destination)]
715721
}
716722

717723
public func description(
718724
for module: ResolvedModule,
719725
context: BuildParameters.Destination
720726
) -> ModuleBuildDescription? {
721-
return self.targetMap[module.id]
727+
let destination: BuildParameters.Destination = switch module.type {
728+
case .macro, .plugin:
729+
.host
730+
default:
731+
context
732+
}
733+
734+
return self.targetMap[.init(moduleID: module.id, destination: destination)]
722735
}
723736
}
724737

‎Sources/SourceKitLSPAPI/BuildDescription.swift

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import struct Foundation.URL
1515
private import struct Basics.AbsolutePath
1616
private import func Basics.resolveSymlinks
1717

18-
private import SPMBuildCore
18+
internal import SPMBuildCore
1919

2020
// FIXME: should import these module with `private` or `internal` access control
2121
import class Build.BuildPlan
@@ -127,9 +127,12 @@ public struct BuildDescription {
127127
self.inputs = buildPlan.inputs
128128
}
129129

130-
// FIXME: should not use `ResolvedTarget` in the public interface
131-
public func getBuildTarget(for target: ResolvedModule, in modulesGraph: ModulesGraph) -> BuildTarget? {
132-
if let description = buildPlan.targetMap[target.id] {
130+
func getBuildTarget(
131+
for module: ResolvedModule,
132+
destination: BuildParameters.Destination
133+
) -> BuildTarget? {
134+
if let description = self.buildPlan.description(for: module, context: destination) {
135+
let modulesGraph = self.buildPlan.graph
133136
switch description {
134137
case .clang(let description):
135138
return WrappedClangTargetBuildDescription(
@@ -143,9 +146,10 @@ public struct BuildDescription {
143146
)
144147
}
145148
} else {
146-
if target.type == .plugin, let package = self.buildPlan.graph.package(for: target) {
149+
if module.type == .plugin, let package = self.buildPlan.graph.package(for: module) {
150+
let modulesGraph = self.buildPlan.graph
147151
return PluginTargetBuildDescription(
148-
target: target,
152+
target: module,
149153
toolsVersion: package.manifest.toolsVersion,
150154
toolchain: buildPlan.toolsBuildParameters.toolchain,
151155
isPartOfRootPackage: modulesGraph.rootPackages.map(\.id).contains(package.id)
@@ -158,17 +162,14 @@ public struct BuildDescription {
158162
public func traverseModules(
159163
callback: (any BuildTarget, _ parent: (any BuildTarget)?, _ depth: Int) -> Void
160164
) {
161-
// TODO: Once the `targetMap` is switched over to use `IdentifiableSet<ModuleBuildDescription>`
162-
// we can introduce `BuildPlan.description(ResolvedModule, BuildParameters.Destination)`
163-
// and start using that here.
164165
self.buildPlan.traverseModules { module, parent, depth in
165166
let parentDescription: (any BuildTarget)? = if let parent {
166-
getBuildTarget(for: parent.0, in: self.buildPlan.graph)
167+
getBuildTarget(for: parent.0, destination: parent.1)
167168
} else {
168169
nil
169170
}
170171

171-
if let description = getBuildTarget(for: module.0, in: self.buildPlan.graph) {
172+
if let description = getBuildTarget(for: module.0, destination: module.1) {
172173
callback(description, parentDescription, depth)
173174
}
174175
}

‎Sources/_InternalTestSupport/MockBuildTestHelper.swift

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,7 @@ public struct BuildPlanResult {
297297
)
298298
self.targetMap = try Dictionary(
299299
throwingUniqueKeysWithValues: plan.targetMap.compactMap {
300-
guard
301-
let target = plan.graph.allModules[$0] ??
302-
IdentifiableSet(plan.derivedTestTargetsMap.values.flatMap { $0 })[$0]
303-
else {
304-
throw BuildError.error("Target \($0) not found.")
305-
}
306-
return (target.id, $1)
300+
($0.module.id, $0)
307301
}
308302
)
309303
}

‎Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ import Build
1717
import PackageGraph
1818

1919
import PackageModel
20-
import SourceKitLSPAPI
20+
@testable import SourceKitLSPAPI
21+
import struct SPMBuildCore.BuildParameters
2122
import _InternalTestSupport
2223
import XCTest
2324

@@ -90,7 +91,7 @@ final class SourceKitLSPAPITests: XCTestCase {
9091
"-I", "/fake/manifestLib/path"
9192
],
9293
isPartOfRootPackage: true,
93-
destination: .tools
94+
destination: .host
9495
)
9596
}
9697

@@ -167,10 +168,10 @@ extension SourceKitLSPAPI.BuildDescription {
167168
graph: ModulesGraph,
168169
partialArguments: [String],
169170
isPartOfRootPackage: Bool,
170-
destination: BuildTriple = .destination
171+
destination: BuildParameters.Destination = .target
171172
) throws -> Bool {
172-
let target = try XCTUnwrap(graph.module(for: targetName, destination: destination))
173-
let buildTarget = try XCTUnwrap(self.getBuildTarget(for: target, in: graph))
173+
let target = try XCTUnwrap(graph.module(for: targetName))
174+
let buildTarget = try XCTUnwrap(self.getBuildTarget(for: target, destination: destination))
174175

175176
guard let file = buildTarget.sources.first else {
176177
XCTFail("build target \(targetName) contains no files")

0 commit comments

Comments
 (0)