-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Make new project template gradle-based for Android. #7902
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
Conversation
With this change, the 'new project' template uses the same gradle-based build for Android as the hello_services example. This has some implications on build performance, since we're now building a complete Android app instead of just combining a pre-compiled .dex with the Flutter assets. The very first build is a little over 2x slower, since it needs to download gradle and build the build scripts before getting to the actual code. Subsequent builds with changes to the code are comparable to the old builds. Null builds are faster. Enabling the gradle daemon speeds up subsequent builds by around 5s.
| import org.gradle.api.tasks.TaskAction | ||
| import org.gradle.api.tasks.bundling.Jar | ||
|
|
||
| class FlutterPlugin implements Plugin<Project> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -0,0 +1,229 @@ | |||
| // Copyright 2016 The Chromium Authors. All rights reserved. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of boilerplate. Is there any way we can factor this out into code that's reused?
Having a lot of code here means:
- If we ever have to change it, we'll lose any users who have already created an app with the old template.
- Users will have all this code they don't understand and didn't write in their codebase, and will likely fiddle with it in ways that break them and we won't understand what's wrong.
If we keep this code we should document the heck out of it, like we do the default main.dart template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Developers aren't supposed to edit this file. We should probably say that explicitly and have the build check that it hasn't been modified. (It's in a directory that developers normally don't expect to modify, but we should make that explicit.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're not going to allow it to be modified, then we shouldn't include it in the template. We should just pull it in at build time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(That would solve both problems above.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can try that, but I'm not sure it's possible. I think the way gradle works is that it uses shells scripts to bootstrap a java build of a tool that then builds your code. This is code that gets linked into the Java tool, and we don't want to mess with their shells scripts, so there might not be a way to jam it in later. Maybe it's possible to reference with some sort of path? It seemed to be using Java-style magic directories when I looked at it before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a couple of ideas I'll try out, to see if we can consume the plugin from the flutter root. I'll let this PR sit in the meantime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We presumably have to be able to hook into the build at some point, to build the Dart code and zip up the assets, for example. Can we provide the code at that point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FlutterPlugin is the code that hooks us into the build, by creating a FlutterTask (that builds the Dart code and zip up the assets), and wire up the dependencies in the build DAG. There are ways in Gradle to depend on external plugins by pointing at an artifact in a Maven repository, but we're special, so we need to tweak it a little.
I'm trying to avoid having flutter tools do too much other than invoke gradle at build time. Ideally, this should also work as a standalone Gradle project, so we can just open it in Android Studio and hit play, but it's probably fine to require an initial build using flutter tools that can set the path to the plugin correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this code in a library that we then link to? We're not including all of the engine code, for example. Can this code belong there? That's what we've done on the iOS side.
|
I've updated this PR. The Flutter Gradle plugin now lives in flutter_tools/gradle/flutter.gradle, and the android/app/build.gradle script includes it from there. The only issue is how we figure out where the Flutter root is. We currently store that in local.properties, so I had to add code to read that in the build.gradle file. Updated both hello_services and the new project template. PTAL. |
|
@abarth any other comments before we merge this? |
|
|
||
| apply plugin: 'com.android.application' | ||
| apply plugin: 'flutter' | ||
| apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat
|
Looks fabulous. |
|
This broke the basic_material_app_size benchmark. |
|
@jakobr-google please follow up with a fix for the basic_material_app__size benchmark failure. Feel free to contact me if you need any pointers. |
|
We're working on it. The Android SDK licences hadn't been accepted on the buildbot machines. That's fixed now, but we also need to update the test to pick up the new path for the .apk. |
|
Fixed in #8137. |

With this change, the 'new project' template uses the same gradle-based build for Android as the hello_services example. This has some implications on build performance, since we're now building a complete Android app instead of just combining a pre-compiled .dex with the Flutter assets.
The very first build is a little over 2x slower, since it needs to download gradle and build the build scripts before getting to the actual code. Subsequent builds with changes to the code are comparable to the old builds. Null builds are faster. Enabling the gradle daemon speeds up subsequent builds by around 5s.
Fixes #4796.