From 0975e612c04a5ffe1e840908a56cb4e1dd6f6470 Mon Sep 17 00:00:00 2001 From: Mikhail Novoseltsev <51940183+Sameri11@users.noreply.github.com> Date: Tue, 1 Oct 2024 20:24:53 +0500 Subject: [PATCH] [tool][android] Allow --target-platform work properly with --debug mode (#154476) This PR addresses an issue where the `--target-platform` flag was not being respected when building APKs in debug mode. Previously, debug builds would always include `x86` and `x64` architectures, regardless of the specified target platform. This change ensures that the `--target-platform` flag is honored across all build modes, including debug. To achieve this, `BuildApkCommand` has been slightly changed to become responsible for list of archs that should be built in the current run,rather than just parsing arguments. Previously, this responsibility was distributed to gradle, which could be frustrating (in my opinion) Fixes #153359 --- .../bin/tasks/build_aar_module_test.dart | 1 - .../tasks/gradle_plugin_light_apk_test.dart | 7 - .../gradle/src/main/groovy/flutter.groovy | 5 - .../lib/src/commands/build_apk.dart | 71 ++++---- .../permeable/build_apk_test.dart | 154 +++++++++++++++++- 5 files changed, 190 insertions(+), 48 deletions(-) diff --git a/dev/devicelab/bin/tasks/build_aar_module_test.dart b/dev/devicelab/bin/tasks/build_aar_module_test.dart index f493bd2682..c30639830d 100644 --- a/dev/devicelab/bin/tasks/build_aar_module_test.dart +++ b/dev/devicelab/bin/tasks/build_aar_module_test.dart @@ -228,7 +228,6 @@ Future main() async { checkFileContains([ 'flutter_embedding_debug', - 'x86_debug', 'x86_64_debug', 'armeabi_v7a_debug', 'arm64_v8a_debug', diff --git a/dev/devicelab/bin/tasks/gradle_plugin_light_apk_test.dart b/dev/devicelab/bin/tasks/gradle_plugin_light_apk_test.dart index 252681a0a4..ee3519a39e 100644 --- a/dev/devicelab/bin/tasks/gradle_plugin_light_apk_test.dart +++ b/dev/devicelab/bin/tasks/gradle_plugin_light_apk_test.dart @@ -34,9 +34,6 @@ Future main() async { ...debugAssets, ...baseApkFiles, 'lib/armeabi-v7a/libflutter.so', - // Debug mode intentionally includes `x86` and `x86_64`. - 'lib/x86/libflutter.so', - 'lib/x86_64/libflutter.so', ], apkFiles); checkCollectionDoesNotContain([ @@ -65,9 +62,7 @@ Future main() async { ...flutterAssets, ...debugAssets, ...baseApkFiles, - // Debug mode intentionally includes `x86` and `x86_64`. 'lib/x86/libflutter.so', - 'lib/x86_64/libflutter.so', ], apkFiles); checkCollectionDoesNotContain([ @@ -96,8 +91,6 @@ Future main() async { ...flutterAssets, ...debugAssets, ...baseApkFiles, - // Debug mode intentionally includes `x86` and `x86_64`. - 'lib/x86/libflutter.so', 'lib/x86_64/libflutter.so', ], apkFiles); diff --git a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy index 0173b45312..a6f13c8ac7 100644 --- a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy +++ b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy @@ -637,11 +637,6 @@ class FlutterPlugin implements Plugin { "io.flutter:flutter_embedding_$flutterBuildMode:$engineVersion") } List platforms = getTargetPlatforms().collect() - // Debug mode includes x86 and x64, which are commonly used in emulators. - if (flutterBuildMode == "debug" && !useLocalEngine()) { - platforms.add("android-x86") - platforms.add("android-x64") - } platforms.each { platform -> String arch = PLATFORM_ARCH_MAP[platform].replace("-", "_") // Add the `libflutter.so` dependency. diff --git a/packages/flutter_tools/lib/src/commands/build_apk.dart b/packages/flutter_tools/lib/src/commands/build_apk.dart index f2dd4d546f..ca34ad7305 100644 --- a/packages/flutter_tools/lib/src/commands/build_apk.dart +++ b/packages/flutter_tools/lib/src/commands/build_apk.dart @@ -48,15 +48,43 @@ class BuildApkCommand extends BuildSubCommand { help: 'Generate build files used by flutter but ' 'do not build any artifacts.') ..addMultiOption('target-platform', - defaultsTo: ['android-arm', 'android-arm64', 'android-x64'], allowed: ['android-arm', 'android-arm64', 'android-x86', 'android-x64'], - // https://github.com/flutter/flutter/issues/153359 tracks debug build type support. help: - 'The target platform for which the app is compiled. Supports release but not debug build types.', + 'The target platform for which the app is compiled.', ); usesTrackWidgetCreation(verboseHelp: verboseHelp); } + BuildMode get _buildMode { + if (boolArg('release')) { + return BuildMode.release; + } else if (boolArg('profile')) { + return BuildMode.profile; + } else if (boolArg('debug')) { + return BuildMode.debug; + } else if (boolArg('jit-release')) { + return BuildMode.jitRelease; + } + return BuildMode.release; + } + static const List _kDefaultJitArchs = [ + 'android-arm', + 'android-arm64', + 'android-x86', + 'android-x64', + ]; + static const List _kDefaultAotArchs = [ + 'android-arm', + 'android-arm64', + 'android-x64', + ]; + List get _targetArchs => stringsArg('target-platform').isEmpty + ? switch (_buildMode) { + BuildMode.release || BuildMode.profile => _kDefaultAotArchs, + BuildMode.debug || BuildMode.jitRelease => _kDefaultJitArchs, + } + : stringsArg('target-platform'); + @override final String name = 'apk'; @@ -81,46 +109,20 @@ class BuildApkCommand extends BuildSubCommand { @override Future get usageValues async { - String buildMode; - - if (boolArg('release')) { - buildMode = 'release'; - } else if (boolArg('debug')) { - buildMode = 'debug'; - } else if (boolArg('profile')) { - buildMode = 'profile'; - } else { - // The build defaults to release. - buildMode = 'release'; - } - return CustomDimensions( - commandBuildApkTargetPlatform: stringsArg('target-platform').join(','), - commandBuildApkBuildMode: buildMode, + commandBuildApkTargetPlatform: _targetArchs.join(','), + commandBuildApkBuildMode: _buildMode.cliName, commandBuildApkSplitPerAbi: boolArg('split-per-abi'), ); } @override Future unifiedAnalyticsUsageValues(String commandPath) async { - final String buildMode; - - if (boolArg('release')) { - buildMode = 'release'; - } else if (boolArg('debug')) { - buildMode = 'debug'; - } else if (boolArg('profile')) { - buildMode = 'profile'; - } else { - // The build defaults to release. - buildMode = 'release'; - } - return Event.commandUsageValues( workflow: commandPath, commandHasTerminal: hasTerminal, - buildApkTargetPlatform: stringsArg('target-platform').join(','), - buildApkBuildMode: buildMode, + buildApkTargetPlatform: _targetArchs.join(','), + buildApkBuildMode: _buildMode.cliName, buildApkSplitPerAbi: boolArg('split-per-abi'), ); } @@ -131,10 +133,11 @@ class BuildApkCommand extends BuildSubCommand { exitWithNoSdkMessage(); } final BuildInfo buildInfo = await getBuildInfo(); + final AndroidBuildInfo androidBuildInfo = AndroidBuildInfo( buildInfo, splitPerAbi: boolArg('split-per-abi'), - targetArchs: stringsArg('target-platform').map(getAndroidArchForName), + targetArchs: _targetArchs.map(getAndroidArchForName), ); validateBuild(androidBuildInfo); displayNullSafetyMode(androidBuildInfo.buildInfo); diff --git a/packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart b/packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart index bc4dae2a71..ddec14e012 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart @@ -48,8 +48,9 @@ void main() { testUsingContext('indicate the default target platforms', () async { final String projectPath = await createProject(tempDir, arguments: ['--no-pub', '--template=app']); - await runBuildApkCommand(projectPath); + // Without buildMode flag. + await runBuildApkCommand(projectPath); expect( fakeAnalytics.sentEvents, contains( @@ -62,6 +63,157 @@ void main() { ), ), ); + + await runBuildApkCommand(projectPath, arguments: ['--debug']); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm,android-arm64,android-x86,android-x64', + buildApkBuildMode: 'debug', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand(projectPath, arguments: ['--jit-release']); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm,android-arm64,android-x86,android-x64', + buildApkBuildMode: 'jit_release', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand(projectPath, arguments: ['--profile']); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm,android-arm64,android-x64', + buildApkBuildMode: 'profile', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand(projectPath, arguments: ['--release']); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm,android-arm64,android-x64', + buildApkBuildMode: 'release', + buildApkSplitPerAbi: false, + ), + ), + ); + + }, overrides: { + AndroidBuilder: () => FakeAndroidBuilder(), + Analytics: () => fakeAnalytics, + }); + + testUsingContext('Each build mode respects --target-platform', () async { + final String projectPath = await createProject(tempDir, + arguments: ['--no-pub', '--template=app']); + + // Without buildMode flag. + await runBuildApkCommand( + projectPath, + arguments: ['--target-platform=android-arm'], + ); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm', + buildApkBuildMode: 'release', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand( + projectPath, + arguments: ['--debug', '--target-platform=android-arm'], + ); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm', + buildApkBuildMode: 'debug', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand( + projectPath, + arguments: ['--release', '--target-platform=android-arm'], + ); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm', + buildApkBuildMode: 'release', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand( + projectPath, + arguments: ['--profile', '--target-platform=android-arm'], + ); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm', + buildApkBuildMode: 'profile', + buildApkSplitPerAbi: false, + ), + ), + ); + + await runBuildApkCommand( + projectPath, + arguments: ['--jit-release', '--target-platform=android-arm'], + ); + expect( + fakeAnalytics.sentEvents, + contains( + Event.commandUsageValues( + workflow: 'apk', + commandHasTerminal: false, + buildApkTargetPlatform: 'android-arm', + buildApkBuildMode: 'jit_release', + buildApkSplitPerAbi: false, + ), + ), + ); }, overrides: { AndroidBuilder: () => FakeAndroidBuilder(), Analytics: () => fakeAnalytics,