Skip to content

Conversation

@jakobr-google
Copy link
Contributor

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.

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

Choose a reason for hiding this comment

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

What's the plan to resolve #7504 ? We might want to fix that issue before we make this template the default because if we need to change anything in this file, we'll be stuck without a fix for #7504

@@ -0,0 +1,229 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
Copy link
Contributor

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.

Copy link
Contributor

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.)

Copy link
Contributor

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.

Copy link
Contributor

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.)

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@jakobr-google
Copy link
Contributor Author

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.

@mit-mit
Copy link
Member

mit-mit commented Feb 9, 2017

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

Choose a reason for hiding this comment

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

Neat

@abarth
Copy link
Contributor

abarth commented Feb 9, 2017

LGTM

@abarth
Copy link
Contributor

abarth commented Feb 9, 2017

Looks fabulous.

@eseidelGoogle
Copy link
Contributor

@abarth tells me this fixes #7504.

@jakobr-google jakobr-google merged commit b246c5e into flutter:master Feb 10, 2017
@jakobr-google jakobr-google deleted the template branch February 10, 2017 08:37
@Hixie
Copy link
Contributor

Hixie commented Feb 10, 2017

This broke the basic_material_app_size benchmark.

@Hixie
Copy link
Contributor

Hixie commented Feb 13, 2017

@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.

@jakobr-google
Copy link
Contributor Author

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.

@jakobr-google
Copy link
Contributor Author

Fixed in #8137.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter create new project template should be built with gradle (like hello_services)

6 participants