-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci please test |
/// 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 { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@swift-ci please test |
@swift-ci test Windows |
extraArgs: ["--target", "MyCommandPlugin"], | ||
buildSystem: data.buildSystem, | ||
) | ||
if data.buildSystem == .native { |
There was a problem hiding this comment.
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")
}
When building with the swiftbuild build system there can be a compilation problem with the
plugin.swift with one of the plugins. However, you won't discover that unless you run the
command plugin specifically. This is not the behaviour of the existing native build system
that does a plugin pass at the start of a build.
Implement a similar plugin pass with the swiftbuild system to catch problems with the plugin
code when building the packages.
Fixes #8977