Revert "[Build] BuildPlan: Start using id from module/product build descriptions for build plan"

This reverts commit 0c1bed8.

Author: xedin

Sources/Basics/Collections/IdentifiableSet.swift

@@ -42,10 +42,6 @@ public struct IdentifiableSet<Element: Identifiable>: Collection {
         Index(storageIndex: self.storage.elements.endIndex)
     }
 
-    public var values: some Sequence<Element> {
-        self.storage.values
-    }
-
     public subscript(position: Index) -> Element {
         self.storage.elements[position.storageIndex].value
     }

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

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
 

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

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<ModuleBuildDescription>
+    public let targetMap: [ResolvedModule.ID: ModuleBuildDescription]
 
     /// The product build description map.
-    public let productMap: IdentifiableSet<ProductBuildDescription>
+    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<ProductBuildDescription>()
+        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<ModuleBuildDescription>()
+        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]
     }
 }
 

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<ModuleBuildDescription>`
+        // 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)
             }
         }

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)
             }
         )
     }

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)
         }