Skip to content

[Build/Commands] Switch SymbolGraphExtract to use build plan #7863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}
Expand Down
21 changes: 18 additions & 3 deletions Sources/Build/BuildDescription/ModuleBuildDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -101,7 +116,7 @@ public enum ModuleBuildDescription {
}
}

var buildParameters: BuildParameters {
public var buildParameters: BuildParameters {
switch self {
case .swift(let buildDescription):
return buildDescription.buildParameters
Expand All @@ -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] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it’s a witness to a public protocol requirement now

switch self {
case .swift(let buildDescription): try buildDescription.symbolGraphExtractArguments()
case .clang(let buildDescription): try buildDescription.symbolGraphExtractArguments()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
13 changes: 4 additions & 9 deletions Sources/Build/BuildPlan/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
AnySequence(self.productMap.values.map { $0 as SPMBuildCore.ProductBuildDescription })
}

public var buildModules: AnySequence<SPMBuildCore.ModuleBuildDescription> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other member uses AnySequence so I used the same for consistency.

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]]

Expand Down Expand Up @@ -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] = []
Expand Down
20 changes: 12 additions & 8 deletions Sources/Commands/PackageCommands/DumpCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,18 @@ 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 {
guard description.module.type == .library,
modulesGraph.rootPackages[description.package.id] != nil
else {
continue
}

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
Expand All @@ -87,9 +91,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)")
}
}
}
Expand Down
52 changes: 26 additions & 26 deletions Sources/Commands/Utilities/PluginDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -398,25 +398,30 @@ 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)”")
}

// 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
func lookupDescription(
for moduleName: String,
destination: BuildParameters.Destination
) throws -> ModuleBuildDescription? {
try buildSystem.buildPlan.buildModules.first {
$0.module.name == moduleName && $0.buildParameters.destination == destination
}
}

// Build the target, if needed.
try await buildSystem.build(subset: .target(target.name, for: buildParameters.destination))
// Build the target, if needed. This would also create a build plan.
try await buildSystem.build(subset: .target(targetName))

// 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 = if let targetDescription = try lookupDescription(for: targetName, destination: .target) {
targetDescription
} else if let hostDescription = try lookupDescription(for: targetName, destination: .host) {
hostDescription
} else {
throw InternalError("could not find a target named: \(targetName)")
}

// Configure the symbol graph extractor.
var symbolGraphExtractor = try SymbolGraphExtract(
Expand All @@ -442,21 +447,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
Expand Down
15 changes: 7 additions & 8 deletions Sources/Commands/Utilities/SymbolGraphExtract.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"]
}
Expand All @@ -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")
Expand Down
23 changes: 19 additions & 4 deletions Sources/SPMBuildCore/BuildSystem/BuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,23 @@ 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 }

/// FIXME: This shouldn't be necessary and ideally
/// there should be a way to ask build system to
/// introduce these arguments while building for symbol
/// graph extraction.
func symbolGraphExtractArguments() throws -> [String]
}

public protocol BuildPlan {
/// Parameters used when building end products for the destination platform.
var destinationBuildParameters: BuildParameters { get }
Expand All @@ -89,12 +106,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 {
Expand Down
6 changes: 3 additions & 3 deletions Sources/_InternalTestSupport/MockBuildTestHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -342,7 +342,7 @@ public struct BuildPlanResult {
}
}

extension ModuleBuildDescription {
extension Build.ModuleBuildDescription {
public func swift() throws -> SwiftModuleBuildDescription {
switch self {
case .swift(let description):
Expand Down