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

Adds support for generating projects that use AndroidX support libraries #31028

Merged
merged 11 commits into from Jun 1, 2019

Conversation

@athornz
Copy link
Contributor

commented Apr 14, 2019

Description

This PR modifies the Flutter tool with support for generating projects that use AndroidX support libraries rather than the legacy support library.

This is a source of pain in Flutter development and now that AndroidX is 1.0 all new Flutter projects should be using it.

Projects generated with AndroidX libraries also use Jetifier which allows any dependencies (e.g. plugins) that still use the legacy support libraries to be upgrade to use AndroidX.

With this change, the flutter tool has the following changes:

  • Adds an --androidx flag when creating flutter projects. This is disabled by default, and can be enabled via --androidx
  • Adds a new androidX property under the module section of a pubspec.yaml file. This is to control whether the generated module project uses AndroidX.
    e.g.
flutter:
  uses-material-design: true

  module:
    androidX: true
    androidPackage:
    iosBundleIdentifier:

Related Issues

Tests

  • create_test.dart/androidx app project
  • create_test.dart/non androidx app project
  • create_test.dart/androidx plugin project
  • create_test.dart/non androidx plugin project

Checklist

  • 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 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

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

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@googlebot googlebot added the cla: yes label Apr 14, 2019

@athornz athornz changed the base branch from master to dev Apr 14, 2019

@goderbauer goderbauer added the tool label Apr 15, 2019

@athornz athornz force-pushed the athornz:androidx branch 2 times, most recently from 87a0556 to 31cd632 Apr 18, 2019

@jiechic

This comment has been minimized.

Copy link

commented May 6, 2019

Why haven't merged yet?
Is it necessary use support library by default and Optional androidx support before code merge?

@athornz

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@dnfield I've seen you comment about AndroidX support a few other places. Is it possible this PR may be merged or is there other internal work happening around it?

@jiechic

This comment has been minimized.

Copy link

commented May 22, 2019

Why should this PR not be merged?

@athornz

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

@matthew-carroll would you be the right person to review this?

@matthew-carroll

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

@xster you're probably in the best position to make a call on this particular solution.

@xster

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

Thanks for your contribution @athornz

@dnfield @mklim, could you guys take a pass at this?

I'm out for a week. I'll check this wrt add-to-app when I'm back.

@dnfield

This comment has been minimized.

Copy link
Member

commented May 30, 2019

Thanks for the contribution!

We've had a number of issues with jetifier not working with our current gradle setup. I'm a bit curious about how this impacts that - are you able to consume a plugin that uses the support libraries with this project structure successfully?

My main concern here is that it's not clear what benefit this really offers to someone developing a Flutter app at this point - unless it's helping with that issue.

@dnfield dnfield requested a review from jonahwilliams May 30, 2019

@athornz

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

Hey Dan,

With this setup, a project will work with both AndroidX and Support dependencies.

Without this, I believe Flutter projects that use plugins with AndroidX dependencies need to be upgraded to AndroidX.

I'm using this branch in a Flutter project using Jetifier and all is working well.

So the benefit is that new Flutter projects using AndroidX should just work with any plugin. Do you have any more information about the issues with Jetifier that you've had? I'd like to test this branch to make sure it solves those issues.

@jiechic

This comment has been minimized.

Copy link

commented May 30, 2019

@athornz
I think it should use support library as the default then androidX library is optional for old support library flutter module project update

@athornz

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

Since support library is deprecated I think AndroidX should be the default. If a Flutter project with AndroidX works with Jetifier then there is no reason to use the support libraries.

We should be making use of Jetifier as Google implemented as a good way to update to AndroidX and be compatible with support libraries.

@mklim

This comment has been minimized.

Copy link
Member

commented May 30, 2019

Thanks for the contribution! @athornz I agree, AndroidX with Jetifier should ideally be our default to get rid of any support issues. The problem we've run into so far is that Jetifier doesn't work with how we build Flutter apps and include plugins: #32714. You can see this by making a new template with this PR or by migrating an app manually, then depending on a an older plugin (like firebase_core: 0.1.0), then running flutter build apk.

Even with Jetifier being broken for us @dnfield I think this is probably an improvement. At least all of flutter/plugins are migrated to AndroidX at this point, so with that it makes sense to me to have the default template be migrated too. Right now people still need to migrate manually.

@tvolkert
Copy link
Contributor

left a comment

Thanks for the contribution @athornz - this is great!

@dnfield having just spent 5 hours wrestling with AndroidX issues in my project, I think this change will help enable new projects to be built.

@athornz

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

Thanks @dnfield and @tvolkert for the review - I think I've addressed all the comments now but let me know if there is anything else.

@tvolkert
Copy link
Contributor

left a comment

LGTM modulo one small nit.

@tvolkert

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

LGTM. Will land on green.

Thanks again!

@athornz

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

@jonahwilliams I've rebased on master now.

@googlebot

This comment has been minimized.

Copy link

commented May 30, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels May 30, 2019

@blasten blasten merged commit d0e45a2 into flutter:master Jun 1, 2019

35 checks passed

WIP Ready for review
Details
add2app-macos Task Summary
Details
add2app-macos
Details
analyze Task Summary
Details
analyze
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
build_tests-macos Task Summary
Details
build_tests-macos
Details
build_tests-windows Task Summary
Details
build_tests-windows
Details
cla/google All necessary CLAs are signed
deploy_gallery Task Summary
Details
deploy_gallery
Details
deploy_gallery-macos Task Summary
Details
deploy_gallery-macos
Details
docs Task Summary
Details
docs
Details
flutter-build
Details
integration_tests-linux Task Summary
Details
integration_tests-linux
Details
integration_tests-windows Task Summary
Details
integration_tests-windows
Details
tests-linux Task Summary
Details
tests-linux
Details
tests-macos Task Summary
Details
tests-macos
Details
tests-windows Task Summary
Details
tests-windows
Details
tool_tests-linux Task Summary
Details
tool_tests-linux
Details
tool_tests-macos Task Summary
Details
tool_tests-macos
Details
tool_tests-windows Task Summary
Details
tool_tests-windows
Details

mklim added a commit that referenced this pull request Jun 7, 2019

Adds the androidX flag to a modules pubspec.yaml template so it is se… (
#34066)

This is a small follow up to the previous AndroidX PR: #31028

This fixes an issue mentioned [here](#28805) where the androidX flag for a module is not set when creating a new project:

`flutter create --androidx -t module my_flutter`

Kiku-Reise added a commit to Kiku-Reise/flutter that referenced this pull request Jun 14, 2019

Kiku-Reise added a commit to Kiku-Reise/flutter that referenced this pull request Jun 14, 2019

Adds the androidX flag to a modules pubspec.yaml template so it is se… (
flutter#34066)

This is a small follow up to the previous AndroidX PR: flutter#31028

This fixes an issue mentioned [here](flutter#28805) where the androidX flag for a module is not set when creating a new project:

`flutter create --androidx -t module my_flutter`

goderbauer added a commit to goderbauer/flutter that referenced this pull request Jul 3, 2019

goderbauer added a commit to goderbauer/flutter that referenced this pull request Jul 3, 2019

Adds the androidX flag to a modules pubspec.yaml template so it is se… (
flutter#34066)

This is a small follow up to the previous AndroidX PR: flutter#31028

This fixes an issue mentioned [here](flutter#28805) where the androidX flag for a module is not set when creating a new project:

`flutter create --androidx -t module my_flutter`
@mcxinyu

This comment has been minimized.

Copy link

commented Jul 9, 2019

Flutter.jar is not compatible with androidX

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

@mcxinyu

Flutter.jar is not compatible with androidX

Currently I have a the app crashed:

    java.lang.NoClassDefFoundError: Failed resolution of: Landroidx/lifecycle/DefaultLifecycleObserver;
        at io.flutter.embedding.engine.FlutterEngine.<init>(FlutterEngine.java:149)

is it related to flutter.jar? cc @athornz @dnfield @tvolkert

@blasten

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

That is because we are missing AndroidX transitive dependencies from the engine. We have a fix in progress. cc @matthew-carroll

@blasten

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

In the meanwhile, you can the dependencies manually to android/build.gradle as indicated on https://developer.android.com/jetpack/androidx/releases/lifecycle#declaring_dependencies

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

The crash still happens even after I added the following snippet to app/build.gradle

    // known issue https://github.com/flutter/flutter/pull/31028#issuecomment-516902006
    def lifecycle_version = "2.0.0"
    // ViewModel and LiveData
    implementation "androidx.lifecycle:lifecycle-extensions:$lifecycle_version"
    // alternatively - just ViewModel
    implementation "androidx.lifecycle:lifecycle-viewmodel-ktx:$lifecycle_version"
    // alternatively - just LiveData
    implementation "androidx.lifecycle:lifecycle-livedata:$lifecycle_version"
    implementation "androidx.lifecycle:lifecycle-runtime:$lifecycle_version"
    kapt "androidx.lifecycle:lifecycle-compiler:$lifecycle_version"
    // optional - ReactiveStreams support for LiveData
    implementation "androidx.lifecycle:lifecycle-reactivestreams-ktx:$lifecycle_version"
    // optional - Test helpers for LiveData
    testImplementation "androidx.arch.core:core-testing:$lifecycle_version"
@blasten

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

@truongsinh would you mind creating an issue with a repro test case? Thanks!

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

@truongsinh would you mind creating an issue with a repro test case? Thanks!

I might take a while, but sure.

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

Actually, trying to reproduce the error on a sample, I encoutered a different problem (here's the code https://github.com/truongsinh/android_flutter_host/tree/built-in-androidx)

Cannot open a library at 'FileMapping(from=/Users/truongsinh/Dev/flutter/android_flutter_host/flutter_embedding/.android/Flutter/build/intermediates/flutter/debug/libs.jar, to=/Users/truongsinh/.gradle/caches/transforms-1/files-1.1/libs.jar/9a5b6ea5d0ab9a291353d9eb6a6ec26b/jetified-libs.jar)'
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel unknown, v1.8.3, on Mac OS X 10.14.5 18F132, locale en-VN)
 
[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[✓] Xcode - develop for iOS and macOS (Xcode 10.3)
[✓] Android Studio (version 3.4)
[✓] VS Code (version 1.36.1)
[✓] Connected device (1 available)

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

@athornz @blasten any update?

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

update from my side,

Cannot open a library at 'FileMapping(from=/Users/truongsinh/Dev/flutter/android_flutter_host/flutter_embedding/.android/Flutter/build/intermediates/flutter/debug/libs.jar, to=/Users/truongsinh/.gradle/caches/transforms-1/files-1.1/libs.jar/9a5b6ea5d0ab9a291353d9eb6a6ec26b/jetified-libs.jar)'

is solved by work around, I have to cd flutter_embedding/.android && ./gradlew assembleDebug to generate those jars file, and then I can reproduce

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: pro.truongsinh.flutter.android_flutter_host.free, PID: 11100
    java.lang.NoClassDefFoundError: Failed resolution of: Landroidx/lifecycle/DefaultLifecycleObserver;
        at io.flutter.embedding.engine.FlutterEngine.<init>(FlutterEngine.java:149)
        at pro.truongsinh.flutter.android_flutter_host.FlutterEmbeddingActivity$Companion.init(FlutterEmbeddingActivity.kt:168)
        at pro.truongsinh.flutter.android_flutter_host.MainActivity.onCreate(MainActivity.kt:17)
        at android.app.Activity.performCreate(Activity.java:7802)
        at android.app.Activity.performCreate(Activity.java:7791)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1299)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3243)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3407)
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7343)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:933)
     Caused by: java.lang.ClassNotFoundException: Didn't find class "androidx.lifecycle.DefaultLifecycleObserver" on path: DexPathList[[zip file "/data/app/pro.truongsinh.flutter.android_flutter_host.free-R3e6unzzN8OPsBvhFb5V2w==/base.apk"],nativeLibraryDirectories=[/data/app/pro.truongsinh.flutter.android_flutter_host.free-R3e6unzzN8OPsBvhFb5V2w==/lib/arm64, /data/app/pro.truongsinh.flutter.android_flutter_host.free-R3e6unzzN8OPsBvhFb5V2w==/base.apk!/lib/arm64-v8a, /system/lib64, /system/product/lib64]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:196)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:379)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
        at io.flutter.embedding.engine.FlutterEngine.<init>(FlutterEngine.java:149) 
        at pro.truongsinh.flutter.android_flutter_host.FlutterEmbeddingActivity$Companion.init(FlutterEmbeddingActivity.kt:168) 
        at pro.truongsinh.flutter.android_flutter_host.MainActivity.onCreate(MainActivity.kt:17) 
        at android.app.Activity.performCreate(Activity.java:7802) 
        at android.app.Activity.performCreate(Activity.java:7791) 
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1299) 
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3243) 
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3407) 
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83) 
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135) 
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016) 
        at android.os.Handler.dispatchMessage(Handler.java:107) 
        at android.os.Looper.loop(Looper.java:214) 
        at android.app.ActivityThread.main(ActivityThread.java:7343) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:933) 
Disconnected from the target VM, address: 'localhost:8606', transport: 'socket'
@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

Figured out, with performance degradation:

  1. I have to use lifecycle_version = "1.1.1", not 2.0.0
    def lifecycle_version = "1.1.1"
    implementation "android.arch.lifecycle:common-java8:$lifecycle_version"
  1. Even with this, my example code still crashed (something like attempt to use destroyed engine), because I use shared cached Flutter enginer, and to fix this, have to override:
    override fun retainFlutterEngineAfterHostDestruction(): Boolean {
        return true
    }

One problem though, it seems the performance of transitioning between Kotlin activity and Flutter activity is much worse than before. Previously, with cached engine, it was really smooth. Even though low performance happens on debug mode only (not profile nor release), it might still be a performance degradation from previous version (1.5.4-hotfix)

@xster

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

It's hard for us to make sure we follow up on comments on previously merged PRs. If there are still remaining issues or inconveniences with the flows, please file a new issue with repro steps.

@blasten

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

@truongsinh Btw, we will start shipping Maven artifacts. As a result, these artifacts will become the source of truth for the engine dependencies such as android.arch.lifecycle:common-java8. Once this happens, there won't be more NoClassDefFoundError.

@truongsinh

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

@truongsinh Btw, we will start shipping Maven artifacts. As a result, these artifacts will become the source of truth for the engine dependencies such as android.arch.lifecycle:common-java8. Once this happens, there won't be more NoClassDefFoundError.

Do we have a github issue or PR to track this?

@blasten

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

There's #11439

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.