Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by email or DM. When contacting the review manager directly, please put "SE-0542" in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at:
I am generally in favour of conditions on plugin usages. One thing that we need to consider is that plugins purposefully run in a context that is independent of the build request, which is also true for the PIF. As a result it can not know the target platform. The idea being we build a single PIF and reuse it for multiple build requests that may have different target platforms. We are reserving this capability for a time we have a long running SwiftPM that manages updates to the model independently of the build requests.
Conditionals on host platform make sense since the build plugin tools may have platform restrictions and they are guaranteed to run on the host.
I am also working on a way to pass properties of the build request to build plugin tools since they do run in the context of a build so the can do the right thing based on the target platform amongst other things. You can see that in my External Packages prototype which currently passes these things to the CustomTask in the environment variables. [Prototype] External Packages by dschaefer2 · Pull Request #10198 · swiftlang/swift-package-manager · GitHub
I support this proposal, with one caveat. I think the proposal does a good job of explaining why allowing conditions on plugins is useful and improves the ergonomics of working with packages that use build tool plugins. I also agree that it's useful to write conditions based on both the host and target platforms. However, I'm not sure I agree with the decision to introduce a new dedicated PluginUsageCondition type rather than reusing TargetDependencyCondition. The proposal considers this as an alternative:
Instead of introducing PluginUsageCondition, we could reuse TargetDependencyCondition. This would reduce API surface, but it conflates two different concepts: a dependency that is linked into the build product, and a plugin that runs during the build process. More concretely, TargetDependencyCondition.when(platforms:) is already shipped and matches against the target platform — for target dependencies, "which platform will the product run on?" is the natural question. Plugins need both host and target filters, and adding a hostPlatforms: parameter to the existing condition type would force every reader of a manifest to remember that bare platforms: happens to mean target on that type. Keeping the condition types separate contains the label meanings to where they are unambiguous.
Separate types also leave room for the APIs to evolve independently — for example, a future configuration condition might make sense for plugins (skip linting in release builds) but not for dependencies.
I don't agree that this would conflate two separate concepts. While the manifest splits them into two separate lists, plugins and dependencies of a target declaration are still both representing build time dependencies, they're just different types of dependencies. In my opinion we should default to treating them the same unless there's a strong motivation not to. The proposal does call out that the existing when(platforms:) is not as clear as it could be in distinguishing whether it applies to the host or target platform. I'm not convinced that this potential confusion justifies introducing a whole new condition type though. If we introduced TargetDependencyCondition.when(hostPlatforms:) alongside TargetDependencyCondition.when(platforms:), I think it would probably be clear to most users that the bare platforms refers to the target platform. The second point the proposal makes is that introducing a new type allows plugin and "regular" target conditions to evolve independently, using build configuration conditions as a hypothetical example. I think it's far more likely that any future conditions should support both types of dependency, which is IMO another reason to use the same type to represent both.
Minor nit, but I think still relevant since proposals live forever, could the title be a little clearer? At least my first association was that we're getting a new type of plugin centered around conditionals.
My suggestion would be "Package Manager Plugin Usage Conditionals".
I guess my comment above is more about the associated PR. We should be able to run plugins independently of the build request. I am planning architecture changes that move plugin invocation closer to manifest load time when creating the package graph so we can properly model the source and resource files that plugins generate.
We can still support target conditionals on plugins but they should really be modelled as target conditions on the Commands that the plugins generate. In fact a plugin should be able to create different commands with different conditionals. The plugin level condition would apply to all the Commands. I'm not sure how difficult it would be to add that to the CustomTasks in the PIF that Commands turn into with Swift Build, but it would definitely be worth exploring.
I also agree with Owen's comment. These really are target dependencies so should have the same conditionals. In fact plugin usages end up being lumped together with the rest of a target's dependencies internally in the package graph. They should have the same conditionals.
Generally, we need to have such conditionals on everything in the package graph. We've even had requests to support conditionals on source files and on targets themselves. But to keep the ergonomics clean, we should be careful when adding these in that there is some unity to how conditions are specified so the user doesn't need to learn them all.
Also a question on target conditional. If a plugin doesn't run because it's an unsupported target platform, wouldn't the compile of the target fail because of the missing files that the plugin would have generated? I wonder if this is maybe a better case for the target itself having the conditional. Or if your plugin needs to use a different generator, whether that's better expressed with a conditional on the Commands the plugin generates.
I also want to make readers are clear since I had the same problem when I started out working on SwiftPM, you do need to differentiate the build tool plugin, sometimes called the plugin script that returns the list of Commands, and the build plugin tool, the tools that the Commands invoke, often executable targets in the package. Plugin scripts are supposed to be fast and are run sandboxed for security. It's really the tools that have the platform constraints, i.e. the generators and in the future other build tools.
Which brings up another question. You use linting as an example. I thought those were using managed with command plugins, which probably also should have platform conditions. I'm not sure they're a natural fit for build tool plugins since they don't have outputs that feed into the build graph.
and rinse and repeat for every new platform I try to support, I could just:
condition: .not(platforms: [.anyAppleOS])),
Sidenote, could we have that? Due to the runtime differences between Swift on Apple platforms and Swift elsewhere, it's not uncommon for me to have dependencies that are specific to Apple or non-Apple platforms, and there's no concise way to express the latter.
Here is the revised proposal. The first commit renames the file. The second commit updates its contents, so you can review the content diff separately.
I can see how the original name was confusing. I changed it to Package Manager Build Tool Plugin Usage Conditionals to make the scope clear.
I also changed the proposal to reuse TargetDependencyCondition. My original concerns were mainly about the API design, but I do not have a strong objection to reusing the existing type. Future conditions should apply to both target dependencies and plugin usages, so a shared type also helps keep the APIs consistent.
I clarified the target-platform behavior based on Doug's comments. For a target-platform condition, SwiftPM invokes the plugin and applies the condition on the plugin usage to all commands and declared outputs that the plugin returns. I added author-defined conditions on individual commands as a future direction. I think that feature deserves its own discussion, so I kept this proposal focused on the consumer-side condition.
SwiftLint does have a build tool plugin, and I think linting is a valid use of a build tool plugin. A build tool plugin does not necessarily have to generate files.
SwiftPM applies the target-platform filter to build commands and to the generated outputs from the plugin usage. A target-platform filter does not guarantee that a prebuild command will not execute. PIF-based builds run prebuild commands while generating the request-independent PIF so that SwiftPM can discover their outputs. SwiftPM applies the filter to those discovered outputs.
First, I think the proposal introduces a confusing inconsistency between built tool plugins and prebuild plugins:
Build tool plugins with a target platform condition would always be invoked to plan tasks to be added to the build graph, but those tasks would only run when the condition is met. I think this is a reasonable design, as the plugin scripts are meant to be extremely lightweight, and it allows supporting builds with multiple destination platforms.
Prebuild plugins with a target platform condition would always be invoked to plan tasks, and those tasks would always be executed, but the outputs of those tasks would only be consumed when the condition is met. This constraint simplifies the implementation, but I think it's the wrong design. It introduces inconsistency with built tool plugins, and I think it's likely to confuse users who aren't already familiar with the implementation.
IMO, it would be acceptable to ban use of target platform conditions on prebuild plugins for now if that allows making incremental improvements in this area. Formalizing the wrong behavior now would make it harder to introduce the desired semantics in the future as the implementation evolves.
Second, I think the updated proposal describes more implementation details than I'd usually want to commit to in an evolution proposal. The detailed design section describes how a plugin is applied:
Evaluate the host and traits. SwiftPM compares hostPlatforms with the host platform. It compares traits with the enabled traits. If a filter does not match, SwiftPM does not invoke the build tool plugin.
Invoke the plugin without a target platform. The build tool plugin returns its commands in a context that does not depend on a build request. SwiftPM does not give the target platform to the build tool plugin.
Filter commands and outputs. SwiftPM applies the platforms filter to each build command from this plugin usage. Build commands become filtered CustomTask values in the PIF. Generated source and resource entries, including files discovered from prebuild commands, use the same filter. The build system selects the commands and generated files for each configured target. A target-platform filter does not guarantee that a prebuild command will not execute. PIF generation has no build request or configured target, and SwiftPM must run prebuild commands to discover the contents of their output directories. For PIF-based builds, SwiftPM therefore runs each applicable prebuild command once during PIF generation and applies the target-platform filter to the discovered outputs. Package authors must use hostPlatforms when a prebuild tool cannot run on a particular host.
Keep dependency resolution unchanged. SwiftPM still resolves and fetches the package dependency. This behavior is consistent with SE-0273.
Handle binary artifacts. SwiftPM does not run the build tool when hostPlatforms excludes the current host. Without this condition, the tool can cause a build failure.
I think it's important to avoid referring to implementations details like PIF and specific model objects like CustomTask when describing how the feature will work, as we'll inevitably want to make future changes and improvements. It's also not clear to me how point 5 relates to binary artifacts.
Thanks! I did mention A target-platform filter does not guarantee that a prebuild command will not execute to indicate that the behavior can change in the future, but I can see where the confusing came from. I've updated the wording and added a note in Future directions.
As for banning the use of target platform conditions on prebuild plugins, I'm on the fence about whether we should fail the build or still allow execution if target platform conditions was specified. I'm more leaning towards allowing execution and maybe throw a warning saying that this doesn't work as intended today, so that consumer can get the benefit without any change once the incremental improvements was delivered. Failing the build would be more disruptive, and problematic if the condition was specified on dependency packages.
Changing the behavior would likely only be possible in a new tools-version, requiring us to continue to support the suboptimal behavior in older versions. IMO we shouldn't ship this part of the feature if we don't think it's the right long-term approach.
One thing I’m not sure about is whether we want to ban target-platform conditions on prebuild plugins for all paths or just the PIF path. We have the target-platform information in the native build-system path, but not during PIF generation. I can see pros and cons to both.