-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
Flutter Gradle Plugin makes use of a few patterns that are considered to be "bad Gradle".
-
Names of AGP tasks are "guessed", instead of hooking up into official extension points.
I learned about this from Extend the Android Gradle Plugin article. Here's a paragraph that might be useful:
When interacting with AGP, use specially made extension points instead of registering the typical Gradle lifecycle callbacks (such as afterEvaluate()) or setting up explicit
Taskdependencies. Tasks created by AGP are considered implementation details and are not exposed as a public API. You must avoid trying to get instances of the Task objects or guessing theTasknames and adding callbacks or dependencies to thoseTaskobjects directly.This is not what FGP does. Instead, it finds tasks by gluing strings together:
Task copyFlutterAssetsTask = project.tasks.create( name: "copyFlutterAssets${variant.name.capitalize()}", type: Copy, ) { // ... dependsOn "clean${mergeAssets.name.capitalize()}" mergeAssets.mustRunAfter("clean${mergeAssets.name.capitalize()}") // ... } // ... def compressAssetsTask = project.tasks.findByName("compress${variant.name.capitalize()}Assets") if (compressAssetsTask) { compressAssetsTask.dependsOn copyFlutterAssetsTask }
This is exactly what the Android docs advise against.
-
Dependencies between tasks are defined explicitly, instead of being inferred on the basis of inputs/outputs. Also, FGP tasks output their files into AGP task's output directories.
I realized this after reading this great article by a former Gradle engineer.
For example,
copyFluterAssets{flavor}puts its output into the output directory ofmerge{flavor}Assetstask here. The relevant excerpt:multiple tasks contributing to the same output directory is the typical example of what would break caching. In fact, it breaks all kinds of up-to-date checking, that is to say, the ability for Gradle to understand that it doesn’t need to execute a task when nothing changed.
Let's take the
copyFlutterAssetsTaskagain as an example.Task copyFlutterAssetsTask = project.tasks.create( name: "copyFlutterAssets${variant.name.capitalize()}", type: Copy, ) { dependsOn compileTask // explicit dependency - bad with compileTask.assets if (isUsedAsSubproject) { dependsOn packageAssets // explicit dependency - bad dependsOn cleanPackageAssets // explicit dependency - bad into packageAssets.outputDir // contributes its own output into output dir of another task - bad return } dependsOn mergeAssets // explicit dependency - bad dependsOn "clean${mergeAssets.name.capitalize()}" // explicit dependency - bad mergeAssets.mustRunAfter("clean${mergeAssets.name.capitalize()}") // forcing order - bad into mergeAssets.outputDir // contributes its own output into output dir of another task - bad }
It's not anything critical, of course, but I think it's good to have these points noted somewhere and fix them in the long run. I think it'd be good to do it after #121541 because writing Kotlin is faster and more pleasant than writing Groovy :)
This is somewhat related to #119196.
Thanks @gnprice for motivating me to write this down.