Skip to content

Let Jetifier work when using AndroidX - #29849

Closed
dnfield wants to merge 17 commits into
flutter:masterfrom
dnfield:mklim-plugin_aar_wip
Closed

Let Jetifier work when using AndroidX#29849
dnfield wants to merge 17 commits into
flutter:masterfrom
dnfield:mklim-plugin_aar_wip

Conversation

@dnfield

@dnfield dnfield commented Mar 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This continues work begun in #29460:

Add extra logic to gradle.dart before the APK
of an app is compiled.

  • Gets a flat list of all transitive Flutter plugin dependencies.
  • Runs an init-script on each one to add the local flutter-jar as a
    compileOnly dependency to it.
  • Compiles each dependency as an AAR.
  • Uses the plugin AARs as dependencies for compiling the APK in the
    flutter.gradle plugin.

and adds with the following changes/updates:

  • Pull in transitive deps from plugins (anything listed as an api or compile dependency).
  • Update settings.gradle and build.gradle in the Flutter project (and templates) to remove deprecated logic that will no longer be compatible. Provide migration in gradle.dart
  • Build either release or debug of the plugin and use it specifically

Problems this doesn't solve:

  1. If a plugin fails to properly declare its dependencies, it won't build. It may have built under the old system if the root project declared those dependencies (e.g. if the Flutter app developer declared them). This is probably acceptable - plugins should really declare their dependencies.
  2. It's possible that two plugins declare dependencies on incompatbile dependencies (e.g. one wants firebase-core X and another wants firebase-core Y) - and Jettifier may not be able to fix those like it can for Android X. This is a problem today.

Problems it does solve:

  1. Jettifier will work for resolving Android Support/Android X incompatibilities.

Related Issues

Fixes #29328

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.
  • My PR includes tests for all changed/updated/fixed behaviors (See Test Coverage).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Michael Klimushyn and others added 6 commits March 15, 2019 15:55
Michael Klimushyn
[WIP] Compile plugins as .aars, not subprojects
f898aac 
This is a very rough proof of concept. It functions on simple app cases
given manual tweaking.

It basically works by adding extra logic to `gradle.dart` before the APK
of an app is compiled.

- Get a flat list of all transitive Flutter plugin dependencies.
- Run an `init-script` on each one to add the local `flutter-jar` as a
  `compileOnly` dependency to it.
- Compile each dependency as an AAR.
- Use the plugin AARs as dependencies for compiling the APK in the
  `flutter.gradle` plugin.

There's two big blockers on this still that I'm currently aware of.

1. This requires apps to manually remove the `subprojects` declarations
   from `android/build.gradle` and the `plugins.each` loop from
   `android/settings.gradle`. It's both breaking and requiring manual
   intervention on the app side to work.
2. Transitive gradle dependencies within our Flutter plugins cause
   crashes at runtime, since the inner deps aren't being included in the
   aar. To see a failing case, use this code with a sample app requiring
   `firebase_auth`. At runtime it will crash because the firebase
   library isn't found.
@dnfield
Merge branch 'plugin_aar_wip' of git://github.com/mklim/flutter into …
bf301ba 
…mklim-plugin_aar_wip
@dnfield
Merge remote-tracking branch 'upstream/master' into mklim-plugin_aar_wip
7d5f212
@dnfield
temp
c701139
@dnfield
Add transitive deps, migrate project structures
59bc5db
@dnfield
assembleRelease instead of build
26fa8fb
@dnfield
dnfield requested a review from mklim March 23, 2019 18:03
@dnfield

dnfield commented Mar 23, 2019

Copy link
Copy Markdown
Contributor Author

This needs tests.

@dnfield dnfield added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. t: gradle "flutter build" and "flutter run" on Android labels Mar 23, 2019
@dnfield dnfield mentioned this pull request Mar 23, 2019
Closed
6 of 10 tasks
mklim
mklim reviewed Mar 25, 2019
View reviewed changes

@mklim mklim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should probably migrate all our sample code in one go, but I think it would also be worth leaving it unmigrated for a patch or two and letting it go through CI just to make sure we're not introducing regressions. Spot checked the errors in CI on the latest patch and it doesn't look related to this, saying "no devices found."

Comment thread packages/flutter_tools/gradle/flutter.gradle Outdated
Comment thread packages/flutter_tools/lib/src/android/gradle.dart Outdated
@matthew-carroll

matthew-carroll commented Mar 25, 2019

Copy link
Copy Markdown
Contributor

Do we have a proposal and/or design doc for this where I can see the whole picture of what's changing?

@dnfield

dnfield commented Mar 25, 2019

Copy link
Copy Markdown
Contributor Author

@matthew-carroll right now it's all in #29328

matthew-carroll
matthew-carroll reviewed Mar 26, 2019
View reviewed changes
Comment thread packages/flutter_tools/gradle/flutter.gradle Outdated
Comment thread packages/flutter_tools/gradle/flutter.gradle Outdated
Comment thread packages/flutter_tools/gradle/flutter.gradle Outdated
Comment thread packages/flutter_tools/gradle/flutter.gradle Outdated
Comment thread packages/flutter_tools/gradle/plugins_init.gradle Outdated
Comment thread packages/flutter_tools/lib/src/android/gradle.dart Outdated
Comment thread packages/flutter_tools/lib/src/android/gradle.dart Outdated
Comment thread packages/flutter_tools/lib/src/android/gradle.dart Outdated
@dnfield
dnfield marked this pull request as ready for review March 26, 2019 04:02
@dnfield dnfield changed the title Refactor the way plugins are built Let Jetifier work when using AndroidX Mar 26, 2019
@dnfield

dnfield commented Apr 5, 2019

Copy link
Copy Markdown
Contributor Author

I'm going to close this for now. It seems like the only relevant difference is the compileSdkVersion. This PR isn't really addressing the right problem as is.

@dnfield dnfield closed this Apr 5, 2019
@mklim mklim mentioned this pull request May 16, 2019
Closed
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matthew-carroll matthew-carroll Awaiting requested review from matthew-carroll
@mklim mklim Awaiting requested review from mklim

Assignees

No one assigned

Labels

platform-android Android applications specifically t: gradle "flutter build" and "flutter run" on Android 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.

Refactor the way Android Plugins are included

4 participants

@dnfield @matthew-carroll @mklim @googlebot