Skip to content

Compile all of the plugins at the start of a build with swiftbuild #9190

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+177 −63
Open

Compile all of the plugins at the start of a build with swiftbuild #9190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1dfe2aa
Compile all of the plugins at the start of a build with swift-build
cmcgee1024 Sep 25, 2025
5174e9b
Remove known issues that have been fixed
cmcgee1024 Sep 26, 2025
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
161 changes: 157 additions & 4 deletions Sources/SwiftBuildSupport/SwiftBuildSystem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import func TSCBasic.withTemporaryFile

import enum TSCUtility.Diagnostics

import var TSCBasic.stdoutStream

import Foundation
import SWBBuildService
import SwiftBuild
Expand Down Expand Up @@ -342,11 +344,22 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
}

public func build(subset: BuildSubset, buildOutputs: [BuildOutput]) async throws -> BuildResult {
// If any plugins are part of the build set, compile them now to surface
// any errors up-front. Returns true if we should proceed with the build
// or false if not. It will already have thrown any appropriate error.
var result = BuildResult(
serializedDiagnosticPathsByTargetName: .failure(StringError("Building was skipped")),
replArguments: nil,
)

guard !buildParameters.shouldSkipBuilding else {
return BuildResult(
serializedDiagnosticPathsByTargetName: .failure(StringError("Building was skipped")),
replArguments: nil,
)
result.serializedDiagnosticPathsByTargetName = .failure(StringError("Building was skipped"))
return result
}

guard try await self.compilePlugins(in: subset) else {
result.serializedDiagnosticPathsByTargetName = .failure(StringError("Plugin compilation failed"))
return result
}

try await writePIF(buildParameters: buildParameters)
Expand All @@ -357,6 +370,145 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
)
}

/// Compiles any plugins specified or implied by the build subset, returning
/// true if the build should proceed. Throws an error in case of failure. A
/// reason why the build might not proceed even on success is if only plugins
/// should be compiled.
func compilePlugins(in subset: BuildSubset) async throws -> Bool {
// Figure out what, if any, plugin descriptions to compile, and whether
// to continue building after that based on the subset.
let graph = try await getPackageGraph()

/// Description for a plugin module. This is treated a bit differently from the
/// regular kinds of modules, and is not included in the LLBuild description.
/// But because the modules graph and build plan are not loaded for incremental
/// builds, this information is included in the BuildDescription, and the plugin
/// modules are compiled directly.
class PluginBuildDescription: Codable {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: classes should be final by default if you're not using inheritance

Copy link
Member Author

Choose a reason for hiding this comment

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

This class is declared within the scope of this function, so I think that would limit the ability for anything external to extend it. Is it still worth declaring it final in this case?

Copy link
Member

Choose a reason for hiding this comment

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

The next question then is, why is this a class and not a value type if the lifetime is so short.

/// The identity of the package in which the plugin is defined.
public let package: PackageIdentity

/// The name of the plugin module in that package (this is also the name of
/// the plugin).
public let moduleName: String

/// The language-level module name.
public let moduleC99Name: String

/// The names of any plugin products in that package that vend the plugin
/// to other packages.
public let productNames: [String]

/// The tools version of the package that declared the module. This affects
/// the API that is available in the PackagePlugin module.
public let toolsVersion: ToolsVersion

/// Swift source files that comprise the plugin.
public let sources: Sources

/// Initialize a new plugin module description. The module is expected to be
/// a `PluginTarget`.
init(
module: ResolvedModule,
products: [ResolvedProduct],
package: ResolvedPackage,
toolsVersion: ToolsVersion,
testDiscoveryTarget: Bool = false,
fileSystem: FileSystem
) throws {
guard module.underlying is PluginModule else {
throw InternalError("underlying target type mismatch \(module)")
}

self.package = package.identity
self.moduleName = module.name
self.moduleC99Name = module.c99name
self.productNames = products.map(\.name)
self.toolsVersion = toolsVersion
self.sources = module.sources
}
}

var allPlugins: [PluginBuildDescription] = []

for pluginModule in graph.allModules.filter({ ($0.underlying as? PluginModule) != nil }) {
guard let package = graph.package(for: pluginModule) else {
throw InternalError("Package not found for module: \(pluginModule.name)")
}

let toolsVersion = package.manifest.toolsVersion

let pluginProducts = package.products.filter { $0.modules.contains(id: pluginModule.id) }

allPlugins.append(try PluginBuildDescription(
module: pluginModule,
products: pluginProducts,
package: package,
toolsVersion: toolsVersion,
fileSystem: fileSystem
))
}

let pluginsToCompile: [PluginBuildDescription]
let continueBuilding: Bool
switch subset {
case .allExcludingTests, .allIncludingTests:
pluginsToCompile = allPlugins
continueBuilding = true
case .product(let productName, _):
pluginsToCompile = allPlugins.filter{ $0.productNames.contains(productName) }
continueBuilding = pluginsToCompile.isEmpty
case .target(let targetName, _):
pluginsToCompile = allPlugins.filter{ $0.moduleName == targetName }
continueBuilding = pluginsToCompile.isEmpty
}

final class Delegate: PluginScriptCompilerDelegate {
var failed: Bool = false
var observabilityScope: ObservabilityScope

public init(observabilityScope: ObservabilityScope) {
self.observabilityScope = observabilityScope
}

func willCompilePlugin(commandLine: [String], environment: [String: String]) { }
func didCompilePlugin(result: PluginCompilationResult) {
if !result.compilerOutput.isEmpty && !result.succeeded {
print(result.compilerOutput, to: &stdoutStream)
} else if !result.compilerOutput.isEmpty {
observabilityScope.emit(info: result.compilerOutput)
}

failed = !result.succeeded
}

func skippedCompilingPlugin(cachedResult: PluginCompilationResult) { }
}

// Compile any plugins we ended up with. If any of them fails, it will
// throw.
for plugin in pluginsToCompile {
let delegate = Delegate(observabilityScope: observabilityScope)

_ = try await self.pluginConfiguration.scriptRunner.compilePluginScript(
sourceFiles: plugin.sources.paths,
pluginName: plugin.moduleName,
toolsVersion: plugin.toolsVersion,
observabilityScope: observabilityScope,
callbackQueue: DispatchQueue.sharedConcurrent,
delegate: delegate
)

if delegate.failed {
throw Diagnostics.fatalError
}
}

// If we get this far they all succeeded. Return whether to continue the
// build, based on the subset.
return continueBuilding
}

private func startSWBuildOperation(
pifTargetName: String,
buildOutputs: [BuildOutput]
Expand All @@ -371,6 +523,7 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
continue
}
}

var replArguments: CLIArguments?
var artifacts: [(String, PluginInvocationBuildResult.BuiltArtifact)]?
return try await withService(connectionMode: .inProcessStatic(swiftbuildServiceEntryPoint)) { service in
Expand Down
79 changes: 20 additions & 59 deletions Tests/CommandsTests/PackageCommandTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6831,10 +6831,6 @@ struct PackageCommandTests {
}

@Test(
.issue(
"https://github.com/swiftlang/swift-package-manager/issues/8977",
relationship: .defect
),
.requiresSwiftConcurrencySupport,
.IssueWindowsLongPath,
.tags(
Expand Down Expand Up @@ -6961,28 +6957,31 @@ struct PackageCommandTests {
ProcessInfo.hostOperatingSystem == .windows && data.buildSystem == .swiftbuild
}

try await withKnownIssue {
// Check that building just one of them just compiles that plugin and doesn't build anything else.
do {
let (stdout, stderr) = try await executeSwiftBuild(
packageDir,
configuration: data.config,
extraArgs: ["--target", "MyCommandPlugin"],
buildSystem: data.buildSystem,
)
if data.buildSystem == .native {
#expect(!stdout.contains("Compiling plugin MyBuildToolPlugin"), "stderr: \(stderr)")
#expect(stdout.contains("Compiling plugin MyCommandPlugin"), "stderr: \(stderr)")
}
#expect(!stdout.contains("Building for \(data.config.buildFor)..."), "stderr: \(stderr)")
// Check that building just one of them just compiles that plugin and doesn't build anything else.
do {
let (stdout, stderr) = try await executeSwiftBuild(
packageDir,
configuration: data.config,
extraArgs: ["--target", "MyCommandPlugin"],
buildSystem: data.buildSystem,
)
if data.buildSystem == .native {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: instead of an if block, can we use a switch block , and explicitly call out we don't expect any output from SwiftBuild? e.g.:

switch data.buildSystem {
   case .native:
        #expect(!stdout.contains("Compiling plugin MyBuildToolPlugin"), "stderr: \(stderr)")
        #expect(stdout.contains("Compiling plugin MyCommandPlugin"), "stderr: \(stderr)")
    case .swiftbuild:
       // nothing specific
       break
     case .xcode:
         Issue.record("Test expected have not been considered")
}

#expect(!stdout.contains("Compiling plugin MyBuildToolPlugin"), "stderr: \(stderr)")
#expect(stdout.contains("Compiling plugin MyCommandPlugin"), "stderr: \(stderr)")
}
} when: {
data.buildSystem == .swiftbuild
#expect(!stdout.contains("Building for \(data.config.buildFor)..."), "stderr: \(stderr)")
}
}
}

private static func commandPluginCompilationErrorImplementation(
@Test(
.requiresSwiftConcurrencySupport,
.tags(
.Feature.Command.Package.CommandPlugin,
),
arguments: getBuildData(for: SupportedBuildSystemOnAllPlatforms),
)
func commandPluginCompilationErrorImplementation(
data: BuildData,
) async throws {
try await fixture(name: "Miscellaneous/Plugins/CommandPluginCompilationError") { packageDir in
Expand Down Expand Up @@ -7017,44 +7016,6 @@ struct PackageCommandTests {
}
}

@Test(
.requiresSwiftConcurrencySupport,
.tags(
.Feature.Command.Package.CommandPlugin,
),
// arguments: getBuildData(for: SupportedBuildSystemOnAllPlatforms),
arguments: getBuildData(for: [.native]),
)
func commandPluginCompilationError(
data: BuildData,
) async throws {
try await Self.commandPluginCompilationErrorImplementation(data: data)
}

@Test(
.disabled("the swift-build process currently has a fatal error"),
.issue(
"https://github.com/swiftlang/swift-package-manager/issues/8977",
relationship: .defect
),
.SWBINTTODO("Building sample package causes a backtrace on linux"),
.requireSwift6_2,
.requiresSwiftConcurrencySupport,
.tags(
.Feature.Command.Package.CommandPlugin,
),
// arguments: getBuildData(for: SupportedBuildSystemOnAllPlatforms),
arguments: getBuildData(for: [.swiftbuild]),
)
func commandPluginCompilationErrorSwiftBuild(
data: BuildData,
) async throws {
// Once this is fix, merge data iunto commandPluginCompilationError
await withKnownIssue {
try await Self.commandPluginCompilationErrorImplementation(data: data)
}
}

@Test(
.requiresSwiftConcurrencySupport,
.tags(
Expand Down