[Build/Commands] Switch SymbolGraphExtract to use build plan

Yet another step on the path to remove `buildTriple` from `Resolved{Product, Module}`.

Instead of trying to lookup modules in the module graph and then going
the build plan for their descriptions, symbol-graph-extract should refer
to the build plan directly.

Author: xedin

Sources/Build/BuildDescription/ClangModuleBuildDescription.swift

@@ -212,6 +212,11 @@ public final class ClangModuleBuildDescription {
     /// this module.
     package func symbolGraphExtractArguments() throws -> [String] {
         var args = [String]()
+
+        args += ["-module-name", self.target.c99name]
+        args += try self.buildParameters.tripleArgs(for: self.target)
+        args += ["-module-cache-path", try self.buildParameters.moduleCache.pathString]
+
         if self.clangTarget.isCXX {
             args += ["-cxx-interoperability-mode=default"]
         }

Sources/Build/BuildDescription/ModuleBuildDescription.swift

@@ -12,10 +12,12 @@
 
 import Basics
 import struct PackageGraph.ResolvedModule
+import struct PackageGraph.ResolvedPackage
 import struct PackageModel.Resource
 import struct PackageModel.ToolsVersion
 import struct SPMBuildCore.BuildToolPluginInvocationResult
 import struct SPMBuildCore.BuildParameters
+import protocol SPMBuildCore.ModuleBuildDescription
 
 public enum BuildDescriptionError: Swift.Error {
     case requestedFileNotPartOfTarget(targetName: String, requestedFilePath: AbsolutePath)
@@ -25,7 +27,7 @@ public enum BuildDescriptionError: Swift.Error {
 public typealias TargetBuildDescription = ModuleBuildDescription
 
 /// A module build description which can either be for a Swift or Clang module.
-public enum ModuleBuildDescription {
+public enum ModuleBuildDescription: SPMBuildCore.ModuleBuildDescription {
     /// Swift target description.
     case swift(SwiftModuleBuildDescription)
 
@@ -73,6 +75,19 @@ public enum ModuleBuildDescription {
         }
     }
 
+    public var package: ResolvedPackage {
+        switch self {
+        case .swift(let description):
+            description.package
+        case .clang(let description):
+            description.package
+        }
+    }
+
+    public var module: ResolvedModule {
+        self.target
+    }
+
     /// Paths to the binary libraries the target depends on.
     var libraryBinaryPaths: Set<AbsolutePath> {
         switch self {
@@ -101,7 +116,7 @@ public enum ModuleBuildDescription {
         }
     }
 
-    var buildParameters: BuildParameters {
+    public var buildParameters: BuildParameters {
         switch self {
         case .swift(let buildDescription):
             return buildDescription.buildParameters
@@ -121,7 +136,7 @@ public enum ModuleBuildDescription {
 
     /// Determines the arguments needed to run `swift-symbolgraph-extract` for
     /// this module.
-    package func symbolGraphExtractArguments() throws -> [String] {
+    public func symbolGraphExtractArguments() throws -> [String] {
         switch self {
         case .swift(let buildDescription): try buildDescription.symbolGraphExtractArguments()
         case .clang(let buildDescription): try buildDescription.symbolGraphExtractArguments()

Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift

@@ -639,6 +639,11 @@ public final class SwiftModuleBuildDescription {
     /// this module.
     package func symbolGraphExtractArguments() throws -> [String] {
         var args = [String]()
+
+        args += ["-module-name", self.target.c99name]
+        args += try self.buildParameters.tripleArgs(for: self.target)
+        args += ["-module-cache-path", try self.buildParameters.moduleCache.pathString]
+
         args += try self.cxxInteroperabilityModeArguments(
             propagateFromCurrentModuleOtherSwiftFlags: true)
 

Sources/Build/BuildPlan/BuildPlan.swift

@@ -216,6 +216,10 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
         AnySequence(self.productMap.values.map { $0 as SPMBuildCore.ProductBuildDescription })
     }
 
+    public var buildModules: AnySequence<SPMBuildCore.ModuleBuildDescription> {
+        AnySequence(self.targetMap.values.map { $0 as SPMBuildCore.ModuleBuildDescription })
+    }
+
     /// The results of invoking any build tool plugins used by targets in this build.
     public let buildToolPluginInvocationResults: [ResolvedModule.ID: [BuildToolPluginInvocationResult]]
 
@@ -674,15 +678,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
         }
     }
 
-    /// Determines the arguments needed to run `swift-symbolgraph-extract` for
-    /// a particular module.
-    public func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String] {
-        guard let description = self.targetMap[module.id] else {
-            throw InternalError("Expected description for module \(module)")
-        }
-        return try description.symbolGraphExtractArguments()
-    }
-
     /// Returns the files and directories that affect the build process of this build plan.
     package var inputs: [Input] {
         var inputs: [Input] = []

Sources/Commands/PackageCommands/DumpCommands.swift

@@ -70,14 +70,14 @@ struct DumpSymbolGraph: AsyncSwiftCommand {
 
         // Run the tool once for every library and executable target in the root package.
         let buildPlan = try buildSystem.buildPlan
+        let modulesGraph = try await buildSystem.getPackageGraph()
         let symbolGraphDirectory = buildPlan.destinationBuildParameters.dataPath.appending("symbolgraph")
-        let targets = try await buildSystem.getPackageGraph().rootPackages.flatMap{ $0.modules }.filter{ $0.type == .library }
-        for target in targets {
-            print("-- Emitting symbol graph for", target.name)
+        for description in buildPlan.buildModules
+            where description.module.type == .library && modulesGraph.rootPackages[description.package.id] != nil
+        {
+            print("-- Emitting symbol graph for", description.module.name)
             let result = try symbolGraphExtractor.extractSymbolGraph(
-                module: target,
-                buildPlan: buildPlan,
-                buildParameters: buildPlan.destinationBuildParameters,
+                for: description,
                 outputRedirection: .collect(redirectStderr: true),
                 outputDirectory: symbolGraphDirectory,
                 verboseOutput: swiftCommandState.logLevel <= .info
@@ -87,9 +87,9 @@ struct DumpSymbolGraph: AsyncSwiftCommand {
                 let commandline = "\nUsing commandline: \(result.arguments)"
                 switch result.output {
                 case .success(let value):
-                    swiftCommandState.observabilityScope.emit(error: "Failed to emit symbol graph for '\(target.c99name)': \(String(decoding: value, as: UTF8.self))\(commandline)")
+                    swiftCommandState.observabilityScope.emit(error: "Failed to emit symbol graph for '\(description.module.c99name)': \(String(decoding: value, as: UTF8.self))\(commandline)")
                 case .failure(let error):
-                    swiftCommandState.observabilityScope.emit(error: "Internal error while emitting symbol graph for '\(target.c99name)': \(error)\(commandline)")
+                    swiftCommandState.observabilityScope.emit(error: "Internal error while emitting symbol graph for '\(description.module.c99name)': \(error)\(commandline)")
                 }
             }
         }

Sources/Commands/Utilities/PluginDelegate.swift

@@ -398,25 +398,34 @@ final class PluginDelegate: PluginInvocationDelegate {
             cacheBuildManifest: false
         )
 
-        // Find the target in the build operation's package graph; it's an error if we don't find it.
-        let packageGraph = try await buildSystem.getPackageGraph()
-        guard let target = packageGraph.module(for: targetName) else {
-            throw StringError("could not find a target named “\(targetName)”")
+        func lookupDescription(
+            for moduleName: String,
+            destination: BuildParameters.Destination
+        ) throws -> ModuleBuildDescription? {
+            try buildSystem.buildPlan.buildModules.first {
+                $0.module.name == moduleName && $0.buildParameters.destination == destination
+            }
         }
 
-        // FIXME: This is currently necessary because `target(for:destination:)` can
-        // produce a module that is targeting host when `targetName`` corresponds to
-        // a macro, plugin, or a test. Ideally we'd ask a build system for a`BuildSubset`
-        // and get the destination from there but there are other places that need
-        // refactoring in that way as well.
-        let buildParameters = if target.buildTriple == .tools {
-                try swiftCommandState.toolsBuildParameters
-            } else {
-                try swiftCommandState.productsBuildParameters
-            }
+        // FIXME: The name alone doesn't give us enough information to figure out what
+        // the destination is, this logic prefers "target" over "host" because that's
+        // historically how this was setup. Ideally we should be building for both "host"
+        // and "target" if module is configured for them but that would require changing
+        // `PluginInvocationSymbolGraphResult` to carry multiple directories.
+        let description: ModuleBuildDescription? = if let targetDescription = try lookupDescription(for: targetName, destination: .target) {
+            targetDescription
+        } else if let hostDescription = try lookupDescription(for: targetName, destination: .host) {
+            hostDescription
+        } else {
+            nil
+        }
+
+        guard let description else {
+            throw InternalError("could not find a target named: \(targetName)")
+        }
 
         // Build the target, if needed.
-        try await buildSystem.build(subset: .target(target.name, for: buildParameters.destination))
+        try await buildSystem.build(subset: .target(targetName, for: description.buildParameters.destination))
 
         // Configure the symbol graph extractor.
         var symbolGraphExtractor = try SymbolGraphExtract(
@@ -442,21 +451,16 @@ final class PluginDelegate: PluginInvocationDelegate {
         symbolGraphExtractor.emitExtensionBlockSymbols = options.emitExtensionBlocks
 
         // Determine the output directory, and remove any old version if it already exists.
-        guard let package = packageGraph.package(for: target) else {
-            throw StringError("could not determine the package for target “\(target.name)”")
-        }
-        let outputDir = buildParameters.dataPath.appending(
+        let outputDir = description.buildParameters.dataPath.appending(
             components: "extracted-symbols",
-            package.identity.description,
-            target.name
+            description.package.identity.description,
+            targetName
         )
         try swiftCommandState.fileSystem.removeFileTree(outputDir)
 
         // Run the symbol graph extractor on the target.
         let result = try symbolGraphExtractor.extractSymbolGraph(
-            module: target,
-            buildPlan: try buildSystem.buildPlan,
-            buildParameters: buildParameters,
+            for: description,
             outputRedirection: .collect,
             outputDirectory: outputDir,
             verboseOutput: self.swiftCommandState.logLevel <= .info

Sources/Commands/Utilities/SymbolGraphExtract.swift

@@ -54,9 +54,7 @@ package struct SymbolGraphExtract {
     /// The `outputDirection` determines how the output from the tool subprocess is handled, and `verbosity` specifies
     /// how much console output to ask the tool to emit.
     package func extractSymbolGraph(
-        module: ResolvedModule,
-        buildPlan: BuildPlan,
-        buildParameters: BuildParameters,
+        for description: ModuleBuildDescription,
         outputRedirection: AsyncProcess.OutputRedirection = .none,
         outputDirectory: AbsolutePath,
         verboseOutput: Bool
@@ -65,12 +63,9 @@ package struct SymbolGraphExtract {
 
         // Construct arguments for extracting symbols for a single target.
         var commandLine = [self.tool.pathString]
-        commandLine += try buildPlan.symbolGraphExtractArguments(for: module)
+        commandLine += try description.symbolGraphExtractArguments()
 
         // FIXME: everything here should be in symbolGraphExtractArguments
-        commandLine += ["-module-name", module.c99name]
-        commandLine += try buildParameters.tripleArgs(for: module)
-        commandLine += ["-module-cache-path", try buildParameters.moduleCache.pathString]
         if verboseOutput {
             commandLine += ["-v"]
         }
@@ -86,7 +81,11 @@ package struct SymbolGraphExtract {
         }
 
         let extensionBlockSymbolsFlag = emitExtensionBlockSymbols ? "-emit-extension-block-symbols" : "-omit-extension-block-symbols"
-        if DriverSupport.checkSupportedFrontendFlags(flags: [extensionBlockSymbolsFlag.trimmingCharacters(in: ["-"])], toolchain: buildParameters.toolchain, fileSystem: fileSystem) {
+        if DriverSupport.checkSupportedFrontendFlags(
+            flags: [extensionBlockSymbolsFlag.trimmingCharacters(in: ["-"])],
+            toolchain: description.buildParameters.toolchain,
+            fileSystem: fileSystem
+        ) {
             commandLine += [extensionBlockSymbolsFlag]
         } else {
             observabilityScope.emit(warning: "dropped \(extensionBlockSymbolsFlag) flag because it is not supported by this compiler version")

Sources/SPMBuildCore/BuildSystem/BuildSystem.swift

@@ -80,6 +80,19 @@ extension ProductBuildDescription {
     }
 }
 
+public protocol ModuleBuildDescription {
+    /// The package the module belongs to.
+    var package: ResolvedPackage { get }
+
+    /// The underlying module this description is for.
+    var module: ResolvedModule { get }
+
+    /// The build parameters.
+    var buildParameters: BuildParameters { get }
+
+    func symbolGraphExtractArguments() throws -> [String]
+}
+
 public protocol BuildPlan {
     /// Parameters used when building end products for the destination platform.
     var destinationBuildParameters: BuildParameters { get }
@@ -89,12 +102,10 @@ public protocol BuildPlan {
 
     var buildProducts: AnySequence<ProductBuildDescription> { get }
 
+    var buildModules: AnySequence<ModuleBuildDescription> { get }
+
     func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String]
     func createREPLArguments() throws -> [String]
-
-    /// Determines the arguments needed to run `swift-symbolgraph-extract` for
-    /// a particular module.
-    func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String]
 }
 
 public protocol BuildSystemFactory {

Sources/_InternalTestSupport/MockBuildTestHelper.swift

@@ -285,7 +285,7 @@ enum BuildError: Swift.Error {
 
 public struct BuildPlanResult {
     public let plan: Build.BuildPlan
-    public let targetMap: [ResolvedModule.ID: ModuleBuildDescription]
+    public let targetMap: [ResolvedModule.ID: Build.ModuleBuildDescription]
     public let productMap: [ResolvedProduct.ID: Build.ProductBuildDescription]
 
     public init(plan: Build.BuildPlan) throws {
@@ -316,7 +316,7 @@ public struct BuildPlanResult {
         XCTAssertEqual(self.plan.productMap.count, count, file: file, line: line)
     }
 
-    public func moduleBuildDescription(for name: String) throws -> ModuleBuildDescription {
+    public func moduleBuildDescription(for name: String) throws -> Build.ModuleBuildDescription {
         let matchingIDs = targetMap.keys.filter({ $0.moduleName == name })
         guard matchingIDs.count == 1, let target = targetMap[matchingIDs[0]] else {
             if matchingIDs.isEmpty {
@@ -342,7 +342,7 @@ public struct BuildPlanResult {
     }
 }
 
-extension ModuleBuildDescription {
+extension Build.ModuleBuildDescription {
     public func swift() throws -> SwiftModuleBuildDescription {
         switch self {
         case .swift(let description):