Skip to content

Conversation

@blasten
Copy link

@blasten blasten commented Apr 10, 2020

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.

@blasten blasten added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Apr 10, 2020
@fluttergithubbot 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
@jonahwilliams
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

blasten commented Apr 10, 2020

ahh good call

@blasten
Copy link
Author

blasten commented Apr 10, 2020

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

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

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
Copy link
Author

blasten commented Apr 11, 2020

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

@fluttergithubbot fluttergithubbot merged commit 54dba4c into flutter:master Apr 12, 2020
pcsosinski pushed a commit to pcsosinski/flutter that referenced this pull request May 28, 2020
pcsosinski pushed a commit that referenced this pull request May 28, 2020
* 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]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. d: examples Sample code and demos tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants