… variant API (flutter#193717) PR 7 ([flutter#193693](flutter#193693)) merged to `master` as `ecd1ab06865`, and `23ddd6b74e6` merges `master` into this branch, so the [Files changed](https://github.com/flutter/flutter/pull/193717/files) tab shows only PR 8's changes. PR 8 commits: - `8c9a54d8297` Unify add-to-app module wiring on the variant API - `2c28d8f8245` Merge PR 7 head `f668780fd47` (conflicts resolved to PR 7's wording where PR 8 does not change the code) - `a6be16034c8` Trim comments added by PR 8 - `b8cd39bcf81` Address review: clarify the variant gate and task registration comments - `2efbd345505` Address review: check the add-to-app module in module_host_with_custom_build_test - `d8f869ec468` Address review: share task registration stubs in FlutterPluginTest - `798ac295725` Address review: take ApplicationVariant in configureApplicationOutputs - `1366874099e` Address review: drop android.compatibility.enableLegacyApi from the qa section - `eb20bbd43de` Merge PR 7 head `25550b3fc49` (no conflicts) - `23ddd6b74e6` Merge `master` at `ecd1ab06865` (PR 7 squash-merged). `FlutterPlugin.kt`, `FlutterPluginUtils.kt` and `FlutterPluginTest.kt` conflicted only because the squash commit is not an ancestor; `master`'s copies are identical to `25550b3fc49`, already merged by `eb20bbd43de`, so this branch's side was kept. ## Description This is PR 8 of 11 in the AGP 9.1.0 / public `gradle-api` migration stack ([flutter#180137](flutter#180137), [flutter#166550](flutter#166550)). An add-to-app module (an Android library) is wired through the same `androidComponents.onVariants` path as an app. The module adds Flutter's assets and native libraries to its own variants, and the host app consumes them like any Android library. Flutter does not look up or configure the host project. With this PR, a host app with AGP 9's `android.newDsl=true` builds a Flutter module from source; on `33f39876b92` it fails with `Check failed.` while configuring `:app`. **Breaking:** `flutter.hostAppProjectName` has no effect. Setting it logs a warning. **Base:** `master`. PR 7 (flutter#193693) merged as `ecd1ab06865`. <details> <summary>Agent authored details</summary> ### Changes 1. **Module variants on `onVariants`** (`FlutterPlugin.kt` `addFlutterTasks`). For every variant, app or library, `onVariants` calls `registerFlutterCompileTask`, `registerFlutterAssetTasks` and `registerFlutterJniLibsTask`. Only application variants then call the new `configureApplicationOutputs` (per-ABI versionCodes and the flutter-apk copy), which takes an `ApplicationVariant`. There is no mid-lambda `return@onVariants`. 2. **Every module variant is configured.** `shouldCompileFlutterForVariant` returns true for a `LibraryVariant`. The command-line check (`shouldConfigureFlutterTask`, flutter#109560) compares the command line with the project's own `assemble<Variant>` names, and a host task such as `:app:assembleDemoStaging` never matches the module variant it consumes (`debug`, via `matchingFallbacks`). A module has three variants: the template's `debug` and `release`, plus the `profile` build type the plugin creates. It declares no flavors (`templates/module/android/library_new_embedding/Flutter.tmpl/build.gradle.tmpl`). The tasks are registered lazily, so a build runs only the module variant the host consumes. `--dry-run` on a host with flavor `demo` and build types `staging` (falls back to `debug`) and `prod` (falls back to `release`): | Host task | Module Flutter tasks in the task graph | | --- | --- | | `app:assembleDemoDebug`, `app:assembleDemoStaging` | `compileFlutterBuildDebug`, `copyFlutterAssetsDebug`, `copyJniLibsflutterBuildDebug` | | `app:assembleDemoRelease`, `app:assembleDemoProd` | `compileFlutterBuildRelease`, `copyFlutterAssetsRelease`, `copyJniLibsflutterBuildRelease` | 3. **Build mode from the module variant.** `registerFlutterCompileTask` reads the library variant's `buildType` and public `Component.debuggable`, so the mode follows the module variant the host consumes. 4. **jniLibs wired from the compile task provider.** `registerFlutterJniLibsTask` takes `compileTaskProvider` and sets `intermediateDir` from `compileTaskProvider.map { it.outputDirectory }`. The `tasks.matching { it.name == … }` dependency and the `findByName` provider are gone. Both copy tasks are registered only together with their compile task, so `intermediateDir` is required (not `@Optional`) in `CopyFlutterJniLibsTask` and `CopyFlutterAssetsTask`, and their `isPresent` branches are removed. 5. **`flutter.hostAppProjectName`.** `warnIfHostAppProjectNameIsSet` reads `project.providers.gradleProperty(…)` (not `rootProject.hasProperty`) and logs `Warning: The Gradle property 'flutter.hostAppProjectName' has no effect. Remove it from gradle.properties.` The `flutter.hostAppProjectName=SampleApp` line is removed from `dev/integration_tests/pure_android_host_apps/android_custom_host_app/gradle.properties`, so devicelab `module_custom_host_app_name_test` (host project `:SampleApp`) checks that a host not named `app` builds without the property. 6. **Deleted:** - From `FlutterPlugin.kt`: the host-project lookup, the `appProject.afterEvaluate` × `libraryVariants.all` × `applicationVariants.all` loop, the `merge<HostVariant>Assets.dependsOn` edge, `addFlutterDepsForModule` and `addCopyFlutterAssetsDependency`. - From `FlutterPluginUtils.kt`: `shouldConfigureFlutterTask(Project, Task)` and `buildModeFor(com.android.builder.model.BuildType)`. Both lost their last caller. 7. **Docs.** - `Migrating-Flutter-Gradle-Plugin-to-AGP-public-API.md`: decision record 5 and "Features that must break" item 6 are rewritten. - [`website-page-draft.md`](https://github.com/flutter/flutter/blob/master/docs/platforms/android/website-page-draft.md) (publishing tracked in [flutter#193713](flutter#193713)): the add-to-app section is rewritten. - The text these replace said that the warning names a removal milestone (it does not), and that a host `staging` debuggable build type gets debug artifacts (the module variant decides). It also said that `tasks.getByPath(":flutter:copyFlutterAssetsDebug")` fails with "Task with path … not found". That is false: in the scratch host below it returns a `CopyFlutterAssetsTask`. 8. **Tests.** - New `FlutterPluginTest` cases: - jniLibs wiring from the compile task provider, with no name lookups; - a module variant configured for a single unrelated command-line task, with no APK wiring; - the module build mode for `debug`, `profile` and `release`; - the `hostAppProjectName` warning, with no host lookup, and no warning when it is unset. - The skip test also checks that no jniLibs task is registered. - The new tests share stub helpers (`stubTaskRegistration`, `captureTaskConfiguration`, `setCommandLineTasks`). - devicelab `module_host_with_custom_build_test` gets a final section. The host fixture adds a build type `qa` (debuggable, falls back to `release`). The section runs `app:assembleDemoQa` with `android.newDsl=true` and `flutter.hostAppProjectName=app`. It checks that the output has the warning, and that `app-demo-qa.apk` has the Flutter assets and `libapp.so` for arm64-v8a and armeabi-v7a, and no debug assets. The existing sections already check `app:assembleDemoStaging` (debug artifacts). ### Audit resolution [PR 8 draft audit](reidbaker-agent#8 (comment)): | Item | Resolution | | --- | --- | | 1. String-based compile task lookup for jniLibs | Provider wiring, `FlutterPlugin.kt:549-553`. Unit test `onVariants wires the jniLibs copy to the output of the variant's compile task`. | | 2. Mid-block `return@onVariants` | None. `onVariants` (`:329-350`) calls helpers; app-only wiring is in `configureApplicationOutputs` (`:647`). | | 3. Redundant `dependsOn` | None on the assets copy (`:497-530`) or the jniLibs copy (`:536-560`). | | 4. Dead overload; `rootProject` read; fixture property | `shouldConfigureFlutterTask(Project, Task)` deleted. `providers.gradleProperty` at `:665`. Fixture line removed. | [PR 7 draft audit](reidbaker-agent#7 (comment)): its four items were resolved in flutter#193693. The stale "registered in `applicationVariants`" KDoc it named has no counterpart here; the `registerFlutterCompileTask` KDoc names no project type. ### Why removing the deleted code is safe - The deleted module path used `com.android.build.gradle.LibraryExtension.libraryVariants`, `AbstractAppExtension.applicationVariants`, `BaseVariant.mergeAssetsProvider` and `BaseVariantOutput.processResourcesProvider`. AGP does not register those extensions with `android.newDsl=true`, which is why `33f39876b92` fails with `Check failed.` in the host `afterEvaluate` block that looks up `LibraryExtension`. Its replacements (`LibraryVariant`, `Component.debuggable`, `Sources.assets`/`jniLibs.addGeneratedSourceDirectory`) are public `com.android.build.api` APIs. FGP compiles against AGP 9.1.1, the minimum on `master` (flutter#193813), and `Component.getDebuggable` also exists in gradle-api 8.11.1 (checked with `javap`). - Remaining `hostAppProjectName` users: none. `git grep hostAppProjectName` hits only this PR's code, tests and docs, plus an unrelated iOS `XcodeProject.hostAppProjectName` in the tool. The templates, `include_flutter.groovy`, `module_plugin_loader.gradle` and devicelab do not read it. devicelab `module_custom_host_app_name_test` (host project `:SampleApp`) passes without the fixture property. - `com.android.builder.model.BuildType` and `shouldConfigureFlutterTask(Project, Task)` have no remaining references. ### Removed comments, docstrings and tests | Removed | Reason | | --- | --- | | KDoc and two `TODO(gmackall)` comments (flutter#166550) on `addFlutterDepsForModule`; KDoc on `addCopyFlutterAssetsDependency` | Functions deleted. flutter#166550 stays open for the rest of the stack. | | The "host app build variant → Flutter variant" table comment in the module block | The mapping is gone; the module variant decides the mode. | | KDoc of `shouldConfigureFlutterTask(Project, Task)` and `buildModeFor(ModelBuildType)` | Functions deleted. | | `flutterCompileTaskName` KDoc | Nothing references the task by name, and the one-line name builder needs no doc. | | flutter#188785 comment in `registerFlutterJniLibsTask`, and the `@Optional` KDoc and "no Flutter build" comment in `CopyFlutterJniLibsTask` | The jniLibs task is registered only with its compile task. `gradle_libapp_so_packaging_test` "app:assembleAndroidTest builds when no Flutter compile task is configured" still passes. | | `@Optional` KDoc on `CopyFlutterAssetsTask.intermediateDir` (it kept `@Optional` for the module migration) | The migration is this PR. | | `FlutterPluginUtilsTest`: `buildModeFor returns profile if the BuildType has name profile`, `buildModeFor returns debug if the BuildType is debuggable`, `buildModeFor returns release if the BuildType is not debuggable and not named profile` | Overload deleted. `buildModeFor with a name and debuggable flag prefers the profile name over debuggability` covers the same cases. | | `FlutterPluginUtilsTest`: four `shouldConfigureFlutterTask` tests used a mocked `Task` | Kept with the same names and inputs, calling the `String` overload. | | `CopyFlutterAssetsTaskTest`: `clears the destination directory when there is no flutter build for the variant` | The input is required, so that case cannot happen. Replaced by `removes staged assets that the flutter build does not produce`, which keeps the stale-file check. | | Comments trimmed per review (`a6be16034c8`): the `PROP_HOST_APP_PROJECT_NAME` KDoc; the `registerFlutterJniLibsTask` KDoc sentence naming its compile task; the "An application project only has application variants." comment above the `check`; restatements in the KDoc of `shouldCompileFlutterForVariant` (PR 7's paragraph restored verbatim), `registerFlutterCompileTask`, `configureApplicationOutputs`, `warnIfHostAppProjectNameIsSet` and both `intermediateDir`s; test helper KDocs and integration test comments; migration doc item 6 | Each restated the code. What remains is why library variants are ungated, why the inputs are required, and the test intent. Added code-comment lines vs `f668780fd47`: 85 → 39. | | `packages/flutter_tools/test/integration.shard/android_add_to_app_module_test.dart` (added by this PR, then deleted per review, `2efbd345505`) | Its `qa` check moved into devicelab `module_host_with_custom_build_test`, which builds the same fixture. Its `staging` check duplicated the devicelab staging section, except for `android.newDsl=true`. The devicelab task runs presubmit on Linux, Mac and Windows (`.ci.yaml`), like the deleted test. | | Rewritten per review (`b8cd39bcf81`): the library paragraph of the `shouldCompileFlutterForVariant` KDoc, the assets `GradleException` comment, the `registerFlutterCompileTask` and `configureApplicationOutputs` KDocs | They explained too little ("What does this mean?") or restated the code. | | The `check(variant is ApplicationVariant)` that PR 7 added in the app `onVariants` path, moved by this PR into `configureApplicationOutputs` (`798ac295725`) | The call site branches on `variant is ApplicationVariant`, and the function takes an `ApplicationVariant`, so the compiler enforces the type. In an application project every variant is an `ApplicationVariant`, so the APK wiring runs for the same variants. | | `-Pandroid.compatibility.enableLegacyApi=false` in the devicelab `qa` section (`1366874099e`) | AGP 9.1.0, 9.1.1 and 9.3.1 mark the option `Deprecated(VERSION_10_0)`. Its consumers back the variant API that `android.newDsl=true` removes. | ### Behavioral and compatibility notes 1. **`flutter.hostAppProjectName` has no effect (breaking).** It logs a warning and names no removal milestone. Its only use was the host lookup. 2. **The Flutter build mode follows the module variant the host consumes.** A host `debug`/`profile`/`release` consumes the module variant of the same name. A custom host build type gets the module variant its `matchingFallbacks` select. With a debuggable `qa` that falls back to `release`, built as the only task, the APKs contain: | Build | `flutter_assets` | `libapp.so` | Flutter tasks run | | --- | --- | --- | --- | | `33f39876b92` (`newDsl=false`) | no | no | `compileFlutterBuildDebug` (not packaged) | | This PR | yes | yes | `compileFlutterBuildRelease` | 3. **A host `assemble<Variant>AndroidTest` run on its own runs the module's `flutter assemble` for that variant.** `--dry-run` of `:app:assembleDemoDebugAndroidTest` lists `:flutter:compileFlutterBuildDebug` with this PR, and only `copyJniLibsflutterBuildDebug` on `33f39876b92`. This is a cost, accepted because the module cannot map host task names to its own variants. 4. **Module assets are a generated assets source directory.** The deleted code copied them into the module's merged-assets output after `mergeAssets`, and made every build re-run `clean<MergeAssetsTask>`. Collisions resolve by source-set priority, as for apps since PR 6. 5. **`copyFlutterAssets<V>` in a module is a `CopyFlutterAssetsTask`, not a `Copy`.** Lookups by name still work; lookups typed as `Copy` fail. 6. Related open work: [flutter#191703](flutter#191703) reports an NPE in `getLegacyAndroidExtension` with `newDsl=true`. That function was removed earlier in the stack, and this PR removes the next failure on that path. A module with plugins on `newDsl=true` is not tested here. No conflict with [flutter#193682](flutter#193682) or [flutter#193610](flutter#193610) (`gradle_errors.dart`); this PR changes no message that the tool matches. [flutter#184409](flutter#184409) asks for separate library and app logic in `addFlutterTasks`; this PR shares `onVariants` and splits only the app outputs. ### Integration test coverage of the changed paths | Path | Tests | | --- | --- | | Module from source, `newDsl=true`, debuggable custom build type on `release`, `hostAppProjectName` warning | devicelab `module_host_with_custom_build_test`, `app:assembleDemoQa` section | | Module from source, flavor and custom build types, debug and release | devicelab `module_host_with_custom_build_test` (other sections; `app:assembleDemoStaging` needs the ungated `LibraryVariant`), `build_android_host_app_with_module_source` | | Host project not named `app`, without `hostAppProjectName` | devicelab `module_custom_host_app_name_test` | | Module AAR (`flutter build aar`) and AAR host | devicelab `build_aar_module_test`, `build_android_host_app_with_module_aar`; `android_obfuscate_test`, `android_plugin_new_output_dir_test` | | App path | `gradle_libapp_so_packaging_test`, `flutter_build_apk_split_per_abi_test`, `android_gradle_asset_merging_test`, `android_run_flutter_gradle_plugin_tests_test` | ### Tests run locally (macOS, JDK 17) On the head after merging `master` (`23ddd6b74e6`; FGP compiles and tests against AGP 9.1.1): - `./gradlew test --rerun-tasks` in `packages/flutter_tools/gradle`: 33 suites, 277 tests, 0 failures. - ktlint 1.5.0 with the CI `.editorconfig` and baseline: clean. - `dart format` and `dart analyze --fatal-infos` on `module_host_with_custom_build_test.dart` and `gradle_libapp_so_packaging_test.dart`: clean. - `gradle_libapp_so_packaging_test`: 4 tests pass. - devicelab, all `success`: `module_host_with_custom_build_test`, `module_custom_host_app_name_test`, `build_android_host_app_with_module_source`. On `1366874099e`: - `./gradlew test --rerun-tasks` in `packages/flutter_tools/gradle`: 33 suites, 274 tests, 0 failures. - ktlint 1.5.0 with the CI `.editorconfig` and baseline: clean. - `dart format` and `dart analyze --fatal-infos` on `module_host_with_custom_build_test.dart`: clean. - devicelab `module_host_with_custom_build_test`: `success`. The `app:assembleDemoQa` section ran `./gradlew app:assembleDemoQa -Pandroid.newDsl=true -Pflutter.hostAppProjectName=app` and logged the `flutter.hostAppProjectName` warning. On `d8f869ec468`: - `./gradlew test --rerun-tasks` in `packages/flutter_tools/gradle`: 33 suites, 274 tests, 0 failures. - ktlint 1.5.0 with the CI `.editorconfig` and baseline: clean. - `dart format` and `dart analyze --fatal-infos` on `module_host_with_custom_build_test.dart`: clean. - devicelab `module_host_with_custom_build_test`: `success`. The `app:assembleDemoQa` section logged the `flutter.hostAppProjectName` warning. On `a6be16034c8` (the merge and the trim): - `./gradlew test` in `packages/flutter_tools/gradle`: 33 suites, 274 tests, 0 failures. - ktlint 1.5.0 with the CI `.editorconfig` and baseline: clean. - `dart format` and `dart analyze --fatal-infos` on the changed Dart files: clean. - `android_add_to_app_module_test` (deleted in `2efbd345505`): passes. - devicelab `module_custom_host_app_name_test` (fixture without `flutter.hostAppProjectName`): `success`, and the log has no `hostAppProjectName` warning. On `8c9a54d8297` (before the merge of PR 7's review changes and the comment trim): - Integration tests, one at a time, all pass: `android_add_to_app_module_test` (1), `android_obfuscate_test` (2), `android_plugin_new_output_dir_test` (1), `gradle_libapp_so_packaging_test` (4), `flutter_build_apk_split_per_abi_test` (4), `android_gradle_asset_merging_test` (4), `android_run_flutter_gradle_plugin_tests_test` (2). - `android_add_to_app_module_test` on `33f39876b92`: fails with `Check failed.`. With `variant is LibraryVariant ||` removed: fails with "does not contain 'assets/flutter_assets/AssetManifest.bin'". - devicelab via `bin/test_runner.dart test --exit`, all `success`: `module_host_with_custom_build_test`, `module_custom_host_app_name_test`, `build_android_host_app_with_module_source`, `build_aar_module_test`, `build_android_host_app_with_module_aar`. - Scratch module plus the devicelab host fixture (AGP 9.3.1, Gradle 9.5.0): - Defaults: `assembleDemoStaging` has `kernel_blob.bin` and no `libapp.so`; `assembleDemoRelease` has `libapp.so` for arm64-v8a, armeabi-v7a and x86_64. - Same with `android.compatibility.enableLegacyApi=false`, and with `android.newDsl=true` added. </details> --- ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

[AGP 9] Add Helpful Error Message For Add-to-app #193682

Description

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions