1

Hello,

I'd like to propose amending SE-0303 so that SwiftPM plugins use @main instead of top-level code for entry points. While it's a little more verbose, this would allow customized entry points for each kind of plugin, and would also make it clearer what the inputs and expected outputs of each plugin are.

I would love to hear any thoughts and opinions on this approach.

Thanks!

Anders

20 Likes
2

+1 Seems like a really good amendment that should make writing plugins more straightforward.

2 Likes
3

+1. This new design seems much nicer.

One question: should the run function perhaps be async?

From SE-0303:

It is a future goal to allow package plugin scripts themselves to depend on custom libraries

So in the future, scripts themselves may depend on other libraries, and those libraries might only offer async APIs.

7 Likes
4

That's a good idea. Since that would require a new proposal to allow plugin scripts to depend on libraries, I'm wondering if it would be better to add async versions of the APIs at that time, though? The switch to using a protocol and @main should make it easier to introduce new versions of APIs (tied to the tools version).

5

Foundation is also adding async APIs, so it would be useful even for scripts which don’t use external libraries.

1 Like
6

Looks like an improvement! Specific comments follow.

The Diagnostics type

Other than it being in a global, I kind of prefer the old DiagnosticsEmitter design with a protocol containing instance methods, rather than an empty concrete type containing static methods. Because it was just a protocol, you could have written a mock DiagnosticsEmitter for testing; that capability seems to have been lost here. Is the new design really an improvement over tucking a DiagnosticsEmitter (possibly renamed) into a PluginContext property?

On the other hand, if you do keep it as a set of static methods grouped together in a type, shouldn't that type be an uninhabited enum instead of a struct so that the compiler can correctly reason about the fact that there aren't any instances of it?

The BuildToolPlugin protocol

If run(context:target:) is going to be an instance method, you will need an initializer in your protocol so that its main() can create the instance.

But as long as you're adding an initializer, I might think about defining the protocol with context and perhaps target as properties so that they don't have to be passed around quite as much between methods of the plugin instance. In either case, the memberwise initializer should normally save users from having to write an initializer by hand.

BuildToolPlugin.run()

Is run only executed once per instance, or could it be executed several times on a single instance, perhaps with a different target each time? This was not really an open question with main.swift, but since it's now a method, users might wonder what the lifecycle is.

The name run seems unnecessarily nondescript—why not name it something like makeBuildCommands that describes what the method needs to accomplish? And once you've done that, would it make sense to split prebuild commands into a separate makePrebuildCommands method? This would eliminate mixing of prebuild and build commands.

Renaming run would open the possibility that, once there are additional plugin capabilities in the future, a single plugin could implement several different capabilities by implementing several different methods. But this would have knock-on effects on the rest of the design—to give some obvious examples, BuildToolPlugin might be better off with a different name and Target.plugin should probably take an array of PluginCapability instances instead of just one (or it should assume all plugins have all capabilities and plugins will just use default no-op implementations of the methods for capabilities they don't want to provide). Does that sound like a good idea to you? If so, what else needs to change?

Nits

You say CommandCreator at one point where I believe you mean CommandConstructor.

I'd like to see a full listing of the PluginContext type somewhere in the proposal body; you list a bunch of declarations that have been removed and show one that was added, but I had to refer back to the original and do some mental gymnastics to get a good picture of this type.

1 Like
7

That's a good point. Unfortunately I think it would mean that plugins would also be usable only on macOS 12 or later (when I try to use async I get an error that I need a minimum deployment target of 12).

8

Well, you'll need a 5.5 toolchain in any case. Whether or not you can run an async function on older OSes using a downloaded/self-built toolchain depends on how back-deployment goes, I suppose.

9

Thanks for clarifying. Since plugins require a main toolchain (which will eventually be 5.6?) the compilation part won't be a problem — the main concern was about being able to run plugin scripts on older OSes. I will check up on back-deployment. Thanks!

2 Likes
10

That's a very good point. Adding a factory type to the context as you suggest would make sense and allow the global to be removed while still allowing those methods to be instance methods. I'll refine the pitch proposal.

The intent with the proposed Diagnostics change in particular was to simplify the verbiage a bit. But your point about the static methods points out a limitation I hadn't considered.

That's a good point, and that was an oversight. In my view your suggestion of a context property is probably better, though.

This is something I considered, but since we know that we want to allow plugin configuration in the future, it seemed to me to make sense to reserve those properties for declared parameters of the plugin.

In short it seemed clearer to say that the function parameters are the inputs from SwiftPM, the return values are the outputs to SwiftPM, and then (in a future version) the instance's properties are the parameters of the plugin (with values provided by the invocation). Perhaps we could then also use attributes to let the plugin declare its configurable properties, in the manner of SwiftArgumentParser.

The intent was to have a separate plugin invocation for each target, i.e. the same as described SE-0303, to prevent leaking of state from one invocation to the other in the same process. It might certainly be more effficient to invoke it repeatedly, but that seemed like a larger change than the API refinements proposed here.

For example, SE-0303 says that if a target uses multiple build tool plugins, they are applied in the order in which they are listed. If two targets list the same two plugins but in opposite order, that would mean that at least in some cases there would need to be multiple plugin invocations with one target each. That might be a good future optimization but it seems as if it can get a bit subtle.

That makes sense, though in this particular case I don't think that it's necessarily desirable to separate out the build commands from prebuild commands (ideally we wouldn't even need prebuild commands, if regular commands in the build system could generate new work that wasn't known until the command ran, but that isn't something that will be addressed soon).

But I see your broader point. It's worth considering whether more varied names would be better here, especially since it would allow multiple capabilities in a single plugin.

Meanwhile, I had actually revised the proposal to get away from the CommandConstructor approach to instead return different kinds of commands via an enum, which seems a little cleaner.

Allowing any plugin to provide any capability has a conceptual appeal (essentially getting rid of the predeclared capabilities). The reason the capability declaration was added to the manifest was so that SwiftPM will know when to invoke the plugin, without having to invoke it just to find out. But allowing multiple capabilities in the same declaration would work, without having a lot of plugin invocations to happen "just in case".

And if the plugin can be statically analyzed to determine what protocols it implements, then that might be a way to avoid having to declare specific capabilities in the first place.

Thanks for pointing out that error!

Good point. Some of the earlier feedback was to focus on only the differences, but seeing the end result would be important here. I'll add that back in.

Thanks a lot for your feedback! A lot of things to consider there...

2 Likes
11

I think what I'm going to do as far as a pitch for an amendment for SE-0303 is to pare this back to focus only on the @main part, and not on taking that opportunity to also add more package model properties "for future use". Putting that into a new pitch and proposal should make it it's easier to discuss separately.

1 Like
12

The proposal for the more limited form of amendment is up at [Amendment] SE-0303: Package Manager Extensible Build Tools. To the extent that additional input information is needed for future plugins, those proposals will extend the context as needed.

Many thanks for the feedback in this thread!

2 Likes
13

Maybe it was a little off-topic. But would you please let me know how can I try the plugin API with a swift-5.5 toolchain?

I add the SWIFTPM_ENABLE_PLUGINS=1, but Xcode just gives me another error, saying No such module 'PackagePlugin'

See more info here How to set SWIFTPM_ENABLE_PLUGINS=1 in Xcode

14

You can't, plugins are only available in Swift 5.6 and later. If you check the original proposal, SE-0303, it indicates what version it was implemented in:

  • Status: Implemented in 5.6
15

Oh, my fault. Did not notice it since I saw I can turn SWIFTPM_ENABLE_PLUGINS on to enable the plugin feature.

But I also notice PackagePlugin's existence in the swift-5.5.5 toolchain's /usr/lib/swift/pm/PluginAPI/PackagePlugin.swiftmodule (the same position in swift-5.6-dev toolchain) which gave me false hope :joy:

16

Using the nightly swift-5.6 toolchain. Still not working in Xcode (Working if run it on terminal with swift build)

How can I inform Xcode to use swift-5.6. It seems not working if I only set the toolchain in Xcode. Is there some flag I need to set?

截屏2022-01-12 13.18.19
截屏2022-01-12 13.18.192476×810 189 KB

17

It’s not possible to replace Xcode’s SPM integration with a toolchain.

1 Like
18

Got it. Maybe I'll have to use AppCode for swift-5.6 now. It's a pity that Xcode won't let use custom toolchain for SPM integration.

Also for anyone interested the flag to work on Terminal is export TOOLCHAINS=swift

19

AppCode & VScode work perfectly for the Plugin API introduced in swift-5.6 (Though there is no autocomplete)
Hoping a new Xcode beta with swift-5.6 will release soon. :grin:

20

Some old plugins are written in swift-5.6-dev and use the old signature, and many depends on the TargetBuildContext.inputFiles property. (eg. swift-distributed-actors/Plugin.swift at cd8daf32c5648c77123698a1b77d2703f40759d8 · apple/swift-distributed-actors · GitHub and godot-swift/main.swift at 8ed90d1956ccb8a4104a802683bb15bba63a0a7b · kelvin13/godot-swift · GitHub)

Some API in the release form is easy to be converted, like TargetBuildContext.targetDirectory to Target.directory
However I did not find a corresponding release API to get the old TargetBuildContext.inputFiles property.

Any suggestion on this?

// Original Proposal API
func createBuildCommands(context: TargetBuildContext) throws -> [Command]

// Released API
func createBuildCommands(context: PluginContext, target: Target) async throws -> [Command]

Update

After looking at the relavent MR in the GitHub repo, I find the example plugin and the corresponding new usage.

Maybe we should reflect/update this change in SE-0303? cc @abertelrud