Don't import plugins that don't support android in settings.gradle by blasten · Pull Request #54407 · flutter/flutter

blasten · 2020-04-10T00:24:14Z

Description

Currently, the Flutter Gradle plugin skips plugins that don't have an android/build.gradle file. However, we shouldn't be importing these plugins in the first place. A new behavior in Android Studio or Gradle could break the current check.

Android Studio currently creates an android/ noop directory in the .pub-cache directory.

Hopefully, when #40460 is fixed, we will have a tool to automate the migration of project files like settings.gradle.

Related Issues

#39657

Tests

Updated tests.

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

jonahwilliams · 2020-04-10T05:10:26Z

If we're making an edit to the settings.gradle, I think the right fix would be to read from .flutter-plugin-dependencies instead. The contents are broken down by platform:

Example:

{
    "info": "This is a generated file; do not edit or check into version control.",
    "plugins": {
        "ios": [
            {
                "name": "connectivity",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\connectivity-0.4.8+2\\\\",
                "dependencies": []
            },
            {
                "name": "device_info",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\device_info-0.4.2+1\\\\",
                "dependencies": []
            },
            {
                "name": "url_launcher",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\url_launcher-5.4.2\\\\",
                "dependencies": []
            },
            {
                "name": "video_player",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\video_player-0.10.6\\\\",
                "dependencies": []
            }
        ],
        "android": [
            {
                "name": "connectivity",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\connectivity-0.4.8+2\\\\",
                "dependencies": []
            },
            {
                "name": "device_info",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\device_info-0.4.2+1\\\\",
                "dependencies": []
            },
            {
                "name": "url_launcher",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\url_launcher-5.4.2\\\\",
                "dependencies": []
            },
            {
                "name": "video_player",
                "path": "C:\\\\Users\\\\Jonah\\\\AppData\\\\Roaming\\\\Pub\\\\Cache\\\\hosted\\\\pub.dartlang.org\\\\video_player-0.10.6\\\\",
                "dependencies": []
            }
        ],

I thought we filled an issue for this, maybe @franciscojma86 knows where it is 😄

franciscojma86 · 2020-04-10T05:34:13Z

I thought I has a draft PR for that but can't find it :( But yeah, flutter-plugins should/will be deprecated anyways, so you should use .flutter-plugins-dependencies.

The only issue that I know of is #39657 but it's already referenced here.

blasten · 2020-04-10T17:48:09Z

ahh good call

packages/flutter_tools/templates/module/android/library/include_flutter.groovy.copy.tmpl

blasten · 2020-04-10T19:34:55Z

I've moved most of the logic in settings.gradle to the Flutter SDK location, so if we have to make a change later on, it's easier to manage. PTAL

jonahwilliams

Overal LGTM

Do we plan to handle migrating users too? If not, we might want to keep a project using the old settings.gradle for testing

blasten · 2020-04-11T22:59:37Z

Yep - it will be migrated. I filed #54566 and added an integration test just in case, so we don't delete .flutter-plugins prematurely.

…lutter#54407)

* fix segment hit test behavior (#57461) * Making DropdownButtonFormField to re-render if parent widget changes (#57037) * Update DropdownButtonFormField's state if widget updates Co-authored-by: Shi-Hao Hong <[email protected]> * throw more specific toolexit when git fails during upgrade (#57162) * [flutter_tools] Refresh VM state before executing hot reload (#53960) * Update engine hash for 1.17.2 * Remove MaterialControls from examples/flutter_view (#57621) Co-authored-by: Jenn Magder <[email protected]> * Prevent building non-android plugins in build aar (#58018) * Allow FLUTTER_APPLICATION_PATH to be null for misconfigured Xcode projects (#57701) * Don't import plugins that don't support android in settings.gradle (#54407) Co-authored-by: LongCatIsLooong <[email protected]> Co-authored-by: Pedro Massango <[email protected]> Co-authored-by: Shi-Hao Hong <[email protected]> Co-authored-by: Christopher Fujino <[email protected]> Co-authored-by: Jason Simmons <[email protected]> Co-authored-by: stuartmorgan <[email protected]> Co-authored-by: Jenn Magder <[email protected]> Co-authored-by: Emmanuel Garcia <[email protected]>

Don't import plugins that don't support android in settings.gradle

8338e76

blasten added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Apr 10, 2020

fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) d: examples Sample code and demos tool Affects the "flutter" command-line tool. See also t: labels. labels Apr 10, 2020

googlebot added the cla: yes label Apr 10, 2020

Fix CI

af13f90

blasten requested review from jonahwilliams and zanderso April 10, 2020 02:58

Move settings.gradle to the SDK location

7566116

blasten commented Apr 10, 2020

View reviewed changes

packages/flutter_tools/templates/module/android/library/include_flutter.groovy.copy.tmpl Show resolved Hide resolved

Emmanuel Garcia added 5 commits April 10, 2020 13:30

typo

858d2f8

Move app/module import back to settings.gradle

fe186fd

Clean up

4d18d22

Fix add-to-app case

3876bfb

Pass jetifier test

8fdd0e2

jonahwilliams approved these changes Apr 11, 2020

View reviewed changes

blasten mentioned this pull request Apr 11, 2020

Migrate existing android/settings.gradle to the new file. #54566

Closed

Test existing settings.gradle until migrated

2f66236

blasten added the waiting for tree to go green label Apr 11, 2020

Update dependencies

49d489d

fluttergithubbot removed the waiting for tree to go green label Apr 12, 2020

flutter deleted a comment from fluttergithubbot Apr 12, 2020

Lint

d64e53d

blasten added the waiting for tree to go green label Apr 12, 2020

fluttergithubbot merged commit 54dba4c into flutter:master Apr 12, 2020

jmagman mentioned this pull request May 6, 2020

Could not read script 'flutter/packages/flutter_tools/gradle/app_plugin_loader.gradle' as it does not exist #55619

Open

igorakkerman mentioned this pull request May 24, 2020

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

Merged

13 tasks

pcsosinski pushed a commit to pcsosinski/flutter that referenced this pull request May 28, 2020

Don't import plugins that don't support android in settings.gradle (f…

4bde236

…lutter#54407)

igorakkerman mentioned this pull request Jun 12, 2020

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

Merged

13 tasks

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't import plugins that don't support android in settings.gradle#54407

Don't import plugins that don't support android in settings.gradle#54407
fluttergithubbot merged 11 commits intoflutter:masterfrom
blasten:plugin_android_exist

blasten commented Apr 10, 2020 •

edited

Loading

Uh oh!

jonahwilliams commented Apr 10, 2020

Uh oh!

franciscojma86 commented Apr 10, 2020

Uh oh!

blasten commented Apr 10, 2020

Uh oh!

Uh oh!

blasten commented Apr 10, 2020

Uh oh!

jonahwilliams left a comment

Uh oh!

blasten commented Apr 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

blasten commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

jonahwilliams commented Apr 10, 2020

Uh oh!

franciscojma86 commented Apr 10, 2020

Uh oh!

blasten commented Apr 10, 2020

Uh oh!

Uh oh!

blasten commented Apr 10, 2020

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

blasten commented Apr 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

blasten commented Apr 10, 2020 •

edited

Loading