From 2afaa619cbb369b032b173fe0cb017d2d3a9b3ef Mon Sep 17 00:00:00 2001 From: Gray Mackall <34871572+gmackall@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:32:58 -0800 Subject: [PATCH] Migrate module templates to declarative application of the Flutter Gradle Plugin (#159770) Fixes https://github.com/flutter/flutter/issues/159729 Cases to consider: 1. Building the module as a standalone app (`flutter run`) 2. Building the module as an aar (`flutter build aar`) 3. Doing (2) and then building an android host app that depends on the aar. 4. Building the host app with it depending on the module as source. Manually tested all 4 and they all are working. Modified `build_android_host_app_with_module_aar.dart` and `build_android_host_app_with_module_source.dart` to add checks on `stderr` to ensure we don't hit the log. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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 documentation (doc comments with `///`). - [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. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [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 [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 --------- Co-authored-by: Gray Mackall --- ...uild_android_host_app_with_module_aar.dart | 28 ++++++++++++++++++- ...d_android_host_app_with_module_source.dart | 28 ++++++++++++++++++- dev/devicelab/lib/framework/utils.dart | 13 +++++++-- .../gradle/aar_init_script.gradle | 4 +++ .../module/android/gradle/build.gradle.tmpl | 17 ----------- .../settings.gradle.copy.tmpl | 25 +++++++++++++++++ .../Flutter.tmpl/build.gradle.tmpl | 13 ++++----- .../include_flutter.groovy.copy.tmpl | 4 +++ 8 files changed, 103 insertions(+), 29 deletions(-) diff --git a/dev/devicelab/bin/tasks/build_android_host_app_with_module_aar.dart b/dev/devicelab/bin/tasks/build_android_host_app_with_module_aar.dart index 97197f5abfd..3865e4a0eba 100644 --- a/dev/devicelab/bin/tasks/build_android_host_app_with_module_aar.dart +++ b/dev/devicelab/bin/tasks/build_android_host_app_with_module_aar.dart @@ -40,6 +40,8 @@ class ModuleTest { static const String buildTarget = 'module-gradle'; final String gradleVersion; + final StringBuffer stdout = StringBuffer(); + final StringBuffer stderr = StringBuffer(); Future call() async { section('Running: $buildTarget-$gradleVersion'); @@ -60,6 +62,8 @@ class ModuleTest { await flutter( 'create', options: ['--org', 'io.flutter.devicelab', '--template=module', 'hello'], + output: stdout, + stderr: stderr, ); }); @@ -68,6 +72,8 @@ class ModuleTest { await flutter( 'config', options: ['--enable-native-assets'], + output: stdout, + stderr: stderr, ); const String ffiPackageName = 'ffi_package'; @@ -86,6 +92,8 @@ class ModuleTest { await flutter( 'packages', options: ['get'], + output: stdout, + stderr: stderr, ); }); @@ -126,6 +134,8 @@ class ModuleTest { await flutter( 'packages', options: ['get'], + output: stdout, + stderr: stderr, ); }); @@ -161,6 +171,8 @@ class ModuleTest { await flutter( 'build', options: ['apk'], + output: stdout, + stderr: stderr, ); }); @@ -181,7 +193,11 @@ class ModuleTest { section('Clean build'); await inDirectory(projectDir, () async { - await flutter('clean'); + await flutter( + 'clean', + output: stdout, + stderr: stderr, + ); }); section('Make Android host app editable'); @@ -190,6 +206,8 @@ class ModuleTest { await flutter( 'make-host-app-editable', options: ['android'], + output: stdout, + stderr: stderr, ); }); @@ -199,6 +217,8 @@ class ModuleTest { await flutter( 'build', options: ['apk'], + output: stdout, + stderr: stderr, ); }); @@ -420,6 +440,12 @@ class ModuleTest { return TaskResult.failure('Failed to make assets user-readable and writable'); } + section('Check for specific log errors.'); + final String finalStderr = stderr.toString(); + if (finalStderr.contains("You are applying Flutter's main Gradle plugin imperatively")) { + return TaskResult.failure('Applied the Flutter Gradle Plugin imperatively'); + } + return TaskResult.success(null); } on TaskResult catch (taskResult) { return taskResult; diff --git a/dev/devicelab/bin/tasks/build_android_host_app_with_module_source.dart b/dev/devicelab/bin/tasks/build_android_host_app_with_module_source.dart index 5a856aed8d8..9164fb193b1 100644 --- a/dev/devicelab/bin/tasks/build_android_host_app_with_module_source.dart +++ b/dev/devicelab/bin/tasks/build_android_host_app_with_module_source.dart @@ -40,6 +40,8 @@ class ModuleTest { static const String buildTarget = 'module-gradle'; final String gradleVersion; + final StringBuffer stdout = StringBuffer(); + final StringBuffer stderr = StringBuffer(); Future call() async { section('Running: $buildTarget-$gradleVersion'); @@ -60,6 +62,8 @@ class ModuleTest { await flutter( 'create', options: ['--org', 'io.flutter.devicelab', '--template=module', 'hello'], + output: stdout, + stderr: stderr, ); }); @@ -68,6 +72,8 @@ class ModuleTest { await flutter( 'config', options: ['--enable-native-assets'], + output: stdout, + stderr: stderr, ); const String ffiPackageName = 'ffi_package'; @@ -86,6 +92,8 @@ class ModuleTest { await flutter( 'packages', options: ['get'], + output: stdout, + stderr: stderr, ); }); @@ -126,6 +134,8 @@ class ModuleTest { await flutter( 'packages', options: ['get'], + output: stdout, + stderr: stderr, ); }); @@ -135,6 +145,8 @@ class ModuleTest { await flutter( 'build', options: ['apk'], + output: stdout, + stderr: stderr, ); }); @@ -155,7 +167,11 @@ class ModuleTest { section('Clean build'); await inDirectory(projectDir, () async { - await flutter('clean'); + await flutter( + 'clean', + output: stdout, + stderr: stderr, + ); }); section('Make Android host app editable'); @@ -164,6 +180,8 @@ class ModuleTest { await flutter( 'make-host-app-editable', options: ['android'], + output: stdout, + stderr: stderr, ); }); @@ -173,6 +191,8 @@ class ModuleTest { await flutter( 'build', options: ['apk'], + output: stdout, + stderr: stderr, ); }); @@ -396,6 +416,12 @@ class ModuleTest { return TaskResult.failure('Failed to make assets user-readable and writable'); } + section('Check for specific log errors.'); + final String finalStderr = stderr.toString(); + if (finalStderr.contains("You are applying Flutter's main Gradle plugin imperatively")) { + return TaskResult.failure('Applied the Flutter Gradle Plugin imperatively'); + } + return TaskResult.success(null); } on TaskResult catch (taskResult) { return taskResult; diff --git a/dev/devicelab/lib/framework/utils.dart b/dev/devicelab/lib/framework/utils.dart index 384d8bb0444..63135acf427 100644 --- a/dev/devicelab/lib/framework/utils.dart +++ b/dev/devicelab/lib/framework/utils.dart @@ -514,12 +514,21 @@ Future flutter(String command, { // DevToolsMemoryTest in perf_tests.dart. Map? environment, String? workingDirectory, + StringBuffer? output, // if not null, the stdout will be written here + StringBuffer? stderr, // if not null, the stderr will be written here }) async { final List args = _flutterCommandArgs( command, options, driveWithDds: driveWithDds, ); - final int exitCode = await exec(path.join(flutterDirectory.path, 'bin', 'flutter'), args, - canFail: canFail, environment: environment, workingDirectory: workingDirectory); + final int exitCode = await exec( + path.join(flutterDirectory.path, 'bin', 'flutter'), + args, + canFail: canFail, + environment: environment, + workingDirectory: workingDirectory, + output: output, + stderr: stderr, + ); if (exitCode != 0 && !canFail) { await _flutterScreenshot(workingDirectory: workingDirectory); diff --git a/packages/flutter_tools/gradle/aar_init_script.gradle b/packages/flutter_tools/gradle/aar_init_script.gradle index 21918806764..d2c47259e4d 100644 --- a/packages/flutter_tools/gradle/aar_init_script.gradle +++ b/packages/flutter_tools/gradle/aar_init_script.gradle @@ -155,6 +155,10 @@ projectsEvaluated { configureProject(rootProject, rootProject.property("output-dir")) return } + if (rootProject.name == "gradle") { + // Skip the "gradle" project, we are looking only for the "android_generated" project. + return + } // The module project is the `:flutter` subproject. Project moduleProject = rootProject.subprojects.find { it.name == "flutter" } assert moduleProject != null diff --git a/packages/flutter_tools/templates/module/android/gradle/build.gradle.tmpl b/packages/flutter_tools/templates/module/android/gradle/build.gradle.tmpl index f2c623cc887..66f4f1c271d 100644 --- a/packages/flutter_tools/templates/module/android/gradle/build.gradle.tmpl +++ b/packages/flutter_tools/templates/module/android/gradle/build.gradle.tmpl @@ -1,18 +1,5 @@ // Generated file. Do not edit. -buildscript { - ext.kotlin_version = "{{kotlinVersion}}" - repositories { - google() - mavenCentral() - } - - dependencies { - classpath("com.android.tools.build:gradle:{{agpVersionForModule}}") - classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version") - } -} - allprojects { repositories { google() @@ -31,7 +18,3 @@ android { targetSdk = {{targetSdkVersion}} } } - -dependencies { - implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version") -} diff --git a/packages/flutter_tools/templates/module/android/host_app_ephemeral/settings.gradle.copy.tmpl b/packages/flutter_tools/templates/module/android/host_app_ephemeral/settings.gradle.copy.tmpl index b40f9f78178..4498ec041eb 100644 --- a/packages/flutter_tools/templates/module/android/host_app_ephemeral/settings.gradle.copy.tmpl +++ b/packages/flutter_tools/templates/module/android/host_app_ephemeral/settings.gradle.copy.tmpl @@ -1,4 +1,29 @@ // Generated file. Do not edit. + +pluginManagement { + def flutterSdkPath = { + def properties = new Properties() + file("local.properties").withInputStream { properties.load(it) } + def flutterSdkPath = properties.getProperty("flutter.sdk") + assert flutterSdkPath != null, "flutter.sdk not set in local.properties" + return flutterSdkPath + }() + + includeBuild("$flutterSdkPath/packages/flutter_tools/gradle") + + repositories { + google() + mavenCentral() + gradlePluginPortal() + } +} + +plugins { + id "dev.flutter.flutter-plugin-loader" version "1.0.0" + id "com.android.library" version "8.7.0" apply false + id "org.jetbrains.kotlin.android" version "1.8.22" apply false +} + include ':app' rootProject.name = 'android_generated' diff --git a/packages/flutter_tools/templates/module/android/library_new_embedding/Flutter.tmpl/build.gradle.tmpl b/packages/flutter_tools/templates/module/android/library_new_embedding/Flutter.tmpl/build.gradle.tmpl index 9a10bea781f..128634409ee 100644 --- a/packages/flutter_tools/templates/module/android/library_new_embedding/Flutter.tmpl/build.gradle.tmpl +++ b/packages/flutter_tools/templates/module/android/library_new_embedding/Flutter.tmpl/build.gradle.tmpl @@ -1,5 +1,10 @@ // Generated file. Do not edit. +plugins { + id "com.android.library" + id "dev.flutter.flutter-gradle-plugin" +} + def localProperties = new Properties() def localPropertiesFile = new File(buildscript.sourceFile.parentFile.parentFile, "local.properties") if (localPropertiesFile.exists()) { @@ -8,11 +13,6 @@ if (localPropertiesFile.exists()) { } } -def flutterRoot = localProperties.getProperty("flutter.sdk") -if (flutterRoot == null) { - throw new GradleException("Flutter SDK not found. Define location with flutter.sdk in the local.properties file.") -} - def flutterVersionCode = localProperties.getProperty("flutter.versionCode") if (flutterVersionCode == null) { flutterVersionCode = "1" @@ -23,9 +23,6 @@ if (flutterVersionName == null) { flutterVersionName = "1.0" } -apply plugin: "com.android.library" -apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle" - group = "{{androidIdentifier}}" version = "1.0" diff --git a/packages/flutter_tools/templates/module/android/library_new_embedding/include_flutter.groovy.copy.tmpl b/packages/flutter_tools/templates/module/android/library_new_embedding/include_flutter.groovy.copy.tmpl index 181d08ba6da..3f6992279a9 100644 --- a/packages/flutter_tools/templates/module/android/library_new_embedding/include_flutter.groovy.copy.tmpl +++ b/packages/flutter_tools/templates/module/android/library_new_embedding/include_flutter.groovy.copy.tmpl @@ -26,3 +26,7 @@ localPropertiesFile.withReader("UTF-8") { reader -> properties.load(reader) } def flutterSdkPath = properties.getProperty("flutter.sdk") assert flutterSdkPath != null, "flutter.sdk not set in local.properties" gradle.apply from: "$flutterSdkPath/packages/flutter_tools/gradle/module_plugin_loader.gradle" + +gradle.pluginManagement{ + includeBuild("$flutterSdkPath/packages/flutter_tools/gradle") +}