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

flutter.gradle: collect list of Android plugins from .flutter-plugins-dependencies #57907

Merged
merged 1 commit into from Jun 4, 2020

Conversation

igorakkerman
Copy link
Contributor

@igorakkerman igorakkerman commented May 24, 2020

Description

Some packages include transitive dependencies to their web version, e.g.

  • shared_preferences -> shared_preferences_web
  • location -> location_web
  • firebase_core -> firebase_core_web

PR #54407 introduced a change to the default settings.gradle file. The required plugins are now loaded through app_plugin_loader.gradle, which retrieves the list from .flutter-plugins-dependencies. Instead, the current logic in flutter.gradle still relies on .flutter-plugins, which lists transitive non-Android plugins.

This leads to a warning in the build:

Plugin project :shared_preferences_web not found. Please update settings.gradle.

The pull request now uses the same logic as in app_plugin_loader.gradle to retrieve the list of Android plugins.

Note: The same logic now exists in 3 different locations and should be extracted to a separate file. The 3 locations are:

  • app_plugin_loader.gradle
  • module_plugin_loader.gradle
  • flutter.gradle

Related Issues

#55827
#57863

Tests

Please help me understand if / where to add tests for the changes. Thank you!

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • 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

Did any tests fail when you ran them? Please read Handling breaking changes.

  • 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.
    • I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
    • I got input from the developer relations team, specifically from: Replace with the names of who gave advice
    • I wrote a migration guide: Replace with a link to your migration guide

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 24, 2020
@igorakkerman igorakkerman force-pushed the fix/android-plugins-list branch 3 times, most recently from a8d96f0 to 02c3666 Compare May 27, 2020 23:09
@zanderso zanderso requested a review from blasten May 28, 2020 20:47
@zanderso
Copy link
Member

@blasten Did you already do something similar to this?

@zanderso
Copy link
Member

zanderso commented Jun 4, 2020

/cc @cyanglaz Could you take a look at this?

@cyanglaz
Copy link
Contributor

cyanglaz commented Jun 4, 2020

@blasten might have the most context and knowledge on this.

@blasten
Copy link

blasten commented Jun 4, 2020

LGTM! Thank you

@blasten blasten merged commit 253eb1c into flutter:master Jun 4, 2020
Properties androidPlugins = new Properties()
allPlugins.each { name, path ->
if (doesSupportAndroidPlatform(path)) {
Copy link

Choose a reason for hiding this comment

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

@igorakkerman this check should be preventing the issue that you are describing. I have created a new app and imported shared_preferences, I don't see this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@blasten Indeed, I could not reproduce the issue with the current shared_preferences package, sorry for that. But I do get the message with location and firebase_core.

I created a flutter project where the issue appears: https://github.com/igorakkerman/flutter_57907_example

> flutter clean
Deleting build...                                                  319ms
Deleting .dart_tool...                                               7ms
Deleting Generated.xcconfig...                                       2ms
Deleting flutter_export_environment.sh...                            1ms
> flutter pub get
Running "flutter pub get" in flutter_57907_example...               1,2s
> flutter run
Using hardware rendering with device Android SDK built for x86 64. If you get graphics artifacts, consider enabling software rendering with
"--enable-software-rendering".
Launching lib\main.dart on Android SDK built for x86 64 in debug mode...
Plugin project :firebase_core_web not found. Please update settings.gradle.
Plugin project :location_web not found. Please update settings.gradle.
Running Gradle task 'assembleDebug'...
Running Gradle task 'assembleDebug'... Done                        38,0s
√ Built build\app\outputs\flutter-apk\app-debug.apk.

Copy link

Choose a reason for hiding this comment

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

Feel free to reopen this PR again (It got rolled back). It's good to remove any reference to .flutter-plugins in the plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @blasten. I re-opened it as #59294 .

jonahwilliams added a commit that referenced this pull request Jun 4, 2020
jonahwilliams added a commit that referenced this pull request Jun 4, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants