From 818133b8b037b184eb136ba5a1663f669bcead6a Mon Sep 17 00:00:00 2001 From: Camille Simon <43054281+camsim99@users.noreply.github.com> Date: Tue, 14 Jan 2025 13:36:52 -0600 Subject: [PATCH] [Android] Actually remove dev dependencies from release builds (#161343) Revises https://github.com/flutter/flutter/pull/158026 to fix https://github.com/flutter/flutter/issues/160407. Makes a number of fixes: - Fixes Groovy logic that attempted to remove dev dependencies from release builds to use `Api` versus `api` and loop to configure the project dependencies per build type - Adds back dependency on embedding to plugin projects since this is needed regardless (the plugin may not be included in a particularly typed build but it still needs it for where it's included) - Fixes integration test to throw and error upon failure, check right APK for the release build it's testing, and configure the flag need for `.flutter-plugin-dependencies` to mark plugins as dev dependencies as expected - Uses @matanlurey's [patch](https://github.com/flutter/flutter/issues/160407#issuecomment-2547546038) to remove dev dependency from the `GeneratedPluginRegistrant` in release mode ## 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. [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 --- ..._builds_exclude_dev_dependencies_test.dart | 7 ++++-- .../gradle/src/main/groovy/flutter.groovy | 19 +++++----------- .../lib/src/flutter_plugins.dart | 22 +++++++++++++++---- packages/flutter_tools/lib/src/project.dart | 4 ++++ .../lib/src/runner/flutter_command.dart | 5 ++++- 5 files changed, 37 insertions(+), 20 deletions(-) diff --git a/dev/devicelab/bin/tasks/android_release_builds_exclude_dev_dependencies_test.dart b/dev/devicelab/bin/tasks/android_release_builds_exclude_dev_dependencies_test.dart index 2bb95682cf..4bc635beb1 100644 --- a/dev/devicelab/bin/tasks/android_release_builds_exclude_dev_dependencies_test.dart +++ b/dev/devicelab/bin/tasks/android_release_builds_exclude_dev_dependencies_test.dart @@ -15,6 +15,9 @@ Future main() async { await task(() async { try { await runProjectTest((FlutterProject flutterProject) async { + // Enable plugins being marked as dev dependncies in the .flutter-plugins-dependencies file. + await flutter('config', options: ['--explicit-package-dependencies']); + // Create dev_dependency plugin to use for test. final Directory tempDir = Directory.systemTemp.createTempSync( 'android_release_builds_exclude_dev_dependencies_test.', @@ -50,7 +53,7 @@ Future main() async { 'app', 'outputs', 'flutter-apk', - 'app-debug.apk', + 'app-$buildMode.apk', ), ); if (!apk.existsSync()) { @@ -66,7 +69,7 @@ Future main() async { final bool apkIncludesDevDependencyAsExpected = isTestingReleaseMode ? !apkIncludesDevDependency : apkIncludesDevDependency; if (!apkIncludesDevDependencyAsExpected) { - return TaskResult.failure( + throw TaskResult.failure( 'Expected to${isTestingReleaseMode ? ' not' : ''} find dev_dependency_plugin in APK built with debug mode but did${isTestingReleaseMode ? '' : ' not'}.', ); } diff --git a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy index 3d21b75ac0..b75a4601a6 100644 --- a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy +++ b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy @@ -738,13 +738,12 @@ class FlutterPlugin implements Plugin { // compile/target/min sdk values. pluginProject.extensions.create("flutter", FlutterExtension) - // Add plugin dependency to the app project. - project.android.buildTypes.each { buildType -> - String flutterBuildMode = buildModeFor(buildType) - if (flutterBuildMode != "release" || !pluginObject.dev_dependency) { - // Only add dependency on dev dependencies in non-release builds. - project.dependencies { - api(pluginProject) + // Add plugin dependency to the app project. We only want to add dependency + // for dev dependencies in non-release builds. + project.afterEvaluate { + project.android.buildTypes.all { buildType -> + if (!pluginObject.dev_dependency || buildType.name != 'release') { + project.dependencies.add("${buildType.name}Api", pluginProject) } } } @@ -760,12 +759,6 @@ class FlutterPlugin implements Plugin { if (!pluginProject.hasProperty("android")) { return } - if (flutterBuildMode == "release" && pluginObject.dev_dependency) { - // This plugin is a dev dependency and will not be included in - // the release build, so no need to add the embedding - // dependency to it. - return - } // Copy build types from the app to the plugin. // This allows to build apps with plugins and custom build types or flavors. pluginProject.android.buildTypes { diff --git a/packages/flutter_tools/lib/src/flutter_plugins.dart b/packages/flutter_tools/lib/src/flutter_plugins.dart index 97e9708fb6..a73e9dd991 100644 --- a/packages/flutter_tools/lib/src/flutter_plugins.dart +++ b/packages/flutter_tools/lib/src/flutter_plugins.dart @@ -356,18 +356,27 @@ public final class GeneratedPluginRegistrant { } '''; -List> _extractPlatformMaps(List plugins, String type) { +List> _extractPlatformMaps(Iterable plugins, String type) { return >[ for (final Plugin plugin in plugins) if (plugin.platforms[type] case final PluginPlatform platformPlugin) platformPlugin.toMap(), ]; } -Future _writeAndroidPluginRegistrant(FlutterProject project, List plugins) async { - final List methodChannelPlugins = _filterMethodChannelPlugins( +Future _writeAndroidPluginRegistrant( + FlutterProject project, + List plugins, { + required bool releaseMode, +}) async { + Iterable methodChannelPlugins = _filterMethodChannelPlugins( plugins, AndroidPlugin.kConfigKey, ); + // TODO(camsim99): Remove dev dependencies from release builds for all platforms. See https://github.com/flutter/flutter/issues/161348. + if (releaseMode) { + methodChannelPlugins = methodChannelPlugins.where((Plugin p) => !p.isDevDependency); + } + final List> androidPlugins = _extractPlatformMaps( methodChannelPlugins, AndroidPlugin.kConfigKey, @@ -1214,6 +1223,7 @@ Future injectPlugins( bool windowsPlatform = false, Iterable? allowedPlugins, DarwinDependencyManagement? darwinDependencyManagement, + bool? releaseMode, }) async { final List plugins = await findPlugins(project); final Map> pluginsByPlatform = _resolvePluginImplementations( @@ -1222,7 +1232,11 @@ Future injectPlugins( ); if (androidPlatform) { - await _writeAndroidPluginRegistrant(project, pluginsByPlatform[AndroidPlugin.kConfigKey]!); + await _writeAndroidPluginRegistrant( + project, + pluginsByPlatform[AndroidPlugin.kConfigKey]!, + releaseMode: releaseMode ?? false, + ); } if (iosPlatform) { await _writeIOSPluginRegistrant(project, pluginsByPlatform[IOSPlugin.kConfigKey]!); diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index 7a5b9aeb1f..b00e070af2 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart @@ -332,6 +332,7 @@ class FlutterProject { Future regeneratePlatformSpecificTooling({ DeprecationBehavior deprecationBehavior = DeprecationBehavior.none, Iterable? allowedPlugins, + bool? releaseMode, }) async { return ensureReadyForPlatformSpecificTooling( androidPlatform: android.existsSync(), @@ -344,6 +345,7 @@ class FlutterProject { webPlatform: featureFlags.isWebEnabled && web.existsSync(), deprecationBehavior: deprecationBehavior, allowedPlugins: allowedPlugins, + releaseMode: releaseMode, ); } @@ -358,6 +360,7 @@ class FlutterProject { bool webPlatform = false, DeprecationBehavior deprecationBehavior = DeprecationBehavior.none, Iterable? allowedPlugins, + bool? releaseMode, }) async { if (!directory.existsSync() || isPlugin) { return; @@ -389,6 +392,7 @@ class FlutterProject { macOSPlatform: macOSPlatform, windowsPlatform: windowsPlatform, allowedPlugins: allowedPlugins, + releaseMode: releaseMode, ); } diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 00ff791e09..57a5bfe4af 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -1907,7 +1907,10 @@ Run 'flutter -h' (or 'flutter -h') for available flutter commands and // The preview device does not currently support any plugins. allowedPlugins = PreviewDevice.supportedPubPlugins; } - await project.regeneratePlatformSpecificTooling(allowedPlugins: allowedPlugins); + await project.regeneratePlatformSpecificTooling( + allowedPlugins: allowedPlugins, + releaseMode: featureFlags.isExplicitPackageDependenciesEnabled && getBuildMode().isRelease, + ); if (reportNullSafety) { await _sendNullSafetyAnalyticsEvents(project); }