Skip to content
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

Add templates for build tool plugins and command plugins #6111

Merged
merged 10 commits into from
Mar 29, 2023

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Feb 2, 2023

Adds command plugin and build tool plugin templates to swift package init.

rdar://107288352

@neonichu neonichu self-assigned this Feb 2, 2023
@abertelrud abertelrud self-assigned this Mar 27, 2023
@abertelrud abertelrud force-pushed the add-cmd-plugin-template branch from 95eebe3 to fb4fbaa Compare March 27, 2023 20:46
@abertelrud
Copy link
Contributor

Rebased on main and resolved conflicts with the changes to add template for compiler macros.

… in #6250

The loss of this newline seems to have been unintentional and changed the behaviour from what it was in SwiftPM 5.8.  This commit restores it.
@tomerd
Copy link
Contributor

tomerd commented Mar 27, 2023

@abertelrud is this ready for review, or still WIP?

neonichu and others added 5 commits March 27, 2023 15:54
This is fairly minimal at the moment, and doesn't try to be a tutorial on how to write command plugins.  Needs a unit test.

rdar://107288352
…n packages

Most often these plugins are vended to other packages using products, so it makes sense to include this from the start.
@abertelrud abertelrud force-pushed the add-cmd-plugin-template branch from fb4fbaa to 1e0d950 Compare March 27, 2023 23:09
@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

@abertelrud is this ready for review, or still WIP?

Just pushed an update that makes it no longer WIP. I haven't yet changed it over to use associated types for the enum, but will do that separately, ideally as a follow-on to this PR after applying other feedback.

@abertelrud abertelrud changed the title WIP: Add a template for a command plugin Add templates for build tool plugins and command plugins Mar 27, 2023
@abertelrud
Copy link
Contributor

I'm thinking that this should also add the #if XcodeProjectPlugin that's needed?

@abertelrud abertelrud marked this pull request as ready for review March 27, 2023 23:13
struct \(typeName): BuildToolPlugin {
func createBuildCommands(context: PluginContext, target: Target) async throws -> [Command] {
print("Hello, World!")
return []
Copy link
Contributor

Choose a reason for hiding this comment

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

A more full-fledged starting point would be to have a shared function between the SwiftPM and Xcode API, and some boilerplate to construct a sample build command. Not sure how far to go in that direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having a sample build command would be good.

A bit less sure about how to handle Xcode vs. SwiftPM plugins, on the one hand, shared function is obviously the right thing for many use cases, on the other hand it could potentially make it confusing that the two entry points even exist?

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @bitjammer @TimTr for opinions on the content of these templates

Copy link
Contributor

Choose a reason for hiding this comment

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

In the long run, it might make sense to have an option for whether or not to add the Xcode specifics. We can leave it out for now but maybe include the URL of the plugins Markdown here in the SwiftPM repository, which talks about how to add them. That would seem to be in line with how the argument parser template works (it puts in this comment: // https://swiftpackageindex.com/apple/swift-argument-parser/documentation).

Copy link

Choose a reason for hiding this comment

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

@tomerd good point that these seem to be using the older style of always having a /Sources/<target>/ folder structure. With the updated templates for executables using just /Sources/ the base templates can start a bit cleaner. Also note this PR: #6294 which makes the SwiftPM folder search path work with /Sources/ by default (no need for a path: entry), and also work with the traditional per-target folder model. So either option is now supported by default.

I don't have a strong opinion for these templates using the traditional or simplified model. But if the common case will be a single target, may be nice to stick with the simpler approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Simplified model sounds good given that the manifest doesn't have to be made more complicated. I'll update the PR. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@TimTr What about the point about whether or not to include the optional XcodeProjectPlugin support to make it easier for new plugins to work with Xcode projects out-of-the-box.

Copy link

Choose a reason for hiding this comment

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

Anything specific to Xcode feels like it shouldn't be there by default in the CLI tools. I could see an option (making the command pretty long to manually type, but trivial for tooling) to support tools like Xcode, or VSCode, or others. This could then be used when a user is inside Xcode already, pretty seamlessly, without implying to the rest of the Swift world that Swift is tied to Xcode.

Copy link
Contributor Author

@neonichu neonichu left a comment

Choose a reason for hiding this comment

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

Hah, can't approve since I am the person who opened the PR, but LGTM. Thanks for taking this over.

Sources/Workspace/InitPackage.swift Outdated Show resolved Hide resolved
Sources/Workspace/InitPackage.swift Outdated Show resolved Hide resolved
…e rules to the text

In the long run, it would be better for this code to construct a `Manifest` and then use `ManifestSourceGeneration` to generate the text
@abertelrud
Copy link
Contributor

@swift-ci smoke test

@abertelrud
Copy link
Contributor

@swift-ci test windows

@abertelrud
Copy link
Contributor

@tomerd @neonichu Updated contents and formatting. Also, what's the magic trick to getting the windows testing to run — it's not starting despite @swift-ci test windows etc.

@tomerd
Copy link
Contributor

tomerd commented Mar 28, 2023

@swift-ci smoke test windows

struct \(typeName): BuildToolPlugin {
func createBuildCommands(context: PluginContext, target: Target) async throws -> [Command] {
// The plugin can choose what parts of the package to process.
guard let target = target as? SourceModuleTarget else { return [] }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have a nice API for this? @neonichu ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do have

public extension Target {
    /// Convenience accessor which casts the receiver to`SourceModuleTarget` if possible.
    var sourceModule: SourceModuleTarget? {
        return self as? SourceModuleTarget
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

So we do this instead?

    let sourceFiles = target.sourceModule?.sourceFiles ?? []

And then proceed as if the target had source files even if it doesn't?

Copy link
Contributor

@abertelrud abertelrud Mar 29, 2023

Choose a reason for hiding this comment

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

Actually that doesn't work since one is a FileList? and the other an array.

So maybe just:

guard let sourceFiles = target.sourceModule?.sourceFiles else { return [] }

Copy link
Contributor

Choose a reason for hiding this comment

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

I went ahead and made this change.

@abertelrud
Copy link
Contributor

@swift-ci smoke test windows

Hm, that didn't work either. @shahmishal are we doing something wrong in how we're invoking the Windows testing here?

@abertelrud abertelrud force-pushed the add-cmd-plugin-template branch from 115784c to 32501f6 Compare March 29, 2023 00:33
@abertelrud
Copy link
Contributor

@swift-ci test

@abertelrud
Copy link
Contributor

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@abertelrud abertelrud merged commit f25e138 into main Mar 29, 2023
abertelrud pushed a commit to abertelrud/swift-package-manager that referenced this pull request Mar 29, 2023
)

Adds templates for build tool plugins and command plugins, allowing them to be created from the `swift package init` command.  The packages are set up to vend the plugins via plugin product declarations, so they can be used by other packages.  The initial code for each each plugin type is minimal, but for the build tool plugin type it shows how to create and return a build command for each source file of a particular type, since that's the most common use of build tool plugins.

rdar://107288352

Co-authored-by: Anders Bertelrud <[email protected]>
(cherry picked from commit f25e138)
@tomerd tomerd deleted the add-cmd-plugin-template branch May 1, 2023 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants