Skip to content

Commit 520d11a

Browse files
xedinxedin
authored
[Build] NFC: Remove uses of buildTriple from computeDependencies(of:) (#7888)
### Motivation: Now that we have all the tools to compute destinations for dependencies of a product, we no longer need `buildTriple` here and dynamic/static library results could be represented by build descriptions as a bonus. ### Modifications: - `BuildPlan.computeDependencies(of:)` has been updated to take a `ProductBuildDescription` and use depth first scan that computes destination; - Further, dynamic and static library dependencies are now presented by build descriptions which means that we no longer need to look stuff up while planning. ### Result: One more use of `buildTriple` is gone.
1 parent 9456d16 commit 520d11a

File tree

2 files changed

+151
-98
lines changed

2 files changed

+151
-98
lines changed

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

Lines changed: 150 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
import struct Basics.AbsolutePath
14-
import struct Basics.Triple
14+
import func Basics.depthFirstSearch
1515
import struct Basics.InternalError
16-
import struct PackageGraph.ResolvedProduct
16+
import struct Basics.Triple
1717
import struct PackageGraph.ResolvedModule
18+
import struct PackageGraph.ResolvedProduct
1819
import class PackageModel.BinaryModule
1920
import class PackageModel.ClangModule
2021

@@ -31,10 +32,7 @@ extension BuildPlan {
3132
/// Plan a product.
3233
func plan(buildProduct: ProductBuildDescription) throws {
3334
// Compute the product's dependency.
34-
let dependencies = try computeDependencies(
35-
of: buildProduct.product,
36-
buildParameters: buildProduct.buildParameters
37-
)
35+
let dependencies = try computeDependencies(of: buildProduct)
3836

3937
// Add flags for system targets.
4038
for systemModule in dependencies.systemModules {
@@ -62,8 +60,8 @@ extension BuildPlan {
6260
if !buildProduct.product.modules.contains(where: \.underlying.isEmbeddedSwiftTarget) {
6361
// Link C++ if needed.
6462
// Note: This will come from build settings in future.
65-
for target in dependencies.staticTargets {
66-
if case let target as ClangModule = target.underlying, target.isCXX {
63+
for description in dependencies.staticTargets {
64+
if case let target as ClangModule = description.module.underlying, target.isCXX {
6765
let triple = buildProduct.buildParameters.triple
6866
if triple.isDarwin() {
6967
buildProduct.additionalFlags += ["-lc++"]
@@ -77,12 +75,12 @@ extension BuildPlan {
7775
}
7876
}
7977

80-
for module in dependencies.staticTargets {
81-
switch module.underlying {
78+
for description in dependencies.staticTargets {
79+
switch description.module.underlying {
8280
case is SwiftModule:
8381
// Swift targets are guaranteed to have a corresponding Swift description.
84-
guard case .swift(let description) = self.description(for: module, context: buildProduct.destination) else {
85-
throw InternalError("unknown module \(module)")
82+
guard case .swift(let description) = description else {
83+
throw InternalError("Expected a Swift module: \(description.module)")
8684
}
8785

8886
// Based on the debugging strategy, we either need to pass swiftmodule paths to the
@@ -101,176 +99,231 @@ extension BuildPlan {
10199
}
102100
}
103101

104-
buildProduct.staticTargets = dependencies.staticTargets
105-
buildProduct.dylibs = try dependencies.dylibs.map {
106-
guard let product = self.description(for: $0, context: buildProduct.destination) else {
107-
throw InternalError("unknown product \($0)")
108-
}
109-
return product
110-
}
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)")
114-
}
115-
return try description.objects
116-
}
102+
buildProduct.staticTargets = dependencies.staticTargets.map(\.module)
103+
buildProduct.dylibs = dependencies.dylibs
104+
buildProduct.objects += try dependencies.staticTargets.flatMap { try $0.objects }
117105
buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths
118-
119106
buildProduct.availableTools = dependencies.availableTools
120107
}
121108

122109
/// Computes the dependencies of a product.
123110
private func computeDependencies(
124-
of product: ResolvedProduct,
125-
buildParameters: BuildParameters
111+
of productDescription: ProductBuildDescription
126112
) throws -> (
127-
dylibs: [ResolvedProduct],
128-
staticTargets: [ResolvedModule],
113+
dylibs: [ProductBuildDescription],
114+
staticTargets: [ModuleBuildDescription],
129115
systemModules: [ResolvedModule],
130116
libraryBinaryPaths: Set<AbsolutePath>,
131117
availableTools: [String: AbsolutePath]
132118
) {
119+
let product = productDescription.product
133120
/* Prior to tools-version 5.9, we used to erroneously recursively traverse executable/plugin dependencies and statically include their
134121
targets. For compatibility reasons, we preserve that behavior for older tools-versions. */
135-
let shouldExcludePlugins: Bool
136-
if let toolsVersion = self.graph.package(for: product)?.manifest.toolsVersion {
137-
shouldExcludePlugins = toolsVersion >= .v5_9
138-
} else {
139-
shouldExcludePlugins = false
140-
}
122+
let shouldExcludePlugins = productDescription.package.manifest.toolsVersion >= .v5_9
141123

142-
// For test targets, we need to consider the first level of transitive dependencies since the first level is always test targets.
143-
let topLevelDependencies: [PackageModel.Module]
144-
if product.type == .test {
145-
topLevelDependencies = product.modules.flatMap { $0.underlying.dependencies }.compactMap {
124+
// For test targets, we need to consider the first level of transitive dependencies since the first level is
125+
// always test targets.
126+
let topLevelDependencies: [PackageModel.Module] = if product.type == .test {
127+
product.modules.flatMap(\.underlying.dependencies).compactMap {
146128
switch $0 {
147129
case .product:
148-
return nil
130+
nil
149131
case .module(let target, _):
150-
return target
132+
target
151133
}
152134
}
153135
} else {
154-
topLevelDependencies = []
136+
[]
155137
}
156138

157139
// get the dynamic libraries for explicitly linking rdar://108561857
158-
func recursiveDynamicLibraries(for product: ResolvedProduct) throws -> [ResolvedProduct] {
159-
let dylibs = try computeDependencies(of: product, buildParameters: buildParameters).dylibs
140+
func recursiveDynamicLibraries(for description: ProductBuildDescription) throws -> [ProductBuildDescription] {
141+
let dylibs = try computeDependencies(of: description).dylibs
160142
return try dylibs + dylibs.flatMap { try recursiveDynamicLibraries(for: $0) }
161143
}
162144

163145
// Sort the product targets in topological order.
164-
let nodes: [ResolvedModule.Dependency] = product.modules.map { .module($0, conditions: []) }
165-
let allTargets = try topologicalSort(nodes, successors: { dependency in
166-
switch dependency {
167-
// Include all the dependencies of a target.
168-
case .module(let target, _):
169-
let isTopLevel = topLevelDependencies.contains(target.underlying) || product.modules.contains(id: target.id)
170-
let topLevelIsMacro = isTopLevel && product.type == .macro
171-
let topLevelIsPlugin = isTopLevel && product.type == .plugin
172-
let topLevelIsTest = isTopLevel && product.type == .test
146+
var allDependencies: [ModuleBuildDescription.Dependency] = []
173147

174-
if !topLevelIsMacro && !topLevelIsTest && target.type == .macro {
175-
return []
176-
}
177-
if shouldExcludePlugins, !topLevelIsPlugin && !topLevelIsTest && target.type == .plugin {
178-
return []
148+
do {
149+
func successors(
150+
for product: ResolvedProduct,
151+
destination: BuildParameters.Destination
152+
) throws -> [TraversalNode] {
153+
let productDependencies: [TraversalNode] = product.modules.map {
154+
.init(module: $0, context: destination)
179155
}
180-
return target.dependencies.filter { $0.satisfies(buildParameters.buildEnvironment) }
181156

182-
// For a product dependency, we only include its content only if we
183-
// need to statically link it.
184-
case .product(let product, _):
185-
guard dependency.satisfies(buildParameters.buildEnvironment) else {
186-
return []
187-
}
188-
189-
let productDependencies: [ResolvedModule.Dependency] = product.modules.map { .module($0, conditions: []) }
190157
switch product.type {
191158
case .library(.automatic), .library(.static):
192159
return productDependencies
193160
case .plugin:
194161
return shouldExcludePlugins ? [] : productDependencies
195162
case .library(.dynamic):
196-
return try recursiveDynamicLibraries(for: product).map { .product($0, conditions: []) }
163+
guard let description = self.description(for: product, context: destination) else {
164+
throw InternalError("Could not find a description for product: \(product)")
165+
}
166+
return try recursiveDynamicLibraries(for: description).map { TraversalNode(
167+
product: $0.product,
168+
context: $0.destination
169+
) }
197170
case .test, .executable, .snippet, .macro:
198171
return []
199172
}
200173
}
201-
})
174+
175+
func successors(
176+
for module: ResolvedModule,
177+
destination: BuildParameters.Destination
178+
) -> [TraversalNode] {
179+
let isTopLevel = topLevelDependencies.contains(module.underlying) || product.modules
180+
.contains(id: module.id)
181+
let topLevelIsMacro = isTopLevel && product.type == .macro
182+
let topLevelIsPlugin = isTopLevel && product.type == .plugin
183+
let topLevelIsTest = isTopLevel && product.type == .test
184+
185+
if !topLevelIsMacro && !topLevelIsTest && module.type == .macro {
186+
return []
187+
}
188+
if shouldExcludePlugins, !topLevelIsPlugin && !topLevelIsTest && module.type == .plugin {
189+
return []
190+
}
191+
return module.dependencies(satisfying: productDescription.buildParameters.buildEnvironment)
192+
.map {
193+
switch $0 {
194+
case .product(let product, _):
195+
.init(product: product, context: destination)
196+
case .module(let module, _):
197+
.init(module: module, context: destination)
198+
}
199+
}
200+
}
201+
202+
let directDependencies = product.modules
203+
.map { TraversalNode(module: $0, context: productDescription.destination) }
204+
205+
var uniqueNodes = Set<TraversalNode>(directDependencies)
206+
207+
try depthFirstSearch(directDependencies) {
208+
let result: [TraversalNode] = switch $0 {
209+
case .product(let product, let destination):
210+
try successors(for: product, destination: destination)
211+
case .module(let module, let destination):
212+
successors(for: module, destination: destination)
213+
case .package:
214+
[]
215+
}
216+
217+
return result.filter { uniqueNodes.insert($0).inserted }
218+
} onNext: { node, _, _ in
219+
switch node {
220+
case .package: break
221+
case .product(let product, let destination):
222+
allDependencies.append(.product(product, self.description(for: product, context: destination)))
223+
case .module(let module, let destination):
224+
allDependencies.append(.module(module, self.description(for: module, context: destination)))
225+
}
226+
}
227+
}
202228

203229
// Create empty arrays to collect our results.
204-
var linkLibraries = [ResolvedProduct]()
205-
var staticTargets = [ResolvedModule]()
230+
var linkLibraries = [ProductBuildDescription]()
231+
var staticTargets = [ModuleBuildDescription]()
206232
var systemModules = [ResolvedModule]()
207233
var libraryBinaryPaths: Set<AbsolutePath> = []
208234
var availableTools = [String: AbsolutePath]()
209235

210-
for dependency in allTargets {
236+
for dependency in allDependencies {
211237
switch dependency {
212-
case .module(let target, _):
213-
switch target.type {
238+
case .module(let module, let description):
239+
switch module.type {
214240
// Executable target have historically only been included if they are directly in the product's
215241
// target list. Otherwise they have always been just build-time dependencies.
216242
// In tool version .v5_5 or greater, we also include executable modules implemented in Swift in
217243
// any test products... this is to allow testing of executables. Note that they are also still
218244
// built as separate products that the test can invoke as subprocesses.
219245
case .executable, .snippet, .macro:
220-
if product.modules.contains(id: target.id) {
221-
staticTargets.append(target)
222-
} else if product.type == .test && (target.underlying as? SwiftModule)?.supportsTestableExecutablesFeature == true {
246+
if product.modules.contains(id: module.id) {
247+
guard let description else {
248+
throw InternalError("Could not find a description for module: \(module)")
249+
}
250+
staticTargets.append(description)
251+
} else if product.type == .test && (module.underlying as? SwiftModule)?
252+
.supportsTestableExecutablesFeature == true
253+
{
223254
// Only "top-level" targets should really be considered here, not transitive ones.
224-
let isTopLevel = topLevelDependencies.contains(target.underlying) || product.modules.contains(id: target.id)
225-
if let toolsVersion = graph.package(for: product)?.manifest.toolsVersion, toolsVersion >= .v5_5, isTopLevel {
226-
staticTargets.append(target)
255+
let isTopLevel = topLevelDependencies.contains(module.underlying) || product.modules
256+
.contains(id: module.id)
257+
if let toolsVersion = graph.package(for: product)?.manifest.toolsVersion, toolsVersion >= .v5_5,
258+
isTopLevel
259+
{
260+
guard let description else {
261+
throw InternalError("Could not find a description for module: \(module)")
262+
}
263+
staticTargets.append(description)
227264
}
228265
}
229266
// Test targets should be included only if they are directly in the product's target list.
230267
case .test:
231-
if product.modules.contains(id: target.id) {
232-
staticTargets.append(target)
268+
if product.modules.contains(id: module.id) {
269+
guard let description else {
270+
throw InternalError("Could not find a description for module: \(module)")
271+
}
272+
staticTargets.append(description)
233273
}
234274
// Library targets should always be included for the same build triple.
235275
case .library:
236-
if target.buildTriple == product.buildTriple {
237-
staticTargets.append(target)
276+
guard let description else {
277+
throw InternalError("Could not find a description for module: \(module)")
278+
}
279+
if description.destination == productDescription.destination {
280+
staticTargets.append(description)
238281
}
239282
// Add system target to system targets array.
240283
case .systemModule:
241-
systemModules.append(target)
284+
systemModules.append(module)
242285
// Add binary to binary paths set.
243286
case .binary:
244-
guard let binaryTarget = target.underlying as? BinaryModule else {
245-
throw InternalError("invalid binary target '\(target.name)'")
287+
guard let binaryTarget = module.underlying as? BinaryModule else {
288+
throw InternalError("invalid binary target '\(module.name)'")
246289
}
247290
switch binaryTarget.kind {
248291
case .xcframework:
249-
let libraries = try self.parseXCFramework(for: binaryTarget, triple: buildParameters.triple)
292+
let libraries = try self.parseXCFramework(
293+
for: binaryTarget,
294+
triple: productDescription.buildParameters.triple
295+
)
250296
for library in libraries {
251297
libraryBinaryPaths.insert(library.libraryPath)
252298
}
253299
case .artifactsArchive:
254-
let tools = try self.parseArtifactsArchive(for: binaryTarget, triple: buildParameters.triple)
255-
tools.forEach { availableTools[$0.name] = $0.executablePath }
256-
case.unknown:
257-
throw InternalError("unknown binary target '\(target.name)' type")
300+
let tools = try self.parseArtifactsArchive(
301+
for: binaryTarget, triple: productDescription.buildParameters.triple
302+
)
303+
tools.forEach { availableTools[$0.name] = $0.executablePath }
304+
case .unknown:
305+
throw InternalError("unknown binary target '\(module.name)' type")
258306
}
259307
case .plugin:
260308
continue
261309
}
262310

263-
case .product(let product, _):
311+
case .product(let product, let description):
264312
// Add the dynamic products to array of libraries to link.
265313
if product.type == .library(.dynamic) {
266-
linkLibraries.append(product)
314+
guard let description else {
315+
throw InternalError("Dynamic library product should have description: \(product)")
316+
}
317+
linkLibraries.append(description)
267318
}
268319
}
269320
}
270321

271322
// Add derived test targets, if necessary
272323
if product.type == .test, let derivedTestTargets = derivedTestTargetsMap[product.id] {
273-
staticTargets.append(contentsOf: derivedTestTargets)
324+
staticTargets.append(contentsOf: derivedTestTargets.compactMap {
325+
self.description(for: $0, context: productDescription.destination)
326+
})
274327
}
275328

276329
return (linkLibraries, staticTargets, systemModules, libraryBinaryPaths, availableTools)
@@ -280,7 +333,7 @@ extension BuildPlan {
280333
private func parseArtifactsArchive(for binaryTarget: BinaryModule, triple: Triple) throws -> [ExecutableInfo] {
281334
try self.externalExecutablesCache.memoize(key: binaryTarget) {
282335
let execInfos = try binaryTarget.parseArtifactArchives(for: triple, fileSystem: self.fileSystem)
283-
return execInfos.filter{!$0.supportedTriples.isEmpty}
336+
return execInfos.filter { !$0.supportedTriples.isEmpty }
284337
}
285338
}
286339
}

‎Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ extension BuildPlan {
934934
extension BuildPlan {
935935
fileprivate typealias Destination = BuildParameters.Destination
936936

937-
fileprivate enum TraversalNode: Hashable {
937+
enum TraversalNode: Hashable {
938938
case package(ResolvedPackage)
939939
case product(ResolvedProduct, BuildParameters.Destination)
940940
case module(ResolvedModule, BuildParameters.Destination)

0 commit comments

Comments
 (0)