Skip to content

Force plugins to inherit minimum iOS version from Flutter app #66590

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.

Sign up for GitHub

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

Jump to bottom
Merged
jmagman merged 2 commits into from
Sep 28, 2020
Merged

Force plugins to inherit minimum iOS version from Flutter app #66590

jmagman merged 2 commits into from
Sep 28, 2020

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Sep 24, 2020 •
edited
Loading

Background

Xcode supports specific SDK ranges, and every release they drop the lowest. For example, Xcode 11 dropped support for iOS 7, and Xcode 12 dropped support for iOS 8.
Xcode emits a warning when build targets support a minimum version lower than the range to indicate it can't validate compilation against the older SDK.

warning: The iOS deployment target is set to 6.0, but the range of supported deployment target versions for this platform is 8.0 to 12.0. (in target 'firebase_analytics')
warning: The iOS deployment target is set to 6.0, but the range of supported deployment target versions for this platform is 8.0 to 12.0. (in target 'firebase_remote_config')
warning: The iOS deployment target is set to 4.3, but the range of supported deployment target versions for this platform is 8.0 to 12.0. (in target 'image_picker')

This warning is benign: if the app supports a minimum of iOS 9.0, it doesn't cause any problems that a plugin framework embedded in the app supports iOS 7.0. These warnings concern users who think there's something wrong with their app, and there's no way for users to fix it (the plugin author would have to up their minimum supported version).

Flutter's plugin template defaults to a minimum of iOS 8, so when Xcode 12 dropped that version, every plugin started emitting this warning, so this problem became much more visible.

Description

If the plugin's minimum version is less than iOS 9, remove the version build setting. This will cause it to inherit from the minimum version set in the Flutter app (currently 9.0).

If the plugin minimum is higher than iOS 9, leave the build setting so if there is a versioning range issue (the plugin supports a minimum iOS 12 but the app needs iOS 9) the correct error (not warning) will be emitted.

Related Issues

Fixes #40109
Fixes #63253

See also
CocoaPods/CocoaPods#7314
#36117

Tests

Updated plugin_dependencies_test to make sure this warning isn't emitted. Confirmed it fails on master.

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 read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • 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

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change. If not, delete the remainder of this section.

@jmagman jmagman added platform-ios iOS applications specifically t: xcode "xcodebuild" on iOS and general Xcode project management labels Sep 24, 2020
@jmagman jmagman self-assigned this Sep 24, 2020
@flutter-dashboard flutter-dashboard bot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Sep 24, 2020
christopherfujino
christopherfujino reviewed Sep 24, 2020
View reviewed changes
@jmagman jmagman mentioned this pull request Sep 24, 2020
Closed
@jmagman
Copy link
Member Author

jmagman commented Sep 24, 2020

@domesticmouse This should resolve #66103.

christopherfujino
christopherfujino reviewed Sep 24, 2020
View reviewed changes
dev/devicelab/bin/tasks/plugin_dependencies_test.dart Outdated
Copy link
Contributor

@christopherfujino christopherfujino Sep 24, 2020

Choose a reason for hiding this comment

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

I may be wrong (hard to tell in GitHub), but it looks to me like lines 285-288 are indented 1 col too far, while 289-290 look ok?

Copy link
Member Author

@jmagman jmagman Sep 26, 2020

Choose a reason for hiding this comment

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

You're right. Good eye (again).

christopherfujino
christopherfujino reviewed Sep 24, 2020
View reviewed changes
packages/flutter_tools/bin/podhelper.rb Outdated
Copy link
Contributor

@christopherfujino christopherfujino Sep 24, 2020

Choose a reason for hiding this comment

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

should we leave a breadcrumb comment at the location in the tool where we define the min version to also update this magic constant?

Copy link
Member Author

@jmagman jmagman Sep 26, 2020

Choose a reason for hiding this comment

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

Good idea.

xster
xster approved these changes Sep 25, 2020
View reviewed changes
dev/devicelab/bin/tasks/plugin_dependencies_test.dart Outdated
Copy link
Member

@xster xster Sep 25, 2020

Choose a reason for hiding this comment

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

I'm not sure this is the best place to put it. Since it's a test to check that interdependencies in plugins work correctly rather than checking the project->plugin connection. Should this just be in plugin_tests.dart?

Also how do we know plugins A/B/C/D says iOS 8? Tangentially, do we need to update

flutter/packages/flutter_tools/templates/plugin/ios-swift.tmpl/projectName.podspec.tmpl

Line 18 in 6b04937

s.platform = :ios, '8.0'
as well?

Copy link
Member Author

@jmagman jmagman Sep 26, 2020

Choose a reason for hiding this comment

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

Should this just be in plugin_tests.dart?

I moved it there, see if you like it better.

Also how do we know plugins A/B/C/D says iOS 8?

I changed the test to make the plugin version low (7.0) so it would trigger the warning on the devicelab machines running Xcode 11.

Tangentially, do we need to update

flutter/packages/flutter_tools/templates/plugin/ios-swift.tmpl/projectName.podspec.tmpl

Line 18 in 6b04937

s.platform = :ios, '8.0'
as well?

If we updated to s.platform = :ios, '9.0' then newly created plugins would not be usable (complication error) in any projects created before #60035, and we have no migration (yet) to increase this value in already existing apps.

@xster
Copy link
Member

xster commented Sep 26, 2020

LGTM!

@jmagman jmagman merged commit 65159af into flutter:master Sep 28, 2020
@jmagman jmagman deleted the plugin-version branch September 28, 2020 17:14
@jmagman jmagman mentioned this pull request Oct 12, 2020
Closed
@jmagman jmagman mentioned this pull request Dec 22, 2020
Merged
@jmagman jmagman mentioned this pull request Mar 4, 2021
Closed
8 tasks
@jmagman jmagman mentioned this pull request Nov 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xster xster xster approved these changes

+1 more reviewer

@christopherfujino christopherfujino christopherfujino left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jmagman jmagman

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically t: xcode "xcodebuild" on iOS and general Xcode project management 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.

Increase plugin template minimum supported iOS version from 8.0 to 9.0 Plugin target deployments are out of range, causes Xcode warning

4 participants

@jmagman @xster @christopherfujino @googlebot