Skip to content

Add swift_versions to plugin template podspec, include default CocoaPod version - #44324

Merged
jmagman merged 2 commits into
flutter:masterfrom
jmagman:swift-plugin-podspec
Nov 9, 2019
Merged

Add swift_versions to plugin template podspec, include default CocoaPod version#44324
jmagman merged 2 commits into
flutter:masterfrom
jmagman:swift-plugin-podspec

Conversation

@jmagman

@jmagman jmagman commented Nov 7, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Description

  1. Set podspec swift_versions in Swift plugin podspec to make new Swift plugins work with CocoaPods in Objective-C apps.
  2. Add __has_include for Swift bridging header to make new Swift plugins buildable as libraries.

See lots of details in #25676 (comment) and the Swift Plugins design doc.

This does not include any migration logic needed for existing Swift plugins to adopt.

Related Issues

Fixes #25676... kind of. It fixes for NEW plugins, not existing ones.

Confirmed apps build and run on real devices and simulators running iOS 11 and iOS 13.

Tests

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@jmagman jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. platform-macos Building on or for macOS specifically labels Nov 7, 2019
@jmagman jmagman self-assigned this Nov 7, 2019
@fluttergithubbot fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Nov 7, 2019
jmagman
jmagman commented Nov 7, 2019
View reviewed changes
Comment thread packages/flutter_tools/templates/plugin/ios-swift.tmpl/projectName.podspec.tmpl Outdated
Comment on lines 23 to 25

@jmagman jmagman Nov 7, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There's disagreement about whether to make swift_versions exact based on the plugin author's tested Swift version, or whether to make it as permissible as possible (like this). We have a meeting to make a decision one way or the other.

If we go with the former, these lines will become:

s.swift_version = '5.0'

@jmagman
jmagman force-pushed the swift-plugin-podspec branch 2 times, most recently from 42ffaaa to 9215ae2 Compare November 7, 2019 05:17
xster
xster reviewed Nov 8, 2019
View reviewed changes

@xster xster left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, once the versioning scheme is discussed.

Comment thread dev/devicelab/bin/tasks/plugin_lint_mac.dart Outdated

@xster xster Nov 8, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are the asserts here? Just that it built? Any file we should check for in the output build/ios directory?

@jmagman jmagman Nov 8, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it will fail if any build commands fail. Most of these build scenarios fail on master due to CocoaPods or linker issues, which cause the build command to fail.
I think we check the output structure in other tests.

Comment thread packages/flutter_tools/templates/plugin/ios-swift.tmpl/projectName.podspec.tmpl Outdated

@xster xster Nov 8, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we just move ios.tmpl/projectName.podspec.tmpl to ios-objc.tmpl/projectName.podspec.tmpl then if ios-swift.tmpl is getting its own?

@jmagman jmagman Nov 8, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was renamed in this commit as well.
.../plugin/ios.tmpl/projectName.podspec.tmpl → ...in/ios-objc.tmpl/projectName.podspec.tmpl

@jmagman

jmagman commented Nov 8, 2019

Copy link
Copy Markdown
Member Author

I'm going to split this into 2 PRs: one for the Swift plugin template, one for the app build settings

@jmagman
jmagman force-pushed the swift-plugin-podspec branch from 9215ae2 to 445a481 Compare November 8, 2019 21:51
@fluttergithubbot

fluttergithubbot commented Nov 8, 2019

Copy link
Copy Markdown
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@xster

xster commented Nov 9, 2019

Copy link
Copy Markdown
Member

LGTM

@jmagman

jmagman commented Nov 9, 2019

Copy link
Copy Markdown
Member Author

Build is green on the dashboard, don't know why it's reporting red here. Merging.

@jmagman
jmagman merged commit 856a90e into flutter:master Nov 9, 2019
@jmagman
jmagman deleted the swift-plugin-podspec branch November 9, 2019 02:00
@jmagman jmagman mentioned this pull request Nov 9, 2019
Closed
preble added a commit to preble/flutter_mdns that referenced this pull request Nov 19, 2019
@preble
swift_version = 5.0, bridging header import fix
4eede2f 
Adapting changes to plugin template from: flutter/flutter#44324
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@xster xster xster left review comments
@christopherfujino christopherfujino Awaiting requested review from christopherfujino
@collinjackson collinjackson Awaiting requested review from collinjackson

Assignees

@jmagman jmagman

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically platform-macos Building on or for macOS specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flutter Swift plugins seems incompatible with most of Flutter plugins

4 participants

@jmagman @fluttergithubbot @xster @googlebot