Skip to content

Allow plugins to not support Android #39657

Closed
Closed
Allow plugins to not support Android#39657
Labels
p: toolingAffects the flutter_plugin_tools packagetoolAffects the "flutter" command-line tool. See also t: labels.

Description

@stuartmorgan-g
stuartmorgan-g
opened on Sep 1, 2019

#10288 added a formal way to allow plugins to indicate which platforms they support, but any plugin that doesn't support Android (as in, has no android/ folder) will cause Gradle failures. It looks from initial investigation like this is because flutter.gradle uses .flutter-plugins, which isn't platform-specific and thus isn't subject to platform filtering.

Presumably we should move away from having a single .flutter-plugins at all, and migrate all current clients to new platform-specific versions.

Currently the only way to make, say, a desktop-specific plugin that doesn't break Android is with a dummy directory.

Activity

  1. stuartmorgan-g
    added
    toolAffects the "flutter" command-line tool. See also t: labels.
    p: toolingAffects the flutter_plugin_tools package
    on Sep 1, 2019

  2. blasten commented on Sep 4, 2019

    @blasten
    blasten
    on Sep 4, 2019

    This makes sense. @amirh's proposal: "Distributed Platform Ownership for Flutter Plugins" touches on this--we could start by listing the platforms in the plugin's pubspec file. I saw you made some comments in the doc, is there general agreement around that (pubspec schema at least)?

  3. stuartmorgan-g commented on Sep 4, 2019

    @stuartmorgan-g
    stuartmorgan-g
    on Sep 4, 2019
    ContributorAuthor

    we could start by listing the platforms in the plugin's pubspec file

    #38632 already added support for that. This bug is specifically about making the right thing happen when you don't list Android there, which currently isn't the case.

  4. blasten commented on Sep 4, 2019

    @blasten
    blasten
    on Sep 4, 2019

    oh excellent. I wasn't aware of that.

    We might still need .flutter-plugins though, but maybe we can split it by platform? Ootherwise, we need to pass the plugin list to Gradle as an argument, but that breaks add to app scenarios where the host app is built from Android Studio instead of the Flutter tool.

  5. stuartmorgan-g commented on Sep 4, 2019

    @stuartmorgan-g
    stuartmorgan-g
    on Sep 4, 2019
    ContributorAuthor

    We might still need .flutter-plugins though, but maybe we can split it by platform?

    That's what I meant by "new platform-specific versions". (iOS and macOS would also benefit from having such files.)

  6. amirh commented on Sep 16, 2019

    @amirh
    amirh
    on Sep 16, 2019
    Contributor

    So I guess the plan is to split .flutter-plugins into multiple .flutter-plugins-<platform> files. It sounds like we will need to implement auto-migration for for the gradle/xcode stuff (e.g the tool will update existing settings.gradle to refer to .flutter-plugins-android).

    Any thoughts? better options?

    @blasten @stuartmorgan @jonahwilliams

  7. blasten commented on Sep 16, 2019

    @blasten
    blasten
    on Sep 16, 2019

    sgtm. One more reason for #40460

  8. stuartmorgan-g commented on Sep 16, 2019

    @stuartmorgan-g
    stuartmorgan-g
    on Sep 16, 2019
    ContributorAuthor

    We don't necessary have to migrate; we could keep making .flutter-plugins for all existing projects, and tell people who want to start using plugins that don't support all of their project's platforms to manually delete the appropriate files (e.g., Podspec) and rebuild so it would generate a new version.

    Not ideal, but possible.

  9. jonahwilliams commented on Sep 16, 2019

    @jonahwilliams
    jonahwilliams
    on Sep 16, 2019
    Contributor

    I think there are a few independent pieces of work here.

    One would be for all of the tooling to gracefully handle a missing plugin implementation. This would at least unblock work in the short-term even it isn't the final strategy.

    Separately, rather than create N .flutter-plugins-platform files I think we should update the tooling to look in the platform projects' ephemeral/output directory and then fall back to the project root. We can output platform specific .flutter-plugins in these directories without breaking any current workflows.

    Once that is done, we can evaluate the cost of removing the project root plugins

  10. amirh commented on Sep 16, 2019

    @amirh
    amirh
    on Sep 16, 2019
    Contributor

    One would be for all of the tooling to gracefully handle a missing plugin implementation. This would at least unblock work in the short-term even it isn't the final strategy.

    @jonahwilliams can you sketch how you envision that? (to make sure we're all on the same page)

  11. jonahwilliams commented on Sep 16, 2019

    @jonahwilliams
    jonahwilliams
    on Sep 16, 2019
    Contributor

    In the android case, it would mean updating flutter.gradle to check for the correct files before creating a plugin task here:

    https://github.com/flutter/flutter/blob/master/packages/flutter_tools/gradle/flutter.gradle#L250

    Maybe it should print an error or warning, but definitely not crash.

  12. blasten commented on Sep 16, 2019

    @blasten
    blasten
    on Sep 16, 2019

    In the android case, it would mean updating flutter.gradle to check for the correct files before creating a plugin task here:

    This also requires changes to android/settings.gradle since the plugins are still included as subprojects in app projects.

    Separately, rather than create N .flutter-plugins-platform files I think we should update the tooling to look in the platform projects' ephemeral/output directory and then fall back to the project root.

    @jonahwilliams This sounds good too. Do you plan to change flutter create so that it takes the list of target platforms as an argument?

  13. jonahwilliams commented on Sep 16, 2019

    @jonahwilliams
    jonahwilliams
    on Sep 16, 2019
    Contributor

    @jonahwilliams This sounds good too. Do you plan to change flutter create so that it takes the list of target platforms as an argument?

    .flutter-plugins is not a part of flutter create.

    This also requires changes to android/settings.gradle since the plugins are still included as subprojects in app projects.

    Ahh yes, that's unfortunate. I would suggest @stuartmorgan's plan then

  14. amirh commented on Sep 16, 2019

    @amirh
    amirh
    on Sep 16, 2019
    Contributor

    I prototyped excluding plugins without an android/build.gradle file from the list of plugins we're building in amirh@bdcf4c5. It seem to work, though I know very little about Gradle so am likely missing something. @blasten what should I expect to break if I didn't change anything in android/settings.gradle?

    Also - do we have a reasonable harness to test such a thing?

  15. 4 remaining items

  16. jonahwilliams commented on Sep 17, 2019

    @jonahwilliams
    jonahwilliams
    on Sep 17, 2019
    Contributor

    Echoing my comment from the iOS bug: what's the advantage of doing this?

    I see this as a general tool hardening, we shouldn't crash if files/folders are missing in easily anticipated ways. The fact that it unblocks some work temporarily is just a side effect.

  17. jonahwilliams commented on Sep 17, 2019

    @jonahwilliams
    jonahwilliams
    on Sep 17, 2019
    Contributor

    Generating these platform-specific files in Dart should be trivial given that we already have all the code for filtering plugins by platform

    I agree, it's not much more difficult- we should do that also.

  18. amirh commented on Sep 17, 2019

    @amirh
    amirh
    on Sep 17, 2019
    Contributor

    Echoing my comment from the iOS bug: what's the advantage of doing this? This is the same file we'd need to change to point it to a different, Android-specific .flutter-plugins file, so this is doing more throw-away work with no benefit that I can see. Generating these platform-specific files in Dart should be trivial given that we already have all the code for filtering plugins by platform, including handling of fallbacks depending on whether or not plugins are using the new or old schema. And if we do that once, we can do iOS and Android the way we want to support long term right now, without an intermediate step.

    I'm not sure how simple and/or reliable can the auto migration of existing apps' settings.gradle be (which currently points to `.flutter-plugins), without that migration we need something like the current patch.

  19. stuartmorgan-g commented on Sep 17, 2019

    @stuartmorgan-g
    stuartmorgan-g
    on Sep 17, 2019
    ContributorAuthor

    Oh, I missed that this is a file that's part of Flutter itself, not the project. I was assuming it would be similar to iOS, where the file is generated into the project and then left there, requiring some kind of update (migrated or manual). If we can retroactively fix all existing projects to handle this gracefully I definitely understand why it's worth doing.

  20. stuartmorgan-g commented on Sep 17, 2019

    @stuartmorgan-g
    stuartmorgan-g
    on Sep 17, 2019
    ContributorAuthor

    I started playing around with generating platform-specific plugin files; WIP here. It's totally untested (even manually), and we'd need to figure out if those are the right file locations on mobile, but I think that's basically the right structure.

    Then we can incrementally work on removing all internal use of .flutter-plugins, except for continuing to write it for existing projects until/unless we migrate them.

    I'm not sure when I'll have time to continue with that so if someone wants to take that and turn it into a real PR please feel free.

  21. jmagman commented on Jan 18, 2020

    @jmagman
    jmagman
    on Jan 18, 2020
    Member

    @franciscojma86 Will this also be fixed by #47258?

  22. franciscojma86 commented on Jan 21, 2020

    @franciscojma86
    franciscojma86
    on Jan 21, 2020
    Contributor

    @jmagman That's an obsolete PR for now. The JSON logic has already landed. The migration part will be discussed here , which might depend on the decisions taken in this proposal

  23. jmagman commented on Jan 21, 2020

    @jmagman
    jmagman
    on Jan 21, 2020
    Member

    @franciscojma86 Can this issue be closed? Or is there more work to track here?

  24. franciscojma86 commented on Jan 21, 2020

    @franciscojma86
    franciscojma86
    on Jan 21, 2020
    Contributor

    We still need no-op implementations for plugins, so we can't close this yet. We will close after we migrate the templates.

  25. blasten commented on Apr 9, 2020

    @blasten
    blasten
    on Apr 9, 2020

    This issue was fixed by #47015. Plugins that don't have an android/build.gradle aren't used when building for Android.

  26. blasten
    closed this as completedon Apr 9, 2020

  27. ditman commented on Apr 10, 2020

    @ditman
    ditman
    on Apr 10, 2020
    Member

    This is great! Does this mean we can remove the noop android directories from plugin packages?

  28. blasten commented on Apr 10, 2020

    @blasten
    blasten
    on Apr 10, 2020

    Yep! However, I'd wait until the next stable release is rolled out. #46304 is tracking that

  29. lock commented on Apr 25, 2020

    @lock
    lockbot
    on Apr 25, 2020

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

  30. lock
    locked and limited conversation to collaborators on Apr 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    p: toolingAffects the flutter_plugin_tools packagetoolAffects the "flutter" command-line tool. See also t: labels.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions