From 3a5475150970555fcab3e43d22cc1a5bbc5ee99e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 14 Aug 2024 18:30:12 -0700 Subject: [PATCH 1/6] Revert "[Build] BuildPlan: Start using id from module/product build descriptions for build plan" This reverts commit 0c1bed823da8ac310df5374a0c5b406da94beebe. --- .../Basics/Collections/IdentifiableSet.swift | 4 -- .../LLBuildManifestBuilder.swift | 28 +++++------ .../Build/BuildPlan/BuildPlan+Product.swift | 18 +++---- Sources/Build/BuildPlan/BuildPlan+Test.swift | 5 +- Sources/Build/BuildPlan/BuildPlan.swift | 49 +++++++------------ .../SourceKitLSPAPI/BuildDescription.swift | 23 +++++---- .../MockBuildTestHelper.swift | 8 ++- .../CrossCompilationBuildPlanTests.swift | 2 +- .../SourceKitLSPAPITests.swift | 11 ++--- 9 files changed, 65 insertions(+), 83 deletions(-) diff --git a/Sources/Basics/Collections/IdentifiableSet.swift b/Sources/Basics/Collections/IdentifiableSet.swift index 1941263c1d2..46383085b2b 100644 --- a/Sources/Basics/Collections/IdentifiableSet.swift +++ b/Sources/Basics/Collections/IdentifiableSet.swift @@ -42,10 +42,6 @@ public struct IdentifiableSet: Collection { Index(storageIndex: self.storage.elements.endIndex) } - public var values: some Sequence { - self.storage.values - } - public subscript(position: Index) -> Element { self.storage.elements[position.storageIndex].value } diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift index 6b74a890067..b12be0a1832 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder.swift @@ -102,7 +102,7 @@ public class LLBuildManifestBuilder { try addTargetsToExplicitBuildManifest() } else { // Create commands for all target descriptions in the plan. - for description in self.plan.targetMap { + for (_, description) in self.plan.targetMap { switch description { case .swift(let desc): try self.createSwiftCompileCommand(desc) @@ -116,7 +116,7 @@ public class LLBuildManifestBuilder { try self.addTestEntryPointGenerationCommand() // Create command for all products in the plan. - for description in self.plan.productMap { + for (_, description) in self.plan.productMap { try self.createProductCommand(description) } @@ -133,7 +133,7 @@ public class LLBuildManifestBuilder { addPackageStructureCommand() - for description in self.plan.targetMap { + for (_, description) in self.plan.targetMap { switch description { case .swift(let desc): try self.createSwiftCompileCommand(desc) @@ -148,7 +148,7 @@ public class LLBuildManifestBuilder { } } - for description in self.plan.productMap { + for (_, description) in self.plan.productMap { // Need to generate macro products switch description.product.type { case .macro, .plugin: @@ -271,7 +271,7 @@ extension LLBuildManifestBuilder { private func addTestDiscoveryGenerationCommand() throws { for testDiscoveryTarget in self.plan.targets.compactMap(\.testDiscoveryTargetBuildDescription) { let testTargets = testDiscoveryTarget.target.dependencies - .compactMap(\.module).compactMap { self.plan.description(for: $0, context: testDiscoveryTarget.destination) } + .compactMap(\.module).compactMap { self.plan.targetMap[$0.id] } let objectFiles = try testTargets.flatMap { try $0.objects }.sorted().map(Node.file) let outputs = testDiscoveryTarget.target.sources.paths @@ -288,22 +288,18 @@ extension LLBuildManifestBuilder { } private func addTestEntryPointGenerationCommand() throws { - for module in self.plan.targets { - guard case .swift(let swiftModule) = module, - case .entryPoint(let isSynthesized) = swiftModule.testTargetRole, + for target in self.plan.targets { + guard case .swift(let target) = target, + case .entryPoint(let isSynthesized) = target.testTargetRole, isSynthesized else { continue } - let testEntryPointTarget = swiftModule + let testEntryPointTarget = target // Get the Swift target build descriptions of all discovery modules this synthesized entry point target // depends on. - let discoveredTargetDependencyBuildDescriptions = module.dependencies(using: self.plan) - .compactMap { - if case .module(_, let description) = $0 { - return description - } - return nil - } + let discoveredTargetDependencyBuildDescriptions = testEntryPointTarget.target.dependencies + .compactMap(\.module?.id) + .compactMap { self.plan.targetMap[$0] } .compactMap(\.testDiscoveryTargetBuildDescription) // The module outputs of the discovery modules this synthesized entry point target depends on are diff --git a/Sources/Build/BuildPlan/BuildPlan+Product.swift b/Sources/Build/BuildPlan/BuildPlan+Product.swift index 11879b8d214..1f2a511d6f8 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Product.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Product.swift @@ -77,12 +77,12 @@ extension BuildPlan { } } - for module in dependencies.staticTargets { - switch module.underlying { + for target in dependencies.staticTargets { + switch target.underlying { case is SwiftModule: // Swift targets are guaranteed to have a corresponding Swift description. - guard case .swift(let description) = self.description(for: module, context: buildProduct.destination) else { - throw InternalError("unknown module \(module)") + guard case .swift(let description) = self.targetMap[target.id] else { + throw InternalError("unknown target \(target)") } // Based on the debugging strategy, we either need to pass swiftmodule paths to the @@ -103,16 +103,16 @@ extension BuildPlan { buildProduct.staticTargets = dependencies.staticTargets buildProduct.dylibs = try dependencies.dylibs.map { - guard let product = self.description(for: $0, context: buildProduct.destination) else { + guard let product = self.productMap[$0.id] else { throw InternalError("unknown product \($0)") } return product } - buildProduct.objects += try dependencies.staticTargets.flatMap { module -> [AbsolutePath] in - guard let description = self.description(for: module, context: buildProduct.destination) else { - throw InternalError("unknown module \(module)") + buildProduct.objects += try dependencies.staticTargets.flatMap { targetName -> [AbsolutePath] in + guard let target = self.targetMap[targetName.id] else { + throw InternalError("unknown target \(targetName)") } - return try description.objects + return try target.objects } buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths diff --git a/Sources/Build/BuildPlan/BuildPlan+Test.swift b/Sources/Build/BuildPlan/BuildPlan+Test.swift index 94a14db29ff..9b67e656fdd 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Test.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Test.swift @@ -34,7 +34,7 @@ import protocol TSCBasic.FileSystem extension BuildPlan { static func makeDerivedTestTargets( - testProducts: [ProductBuildDescription], + testProducts: [(product: ResolvedProduct, buildDescription: ProductBuildDescription)], destinationBuildParameters: BuildParameters, toolsBuildParameters: BuildParameters, shouldDisableSandbox: Bool, @@ -51,8 +51,7 @@ extension BuildPlan { var isDiscoveryEnabledRedundantly = explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly var result: [(ResolvedProduct, SwiftModuleBuildDescription?, SwiftModuleBuildDescription)] = [] - for testBuildDescription in testProducts { - let testProduct = testBuildDescription.product + for (testProduct, testBuildDescription) in testProducts { let package = testBuildDescription.package isDiscoveryEnabledRedundantly = isDiscoveryEnabledRedundantly && nil == testProduct.testEntryPointModule diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 31a193e85d0..d2b0c1c4164 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -197,10 +197,10 @@ public class BuildPlan: SPMBuildCore.BuildPlan { public let graph: ModulesGraph /// The target build description map. - public let targetMap: IdentifiableSet + public let targetMap: [ResolvedModule.ID: ModuleBuildDescription] /// The product build description map. - public let productMap: IdentifiableSet + public let productMap: [ResolvedProduct.ID: ProductBuildDescription] /// The plugin descriptions. Plugins are represented in the package graph /// as targets, but they are not directly included in the build graph. @@ -290,12 +290,13 @@ public class BuildPlan: SPMBuildCore.BuildPlan { var prebuildCommandResults: [ResolvedModule.ID: [CommandPluginResult]] = [:] // Create product description for each product we have in the package graph that is eligible. - var productMap = IdentifiableSet() + var productMap: [ResolvedProduct.ID: (product: ResolvedProduct, buildDescription: ProductBuildDescription)] = + [:] // Create build target description for each target which we need to plan. // Plugin targets are noted, since they need to be compiled, but they do // not get directly incorporated into the build description that will be // given to LLBuild. - var targetMap = IdentifiableSet() + var targetMap = [ResolvedModule.ID: ModuleBuildDescription]() var pluginDescriptions = [PluginBuildDescription]() var shouldGenerateTestObservation = true @@ -311,7 +312,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { throw InternalError("Package not found for product: \(product.name)") } - try productMap.insert(ProductBuildDescription( + productMap[product.id] = try (product, ProductBuildDescription( package: package, product: product, toolsVersion: package.manifest.toolsVersion, @@ -383,7 +384,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { shouldGenerateTestObservation = false // Only generate the code once. } - try targetMap.insert(.swift( + targetMap[module.id] = try .swift( SwiftModuleBuildDescription( package: package, target: module, @@ -398,9 +399,9 @@ public class BuildPlan: SPMBuildCore.BuildPlan { fileSystem: fileSystem, observabilityScope: planningObservabilityScope ) - )) + ) case is ClangModule: - try targetMap.insert(.clang( + targetMap[module.id] = try .clang( ClangModuleBuildDescription( package: package, target: module, @@ -412,7 +413,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { fileSystem: fileSystem, observabilityScope: planningObservabilityScope ) - )) + ) case is PluginModule: try module.dependencies.compactMap { switch $0 { @@ -428,7 +429,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { return nil } }.forEach { - try productMap.insert(ProductBuildDescription( + productMap[$0.id] = try ($0, ProductBuildDescription( package: package, product: $0, toolsVersion: package.manifest.toolsVersion, @@ -467,7 +468,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { // Plan the derived test targets, if necessary. let derivedTestTargets = try Self.makeDerivedTestTargets( - testProducts: productMap.filter { + testProducts: productMap.values.filter { $0.product.type == .test }, destinationBuildParameters: destinationBuildParameters, @@ -479,12 +480,12 @@ public class BuildPlan: SPMBuildCore.BuildPlan { for item in derivedTestTargets { var derivedTestTargets = [item.entryPointTargetBuildDescription.target] - targetMap.insert(.swift( + targetMap[item.entryPointTargetBuildDescription.target.id] = .swift( item.entryPointTargetBuildDescription - )) + ) if let discoveryTargetBuildDescription = item.discoveryTargetBuildDescription { - targetMap.insert(.swift(discoveryTargetBuildDescription)) + targetMap[discoveryTargetBuildDescription.target.id] = .swift(discoveryTargetBuildDescription) derivedTestTargets.append(discoveryTargetBuildDescription.target) } @@ -494,7 +495,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { self.buildToolPluginInvocationResults = buildToolPluginInvocationResults self.prebuildCommandResults = prebuildCommandResults - self.productMap = productMap + self.productMap = productMap.mapValues(\.buildDescription) self.targetMap = targetMap self.pluginDescriptions = pluginDescriptions @@ -710,28 +711,14 @@ public class BuildPlan: SPMBuildCore.BuildPlan { for product: ResolvedProduct, context: BuildParameters.Destination ) -> ProductBuildDescription? { - let destination: BuildParameters.Destination = switch product.type { - case .macro, .plugin: - .host - default: - context - } - - return self.productMap[.init(productID: product.id, destination: destination)] + return self.productMap[product.id] } public func description( for module: ResolvedModule, context: BuildParameters.Destination ) -> ModuleBuildDescription? { - let destination: BuildParameters.Destination = switch module.type { - case .macro, .plugin: - .host - default: - context - } - - return self.targetMap[.init(moduleID: module.id, destination: destination)] + return self.targetMap[module.id] } } diff --git a/Sources/SourceKitLSPAPI/BuildDescription.swift b/Sources/SourceKitLSPAPI/BuildDescription.swift index 6892917821e..0c7db59fe79 100644 --- a/Sources/SourceKitLSPAPI/BuildDescription.swift +++ b/Sources/SourceKitLSPAPI/BuildDescription.swift @@ -15,7 +15,7 @@ import struct Foundation.URL private import struct Basics.AbsolutePath private import func Basics.resolveSymlinks -internal import SPMBuildCore +private import SPMBuildCore // FIXME: should import these module with `private` or `internal` access control import class Build.BuildPlan @@ -127,12 +127,9 @@ public struct BuildDescription { self.inputs = buildPlan.inputs } - func getBuildTarget( - for module: ResolvedModule, - destination: BuildParameters.Destination - ) -> BuildTarget? { - if let description = self.buildPlan.description(for: module, context: destination) { - let modulesGraph = self.buildPlan.graph + // FIXME: should not use `ResolvedTarget` in the public interface + public func getBuildTarget(for target: ResolvedModule, in modulesGraph: ModulesGraph) -> BuildTarget? { + if let description = buildPlan.targetMap[target.id] { switch description { case .clang(let description): return WrappedClangTargetBuildDescription( @@ -146,10 +143,9 @@ public struct BuildDescription { ) } } else { - if module.type == .plugin, let package = self.buildPlan.graph.package(for: module) { - let modulesGraph = self.buildPlan.graph + if target.type == .plugin, let package = self.buildPlan.graph.package(for: target) { return PluginTargetBuildDescription( - target: module, + target: target, toolsVersion: package.manifest.toolsVersion, toolchain: buildPlan.toolsBuildParameters.toolchain, isPartOfRootPackage: modulesGraph.rootPackages.map(\.id).contains(package.id) @@ -162,14 +158,17 @@ public struct BuildDescription { public func traverseModules( callback: (any BuildTarget, _ parent: (any BuildTarget)?, _ depth: Int) -> Void ) { + // TODO: Once the `targetMap` is switched over to use `IdentifiableSet` + // we can introduce `BuildPlan.description(ResolvedModule, BuildParameters.Destination)` + // and start using that here. self.buildPlan.traverseModules { module, parent, depth in let parentDescription: (any BuildTarget)? = if let parent { - getBuildTarget(for: parent.0, destination: parent.1) + getBuildTarget(for: parent.0, in: self.buildPlan.graph) } else { nil } - if let description = getBuildTarget(for: module.0, destination: module.1) { + if let description = getBuildTarget(for: module.0, in: self.buildPlan.graph) { callback(description, parentDescription, depth) } } diff --git a/Sources/_InternalTestSupport/MockBuildTestHelper.swift b/Sources/_InternalTestSupport/MockBuildTestHelper.swift index 30a68afc3a1..94ca81d1ec0 100644 --- a/Sources/_InternalTestSupport/MockBuildTestHelper.swift +++ b/Sources/_InternalTestSupport/MockBuildTestHelper.swift @@ -297,7 +297,13 @@ public struct BuildPlanResult { ) self.targetMap = try Dictionary( throwingUniqueKeysWithValues: plan.targetMap.compactMap { - ($0.module.id, $0) + guard + let target = plan.graph.allModules[$0] ?? + IdentifiableSet(plan.derivedTestTargetsMap.values.flatMap { $0 })[$0] + else { + throw BuildError.error("Target \($0) not found.") + } + return (target.id, $1) } ) } diff --git a/Tests/BuildTests/CrossCompilationBuildPlanTests.swift b/Tests/BuildTests/CrossCompilationBuildPlanTests.swift index 607d1c8767e..6cdc02c4fec 100644 --- a/Tests/BuildTests/CrossCompilationBuildPlanTests.swift +++ b/Tests/BuildTests/CrossCompilationBuildPlanTests.swift @@ -297,7 +297,7 @@ final class CrossCompilationBuildPlanTests: XCTestCase { ) // Make sure that build plan doesn't have any "target" tests. - for description in plan.targetMap where description.module.underlying.type == .test { + for (_, description) in plan.targetMap where description.module.underlying.type == .test { XCTAssertEqual(description.buildParameters.destination, .host) } diff --git a/Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift b/Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift index 399544d3d2d..5c7eec517dd 100644 --- a/Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift +++ b/Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift @@ -17,8 +17,7 @@ import Build import PackageGraph import PackageModel -@testable import SourceKitLSPAPI -import struct SPMBuildCore.BuildParameters +import SourceKitLSPAPI import _InternalTestSupport import XCTest @@ -91,7 +90,7 @@ final class SourceKitLSPAPITests: XCTestCase { "-I", "/fake/manifestLib/path" ], isPartOfRootPackage: true, - destination: .host + destination: .tools ) } @@ -168,10 +167,10 @@ extension SourceKitLSPAPI.BuildDescription { graph: ModulesGraph, partialArguments: [String], isPartOfRootPackage: Bool, - destination: BuildParameters.Destination = .target + destination: BuildTriple = .destination ) throws -> Bool { - let target = try XCTUnwrap(graph.module(for: targetName)) - let buildTarget = try XCTUnwrap(self.getBuildTarget(for: target, destination: destination)) + let target = try XCTUnwrap(graph.module(for: targetName, destination: destination)) + let buildTarget = try XCTUnwrap(self.getBuildTarget(for: target, in: graph)) guard let file = buildTarget.sources.first else { XCTFail("build target \(targetName) contains no files") From 1e5157132d4759347231663c8c25c8f284638e06 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 14 Aug 2024 18:30:12 -0700 Subject: [PATCH 2/6] Revert "[Build] NFC: Start using `ModuleBuildDescription.{recursive}Dependencies`" This reverts commit 659199f1dd6a34be98a20a56e8bca0427b6a19a7. --- .../LLBuildManifestBuilder+Clang.swift | 30 ++--- .../LLBuildManifestBuilder+Swift.swift | 110 ++++++++---------- Sources/Build/BuildPlan/BuildPlan+Clang.swift | 8 +- Sources/Build/BuildPlan/BuildPlan+Swift.swift | 5 +- 4 files changed, 70 insertions(+), 83 deletions(-) diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift index 369206bddb1..a059bcce1eb 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Clang.swift @@ -32,36 +32,30 @@ extension LLBuildManifestBuilder { inputs.append(resourcesNode) } - func addStaticTargetInputs(_ description: ModuleBuildDescription?) { - if case .swift(let desc) = description, desc.target.type == .library { + func addStaticTargetInputs(_ target: ResolvedModule) { + if case .swift(let desc)? = self.plan.targetMap[target.id], target.type == .library { inputs.append(file: desc.moduleOutputPath) } } - for dependency in target.dependencies(using: self.plan) { + for dependency in target.target.dependencies(satisfying: target.buildEnvironment) { switch dependency { - case .module(_, let description): - addStaticTargetInputs(description) + case .module(let target, _): + addStaticTargetInputs(target) - case .product(let product, let productDescription): + case .product(let product, _): switch product.type { case .executable, .snippet, .library(.dynamic), .macro: - guard let productDescription else { - throw InternalError("No build description for product: \(product)") + guard let planProduct = plan.productMap[product.id] else { + throw InternalError("unknown product \(product)") } // Establish a dependency on binary of the product. - try inputs.append(file: productDescription.binaryPath) + let binary = try planProduct.binaryPath + inputs.append(file: binary) case .library(.automatic), .library(.static), .plugin: - for module in product.modules { - guard let dependencyDescription = self.plan.description( - for: module, - context: product.type == .plugin ? .host : target.destination - ) else - { - throw InternalError("unknown module: \(module)") - } - addStaticTargetInputs(dependencyDescription) + for target in product.modules { + addStaticTargetInputs(target) } case .test: break diff --git a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift index 0a7da8c47b1..822ecc66a07 100644 --- a/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift +++ b/Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift @@ -191,8 +191,10 @@ extension LLBuildManifestBuilder { public func addTargetsToExplicitBuildManifest() throws { // Sort the product targets in topological order in order to collect and "bubble up" // their respective dependency graphs to the depending targets. - let allPackageDependencies = self.plan.targets.flatMap { $0.recursiveDependencies(using: self.plan) } - + let nodes = self.plan.targets.compactMap { + ResolvedModule.Dependency.module($0.module, conditions: []) + } + let allPackageDependencies = try topologicalSort(nodes, successors: { $0.dependencies }) // Instantiate the inter-module dependency oracle which will cache commonly-scanned // modules across targets' Driver instances. let dependencyOracle = InterModuleDependencyOracle() @@ -204,15 +206,14 @@ extension LLBuildManifestBuilder { // Create commands for all module descriptions in the plan. for dependency in allPackageDependencies.reversed() { - guard case .module(let module, let description) = dependency else { + guard case .module(let target, _) = dependency else { // Product dependency build jobs are added after the fact. // Targets that depend on product dependencies will expand the corresponding // product into its constituent targets. continue } - - guard module.underlying.type != .systemModule, - module.underlying.type != .binary + guard target.underlying.type != .systemModule, + target.underlying.type != .binary else { // Much like non-Swift targets, system modules will consist of a modulemap // somewhere in the filesystem, with the path to that module being either @@ -224,11 +225,9 @@ extension LLBuildManifestBuilder { // be able to detect such targets' modules. continue } - - guard let description else { - throw InternalError("Expected description for module \(module)") + guard let description = plan.targetMap[target.id] else { + throw InternalError("Expected description for target \(target)") } - switch description { case .swift(let desc): try self.createExplicitSwiftTargetCompileCommand( @@ -320,29 +319,32 @@ extension LLBuildManifestBuilder { for targetDescription: ModuleBuildDescription, dependencyModuleDetailsMap: inout SwiftDriver.ExternalTargetModuleDetailsMap ) throws { - for dependency in targetDescription.dependencies(using: self.plan) { + for dependency in targetDescription.module.dependencies(satisfying: targetDescription.buildParameters.buildEnvironment) { switch dependency { - case .product(let product, let productDescription): - for productDependency in product.modules { - guard let dependencyModuleDescription = self.plan.description( - for: productDependency, - context: productDescription?.destination ?? targetDescription.destination - ) else - { - throw InternalError("unknown dependency target for \(productDependency)") + case .product: + // Product dependencies are broken down into the targets that make them up. + guard let dependencyProduct = dependency.product else { + throw InternalError("unknown dependency product for \(dependency)") + } + for dependencyProductTarget in dependencyProduct.modules { + guard let dependencyTargetDescription = self.plan.targetMap[dependencyProductTarget.id] else { + throw InternalError("unknown dependency target for \(dependencyProductTarget)") } try self.addTargetDependencyInfo( - for: dependencyModuleDescription, + for: dependencyTargetDescription, dependencyModuleDetailsMap: &dependencyModuleDetailsMap ) } - case .module(let dependencyModule, let dependencyDescription): - guard let dependencyDescription else { - throw InternalError("No build description for module: \(dependencyModule)") - } + case .module: // Product dependencies are broken down into the targets that make them up. + guard + let dependencyTarget = dependency.module, + let dependencyTargetDescription = self.plan.targetMap[dependencyTarget.id] + else { + throw InternalError("unknown dependency target for \(dependency)") + } try self.addTargetDependencyInfo( - for: dependencyDescription, + for: dependencyTargetDescription, dependencyModuleDetailsMap: &dependencyModuleDetailsMap ) } @@ -420,73 +422,63 @@ extension LLBuildManifestBuilder { let prepareForIndexing = target.buildParameters.prepareForIndexing - func addStaticTargetInputs(_ module: ResolvedModule, _ description: ModuleBuildDescription?) throws { + func addStaticTargetInputs(_ target: ResolvedModule) throws { // Ignore C Modules. - if module.underlying is SystemLibraryModule { return } + if target.underlying is SystemLibraryModule { return } // Ignore Binary Modules. - if module.underlying is BinaryModule { return } + if target.underlying is BinaryModule { return } // Ignore Plugin Modules. - if module.underlying is PluginModule { return } - - guard let description else { - throw InternalError("No build description for module: \(module)") - } + if target.underlying is PluginModule { return } // Depend on the binary for executable targets. - if module.type == .executable && prepareForIndexing == .off { - // FIXME: Optimize. Build plan could build a mapping between executable modules - // and their products to speed up search here, which is inefficient if the plan - // contains a lot of products. + if target.type == .executable && prepareForIndexing == .off { + // FIXME: Optimize. if let productDescription = try plan.productMap.values.first(where: { - try $0.product.type == .executable && - $0.product.executableModule.id == module.id && - $0.destination == description.destination + try $0.product.type == .executable && $0.product.executableModule.id == target.id }) { try inputs.append(file: productDescription.binaryPath) } return } - switch description { - case .swift(let swiftDescription): - inputs.append(file: swiftDescription.moduleOutputPath) - case .clang(let clangDescription): + switch self.plan.targetMap[target.id] { + case .swift(let target)?: + inputs.append(file: target.moduleOutputPath) + case .clang(let target)?: if prepareForIndexing != .off { // In preparation, we're only building swiftmodules // propagate the dependency to the header files in this target - for header in clangDescription.clangTarget.headers { + for header in target.clangTarget.headers { inputs.append(file: header) } } else { - for object in try clangDescription.objects { + for object in try target.objects { inputs.append(file: object) } } + case nil: + throw InternalError("unexpected: target \(target) not in target map \(self.plan.targetMap)") } } - for dependency in target.dependencies(using: self.plan) { + for dependency in target.target.dependencies(satisfying: target.buildParameters.buildEnvironment) { switch dependency { - case .module(let module, let description): - try addStaticTargetInputs(module, description) + case .module(let target, _): + try addStaticTargetInputs(target) - case .product(let product, let productDescription): + case .product(let product, _): switch product.type { case .executable, .snippet, .library(.dynamic), .macro: - guard let productDescription else { - throw InternalError("No description for product: \(product)") + guard let planProduct = plan.productMap[product.id] else { + throw InternalError("unknown product \(product)") } // Establish a dependency on binary of the product. - try inputs.append(file: productDescription.binaryPath) + try inputs.append(file: planProduct.binaryPath) // For automatic and static libraries, and plugins, add their targets as static input. case .library(.automatic), .library(.static), .plugin: - for module in product.modules { - let description = self.plan.description( - for: module, - context: product.type == .plugin ? .host : target.destination - ) - try addStaticTargetInputs(module, description) + for target in product.modules { + try addStaticTargetInputs(target) } case .test: diff --git a/Sources/Build/BuildPlan/BuildPlan+Clang.swift b/Sources/Build/BuildPlan/BuildPlan+Clang.swift index ca83e4b2e6a..1c7c9575a1a 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Clang.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Clang.swift @@ -18,12 +18,12 @@ import class PackageModel.SystemLibraryModule extension BuildPlan { /// Plan a Clang target. func plan(clangTarget: ClangModuleBuildDescription) throws { - let dependencies = clangTarget.recursiveDependencies(using: self) + let dependencies = try clangTarget.target.recursiveDependencies(satisfying: clangTarget.buildEnvironment) - for case .module(let dependency, let description) in dependencies { + for case .module(let dependency, _) in dependencies { switch dependency.underlying { case is SwiftModule: - if case let .swift(dependencyTargetDescription)? = description { + if case let .swift(dependencyTargetDescription)? = targetMap[dependency.id] { if let moduleMap = dependencyTargetDescription.moduleMap { clangTarget.additionalFlags += ["-fmodule-map-file=\(moduleMap.pathString)"] } @@ -34,7 +34,7 @@ extension BuildPlan { clangTarget.additionalFlags += ["-I", target.includeDir.pathString] // Add the modulemap of the dependency if it has one. - if case let .clang(dependencyTargetDescription)? = description { + if case let .clang(dependencyTargetDescription)? = targetMap[dependency.id] { if let moduleMap = dependencyTargetDescription.moduleMap { clangTarget.additionalFlags += ["-fmodule-map-file=\(moduleMap.pathString)"] } diff --git a/Sources/Build/BuildPlan/BuildPlan+Swift.swift b/Sources/Build/BuildPlan/BuildPlan+Swift.swift index 7d387f8cfd5..9a56e5732c5 100644 --- a/Sources/Build/BuildPlan/BuildPlan+Swift.swift +++ b/Sources/Build/BuildPlan/BuildPlan+Swift.swift @@ -20,10 +20,11 @@ extension BuildPlan { func plan(swiftTarget: SwiftModuleBuildDescription) throws { // We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target // depends on. - for case .module(let dependency, let description) in swiftTarget.recursiveDependencies(using: self) { + let environment = swiftTarget.buildParameters.buildEnvironment + for case .module(let dependency, _) in try swiftTarget.target.recursiveDependencies(satisfying: environment) { switch dependency.underlying { case let underlyingTarget as ClangModule where underlyingTarget.type == .library: - guard case let .clang(target)? = description else { + guard case let .clang(target)? = targetMap[dependency.id] else { throw InternalError("unexpected clang target \(underlyingTarget)") } // Add the path to modulemap of the dependency. Currently we require that all Clang targets have a From 1f67d963bcb797e49b5b37c026c0121b3cadbee7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 14 Aug 2024 18:30:12 -0700 Subject: [PATCH 3/6] Revert "[BuildDescription] Add a way to get (recursive) dependencies from a module build description" This reverts commit f330f945031d593b98314766cc93e50e87fd6f6c. --- .../ClangModuleBuildDescription.swift | 14 ---- .../ModuleBuildDescription.swift | 36 +-------- .../SwiftModuleBuildDescription.swift | 14 ---- Sources/Build/BuildPlan/BuildPlan.swift | 5 -- .../BuildParameters/BuildParameters.swift | 2 +- .../BuildTests/BuildPlanTraversalTests.swift | 81 ------------------- 6 files changed, 2 insertions(+), 150 deletions(-) diff --git a/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift b/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift index b149675ebe2..5fa90724974 100644 --- a/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift @@ -526,17 +526,3 @@ public final class ClangModuleBuildDescription { ) } } - -extension ClangModuleBuildDescription { - package func dependencies( - using plan: BuildPlan - ) -> [ModuleBuildDescription.Dependency] { - ModuleBuildDescription.clang(self).dependencies(using: plan) - } - - package func recursiveDependencies( - using plan: BuildPlan - ) -> [ModuleBuildDescription.Dependency] { - ModuleBuildDescription.clang(self).recursiveDependencies(using: plan) - } -} \ No newline at end of file diff --git a/Sources/Build/BuildDescription/ModuleBuildDescription.swift b/Sources/Build/BuildDescription/ModuleBuildDescription.swift index 8f52d45f370..d758c107abc 100644 --- a/Sources/Build/BuildDescription/ModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/ModuleBuildDescription.swift @@ -13,7 +13,6 @@ import Basics import struct PackageGraph.ResolvedModule import struct PackageGraph.ResolvedPackage -import struct PackageGraph.ResolvedProduct import struct PackageModel.Resource import struct PackageModel.ToolsVersion import struct SPMBuildCore.BuildToolPluginInvocationResult @@ -159,37 +158,4 @@ extension ModuleBuildDescription: Identifiable { public var id: ID { ID(moduleID: self.module.id, destination: self.destination) } -} - -extension ModuleBuildDescription { - package enum Dependency { - /// Not all of the modules and products have build descriptions - case product(ResolvedProduct, ProductBuildDescription?) - case module(ResolvedModule, ModuleBuildDescription?) - } - - package func dependencies(using plan: BuildPlan) -> [Dependency] { - self.module - .dependencies(satisfying: self.buildParameters.buildEnvironment) - .map { - switch $0 { - case .product(let product, _): - let productDescription = plan.description(for: product, context: self.destination) - return .product(product, productDescription) - case .module(let module, _): - let moduleDescription = plan.description(for: module, context: self.destination) - return .module(module, moduleDescription) - } - } - } - - package func recursiveDependencies(using plan: BuildPlan) -> [Dependency] { - var dependencies: [Dependency] = [] - plan.traverseDependencies(of: self) { product, _, description in - dependencies.append(.product(product, description)) - } onModule: { module, _, description in - dependencies.append(.module(module, description)) - } - return dependencies - } -} +} \ No newline at end of file diff --git a/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift b/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift index 63cbfe2ab61..260b55c1226 100644 --- a/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift @@ -988,17 +988,3 @@ public final class SwiftModuleBuildDescription { return arguments } } - -extension SwiftModuleBuildDescription { - package func dependencies( - using plan: BuildPlan - ) -> [ModuleBuildDescription.Dependency] { - ModuleBuildDescription.swift(self).dependencies(using: plan) - } - - package func recursiveDependencies( - using plan: BuildPlan - ) -> [ModuleBuildDescription.Dependency] { - ModuleBuildDescription.swift(self).recursiveDependencies(using: plan) - } -} \ No newline at end of file diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index d2b0c1c4164..2154e861b8a 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -1137,15 +1137,12 @@ extension BuildPlan { onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void, onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void ) { - var visited = Set() func successors( for product: ResolvedProduct, destination: Destination ) -> [TraversalNode] { product.modules.map { module in TraversalNode(module: module, context: destination) - }.filter { - visited.insert($0).inserted } } @@ -1162,8 +1159,6 @@ extension BuildPlan { case .module(let module, _): partial.append(.init(module: module, context: destination)) } - }.filter { - visited.insert($0).inserted } } diff --git a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift index a95c69a53ce..89e486677a2 100644 --- a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift +++ b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift @@ -38,7 +38,7 @@ public struct BuildParameters: Encodable { } /// The destination for which code should be compiled for. - public enum Destination: Hashable, Encodable { + public enum Destination: Encodable { /// The destination for which build tools are compiled. case host diff --git a/Tests/BuildTests/BuildPlanTraversalTests.swift b/Tests/BuildTests/BuildPlanTraversalTests.swift index 1babf8f007d..40fad431e80 100644 --- a/Tests/BuildTests/BuildPlanTraversalTests.swift +++ b/Tests/BuildTests/BuildPlanTraversalTests.swift @@ -185,86 +185,5 @@ final class BuildPlanTraversalTests: XCTestCase { XCTAssertEqual(moduleDependencies[index].1, .host) XCTAssertNotNil(moduleDependencies[index].2) } - - let directDependencies = mmioModule.dependencies(using: plan) - - XCTAssertEqual(directDependencies.count, 1) - - let dependency = try XCTUnwrap(directDependencies.first) - if case .module(let module, let description) = dependency { - XCTAssertEqual(module.name, "MMIOMacros") - try XCTAssertEqual(XCTUnwrap(description).destination, .host) - } else { - XCTFail("Expected MMIOMacros module") - } - - let dependencies = mmioModule.recursiveDependencies(using: plan) - - XCTAssertEqual(dependencies.count, 3) - - // MMIOMacros (module) -> SwiftSyntax (product) -> SwiftSyntax (module) - - if case .module(let module, let description) = dependencies[0] { - XCTAssertEqual(module.name, "MMIOMacros") - try XCTAssertEqual(XCTUnwrap(description).destination, .host) - } else { - XCTFail("Expected MMIOMacros module") - } - - if case .product(let product, let description) = dependencies[1] { - XCTAssertEqual(product.name, "SwiftSyntax") - XCTAssertNil(description) - } else { - XCTFail("Expected SwiftSyntax product") - } - - if case .module(let module, let description) = dependencies[2] { - XCTAssertEqual(module.name, "SwiftSyntax") - try XCTAssertEqual(XCTUnwrap(description).destination, .host) - } else { - XCTFail("Expected SwiftSyntax module") - } - } - - func testRecursiveDependencyTraversalWithDuplicates() async throws { - let destinationTriple = Triple.arm64Linux - let toolsTriple = Triple.x86_64MacOS - - let (graph, fs, scope) = try macrosTestsPackageGraph() - let plan = try await BuildPlan( - destinationBuildParameters: mockBuildParameters( - destination: .target, - triple: destinationTriple - ), - toolsBuildParameters: mockBuildParameters( - destination: .host, - triple: toolsTriple - ), - graph: graph, - fileSystem: fs, - observabilityScope: scope - ) - - let testModule = try XCTUnwrap(plan.description(for: graph.module(for: "MMIOMacrosTests")!, context: .host)) - - let dependencies = testModule.recursiveDependencies(using: plan) - XCTAssertEqual(dependencies.count, 9) - - struct ModuleResult: Hashable { - let module: ResolvedModule - let destination: Dest - } - - var uniqueModules = Set() - for dependency in dependencies { - if case .module(let module, let description) = dependency { - XCTAssertNotNil(description) - XCTAssertEqual(description!.destination, .host) - XCTAssertTrue( - uniqueModules.insert(.init(module: module, destination: description!.destination)) - .inserted - ) - } - } } } From 46d5038b2ba5c33246dc713b1ba8ce163377e047 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 14 Aug 2024 18:30:12 -0700 Subject: [PATCH 4/6] Revert "[Build] BuildPlan: Add a way to traverse module dependencies" This reverts commit bf7f706aea0d9973af71ff269afef1b366ca67b8. --- Sources/Build/BuildPlan/BuildPlan.swift | 67 ------------------- .../BuildTests/BuildPlanTraversalTests.swift | 43 ------------ 2 files changed, 110 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 2154e861b8a..2a9ae61390f 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -706,20 +706,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan { } return inputs } - - public func description( - for product: ResolvedProduct, - context: BuildParameters.Destination - ) -> ProductBuildDescription? { - return self.productMap[product.id] - } - - public func description( - for module: ResolvedModule, - context: BuildParameters.Destination - ) -> ModuleBuildDescription? { - return self.targetMap[module.id] - } } extension BuildPlan { @@ -1131,59 +1117,6 @@ extension BuildPlan { } } } - - package func traverseDependencies( - of description: ModuleBuildDescription, - onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void, - onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void - ) { - func successors( - for product: ResolvedProduct, - destination: Destination - ) -> [TraversalNode] { - product.modules.map { module in - TraversalNode(module: module, context: destination) - } - } - - func successors( - for module: ResolvedModule, - destination: Destination - ) -> [TraversalNode] { - module - .dependencies(satisfying: description.buildParameters.buildEnvironment) - .reduce(into: [TraversalNode]()) { partial, dependency in - switch dependency { - case .product(let product, _): - partial.append(.init(product: product, context: destination)) - case .module(let module, _): - partial.append(.init(module: module, context: destination)) - } - } - } - - depthFirstSearch(successors(for: description.module, destination: description.destination)) { - switch $0 { - case .module(let module, let destination): - successors(for: module, destination: destination) - case .product(let product, let destination): - successors(for: product, destination: destination) - case .package: - [] - } - } onNext: { module, _, _ in - switch module { - case .package: - break - - case .product(let product, let destination): - onProduct(product, destination, self.description(for: product, context: destination)) - - case .module(let module, let destination): - onModule(module, destination, self.description(for: module, context: destination)) - } - } - } } extension Basics.Diagnostic { diff --git a/Tests/BuildTests/BuildPlanTraversalTests.swift b/Tests/BuildTests/BuildPlanTraversalTests.swift index 40fad431e80..41777385362 100644 --- a/Tests/BuildTests/BuildPlanTraversalTests.swift +++ b/Tests/BuildTests/BuildPlanTraversalTests.swift @@ -143,47 +143,4 @@ final class BuildPlanTraversalTests: XCTestCase { XCTAssertEqual(self.getUniqueOccurrences(in: results, for: "SwiftSyntax", destination: .host), [2, 3, 4, 5, 6]) XCTAssertEqual(self.getUniqueOccurrences(in: results, for: "HAL"), [1, 2, 3]) } - - func testRecursiveDependencyTraversal() async throws { - let destinationTriple = Triple.arm64Linux - let toolsTriple = Triple.x86_64MacOS - - let (graph, fs, scope) = try macrosPackageGraph() - let plan = try await BuildPlan( - destinationBuildParameters: mockBuildParameters( - destination: .target, - triple: destinationTriple - ), - toolsBuildParameters: mockBuildParameters( - destination: .host, - triple: toolsTriple - ), - graph: graph, - fileSystem: fs, - observabilityScope: scope - ) - - let mmioModule = try XCTUnwrap(plan.description(for: graph.module(for: "MMIO")!, context: .target)) - - var moduleDependencies: [(ResolvedModule, Dest, Build.ModuleBuildDescription?)] = [] - plan.traverseDependencies(of: mmioModule) { product, destination, description in - XCTAssertEqual(product.name, "SwiftSyntax") - XCTAssertEqual(destination, .host) - XCTAssertNil(description) - } onModule: { module, destination, description in - moduleDependencies.append((module, destination, description)) - } - - XCTAssertEqual(moduleDependencies.count, 2) - - // The ordering is guaranteed by the traversal - - XCTAssertEqual(moduleDependencies[0].0.name, "MMIOMacros") - XCTAssertEqual(moduleDependencies[1].0.name, "SwiftSyntax") - - for index in 0 ..< moduleDependencies.count { - XCTAssertEqual(moduleDependencies[index].1, .host) - XCTAssertNotNil(moduleDependencies[index].2) - } - } } From 126ced25a2bb108b6126ce417cab16b9b28ac15e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 14 Aug 2024 18:30:12 -0700 Subject: [PATCH 5/6] Revert "[BuildPlan] Make build descriptions identifiable just like Resolved{Product, Module}" This reverts commit dc34427039451e11a3e09fb764a1219430912a0e. --- .../ClangModuleBuildDescription.swift | 5 ----- .../ModuleBuildDescription.swift | 20 ------------------- .../ProductBuildDescription.swift | 16 --------------- .../SwiftModuleBuildDescription.swift | 5 ----- 4 files changed, 46 deletions(-) diff --git a/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift b/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift index 5fa90724974..08416d19730 100644 --- a/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift @@ -42,11 +42,6 @@ public final class ClangModuleBuildDescription { /// The build parameters. let buildParameters: BuildParameters - /// The destination for while this module is built. - public var destination: BuildParameters.Destination { - self.buildParameters.destination - } - /// The build environment. var buildEnvironment: BuildEnvironment { buildParameters.buildEnvironment diff --git a/Sources/Build/BuildDescription/ModuleBuildDescription.swift b/Sources/Build/BuildDescription/ModuleBuildDescription.swift index d758c107abc..b2e130cbedc 100644 --- a/Sources/Build/BuildDescription/ModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/ModuleBuildDescription.swift @@ -121,15 +121,6 @@ public enum ModuleBuildDescription: SPMBuildCore.ModuleBuildDescription { } } - var destination: BuildParameters.Destination { - switch self { - case .swift(let buildDescription): - buildDescription.destination - case .clang(let buildDescription): - buildDescription.destination - } - } - var toolsVersion: ToolsVersion { switch self { case .swift(let buildDescription): @@ -148,14 +139,3 @@ public enum ModuleBuildDescription: SPMBuildCore.ModuleBuildDescription { } } } - -extension ModuleBuildDescription: Identifiable { - public struct ID: Hashable { - let moduleID: ResolvedModule.ID - let destination: BuildParameters.Destination - } - - public var id: ID { - ID(moduleID: self.module.id, destination: self.destination) - } -} \ No newline at end of file diff --git a/Sources/Build/BuildDescription/ProductBuildDescription.swift b/Sources/Build/BuildDescription/ProductBuildDescription.swift index 6a3ec17c26c..f04fb7ec09c 100644 --- a/Sources/Build/BuildDescription/ProductBuildDescription.swift +++ b/Sources/Build/BuildDescription/ProductBuildDescription.swift @@ -37,11 +37,6 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription /// The build parameters. public let buildParameters: BuildParameters - /// The destination for while this product is built. - public var destination: BuildParameters.Destination { - self.buildParameters.destination - } - /// All object files to link into this product. /// // Computed during build planning. @@ -402,17 +397,6 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription } } -extension ProductBuildDescription: Identifiable { - public struct ID: Hashable { - let productID: ResolvedProduct.ID - let destination: BuildParameters.Destination - } - - public var id: ID { - ID(productID: self.product.id, destination: self.destination) - } -} - extension SortedArray where Element == AbsolutePath { public static func +=(lhs: inout SortedArray, rhs: S) where S.Iterator.Element == AbsolutePath { lhs.insert(contentsOf: rhs) diff --git a/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift b/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift index 260b55c1226..693ac4da8e6 100644 --- a/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift @@ -49,11 +49,6 @@ public final class SwiftModuleBuildDescription { /// The build parameters for this target. let buildParameters: BuildParameters - /// The destination for while this module is built. - public var destination: BuildParameters.Destination { - self.buildParameters.destination - } - /// The build parameters for the macro dependencies of this target. let macroBuildParameters: BuildParameters From 8ed04923b50dc6fa55cd0e8b01fb3ac9e304a447 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 14 Aug 2024 18:31:24 -0700 Subject: [PATCH 6/6] =?UTF-8?q?Revert=20"[Build]=20BuildPlan:=20Always=20d?= =?UTF-8?q?iscover=20test=20modules=20through=20their=20aggrega=E2=80=A6?= =?UTF-8?q?=20(#7879)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 648122075377444043cfb62fd0f00628f9cdde95. --- Sources/Build/BuildPlan/BuildPlan.swift | 8 ++++---- .../_InternalTestSupport/MockPackageGraphs.swift | 6 ------ .../BuildTests/CrossCompilationBuildPlanTests.swift | 8 +------- .../CrossCompilationPackageGraphTests.swift | 13 +------------ 4 files changed, 6 insertions(+), 29 deletions(-) diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index 2a9ae61390f..c5648a2419e 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -986,9 +986,9 @@ extension BuildPlan { } for module in package.modules { - // Tests are discovered through an aggregate product which also - // informs their destination. - if case .test = module.underlying.type { + if case .test = module.underlying.type, + !graph.rootPackages.contains(id: package.id) + { continue } @@ -1002,7 +1002,7 @@ extension BuildPlan { for product: ResolvedProduct, destination: Destination ) -> [TraversalNode] { - guard destination == .host || product.underlying.type == .test else { + guard destination == .host else { return [] } diff --git a/Sources/_InternalTestSupport/MockPackageGraphs.swift b/Sources/_InternalTestSupport/MockPackageGraphs.swift index 6d5a92a2f1e..ba576b84b4c 100644 --- a/Sources/_InternalTestSupport/MockPackageGraphs.swift +++ b/Sources/_InternalTestSupport/MockPackageGraphs.swift @@ -137,7 +137,6 @@ package func macrosTestsPackageGraph() throws -> MockPackageGraph { "/swift-mmio/Sources/MMIOMacros/source.swift", "/swift-mmio/Sources/MMIOMacrosTests/source.swift", "/swift-mmio/Sources/MMIOMacro+PluginTests/source.swift", - "/swift-mmio/Sources/NOOPTests/source.swift", "/swift-syntax/Sources/SwiftSyntax/source.swift", "/swift-syntax/Sources/SwiftSyntaxMacrosTestSupport/source.swift", "/swift-syntax/Sources/SwiftSyntaxMacros/source.swift", @@ -204,11 +203,6 @@ package func macrosTestsPackageGraph() throws -> MockPackageGraph { .target(name: "MMIOMacros") ], type: .test - ), - TargetDescription( - name: "NOOPTests", - dependencies: [], - type: .test ) ] ), diff --git a/Tests/BuildTests/CrossCompilationBuildPlanTests.swift b/Tests/BuildTests/CrossCompilationBuildPlanTests.swift index 6cdc02c4fec..d053b876987 100644 --- a/Tests/BuildTests/CrossCompilationBuildPlanTests.swift +++ b/Tests/BuildTests/CrossCompilationBuildPlanTests.swift @@ -295,15 +295,9 @@ final class CrossCompilationBuildPlanTests: XCTestCase { fileSystem: fs, observabilityScope: scope ) - - // Make sure that build plan doesn't have any "target" tests. - for (_, description) in plan.targetMap where description.module.underlying.type == .test { - XCTAssertEqual(description.buildParameters.destination, .host) - } - let result = try BuildPlanResult(plan: plan) result.checkProductsCount(2) - result.checkTargetsCount(17) + result.checkTargetsCount(16) XCTAssertTrue(try result.allTargets(named: "SwiftSyntax") .map { try $0.swift() } diff --git a/Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift b/Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift index 790c62e114a..c749ea28aba 100644 --- a/Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift +++ b/Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift @@ -116,11 +116,7 @@ final class CrossCompilationPackageGraphTests: XCTestCase { "SwiftSyntaxMacrosTestSupport", "SwiftSyntaxMacrosTestSupport" ) - // TODO: NOOPTests are mentioned twice because in the graph they appear - // as if they target both "tools" and "destination", see the test below. - // Once the `buildTriple` is gone, there is going to be only one mention - // left. - result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests", "NOOPTests", "NOOPTests") + result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests") result.checkTarget("MMIO") { result in result.check(buildTriple: .destination) result.check(dependencies: "MMIOMacros") @@ -184,13 +180,6 @@ final class CrossCompilationPackageGraphTests: XCTestCase { XCTAssertEqual(graph.package(for: result.target)?.identity, .plain("swift-syntax")) } } - - result.checkTargets("NOOPTests") { results in - XCTAssertEqual(results.count, 2) - - XCTAssertEqual(results.filter({ $0.target.buildTriple == .tools }).count, 1) - XCTAssertEqual(results.filter({ $0.target.buildTriple == .destination }).count, 1) - } } }