Wire up the plugin script runner to the PIF Builder for build tool plugins #8728
Conversation
Add a plugin test case to verify plugins with swift build build system.
cmcgee1024
force-pushed
the
build_tool_plugins
branch
from
12768d5 to
561322a
Compare
…ager into build_tool_plugins
cmcgee1024
changed the title
|
@swift-ci please test |
| @@ -130,9 +202,51 @@ public final class PIFBuilder { | |||
|
|
|||
| private var cachedPIF: PIF.TopLevelObject? | |||
|
|
|||
| private func buildPluginTools( | |||
There was a problem hiding this comment.
nit: I find this name a little confusing since this function is not actually performing the build of the tools, might be worth a comment
|
@swift-ci test macOS |
|
@swift-ci test Windows |
cmcgee1024
changed the title
|
@swift-ci please test |
cmcgee1024
requested review from
jakepetroules,
dschaefer2,
shawnhyam,
bripeticca,
plemarquand,
bkhouri and
daveyc123
as code owners
|
@swift-ci test Linux |
|
@swift-ci test Windows |
| import func TSCBasic.topologicalSort | ||
| import var TSCBasic.stdoutStream | ||
|
|
||
| import enum SwiftBuild.ProjectModel | ||
|
|
||
| public func memoize<T>(to cache: inout T?, build: () async throws -> T) async rethrows -> T { |
There was a problem hiding this comment.
Does this function really need to be public?
| pluginScriptRunner: PluginScriptRunner, | ||
| pluginWorkingDirectory: AbsolutePath, | ||
| pkgConfigDirectories: [Basics.AbsolutePath], | ||
| additionalFileRules: [FileRuleDescription] | ||
| ) { |
There was a problem hiding this comment.
Consider moving pluginWorkingDirectory, pkgConfigDirectories, and additionalFileRules to PIFBuilderParameters instead. Assuming that doesn't wreak havoc on the calling code ;)
PS. The PluginScriptRunner seems like should be passed on the init directly as you just did.
| observabilityScope: observabilityScope | ||
| ) | ||
| let pluginDerivedResources = derivedResources | ||
| derivedSources.forEach { absPath in |
There was a problem hiding this comment.
Nitpick: According to the code review we did in Swift Build, the Swift folks suggested we use plain for loops for such cases.
| } | ||
|
|
||
| extension ModulesGraph { | ||
| public static func computePluginGeneratedFiles( |
There was a problem hiding this comment.
Can't this extension be fileprivate instead?
|
|
||
| let pluginWorkingDirectory: AbsolutePath | ||
|
|
||
| let pkgConfigDirectories: [Basics.AbsolutePath] |
|
|
||
| // Add a BuildToolPluginInvocationResult to the mapping. | ||
| buildToolPluginResults.append(result2) | ||
| buildtoolPluginResults[module.name] = result2 |
There was a problem hiding this comment.
Nit: the close naming here seems tricky to parse 😬.
Maybe rename buildtoolPluginResults to buildToolPluginResultsByModuleName?
| @@ -241,16 +274,14 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem { | |||
| } | |||
|
|
|||
| let pifBuilder = try await getPIFBuilder() | |||
| let pif = try pifBuilder.generatePIF( | |||
| let pif = try await pifBuilder.generatePIF( | |||
There was a problem hiding this comment.
Praise: Great to see this code path slowly becoming async! 👏
This will activate build tool plugins and enables more plugin testing with the new build system.