From 3fe6668849b17bb657e9b7bb242d44a2db3afb55 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 19 Dec 2019 13:44:21 -0800 Subject: [PATCH] [flutter_tool] ensure extraGenSnapshotArguments are forwarded to gen_snapshot from Android builds (#47059) --- packages/flutter_tools/gradle/flutter.gradle | 2 +- .../lib/src/build_system/targets/android.dart | 3 ++ .../lib/src/build_system/targets/dart.dart | 3 ++ .../lib/src/commands/assemble.dart | 15 +++++--- .../hermetic/assemble_test.dart | 12 +++++++ .../build_system/targets/android_test.dart | 35 +++++++++++++++++++ .../build_system/targets/dart_test.dart | 24 ++++++++++++- 7 files changed, 87 insertions(+), 7 deletions(-) diff --git a/packages/flutter_tools/gradle/flutter.gradle b/packages/flutter_tools/gradle/flutter.gradle index af611b68bca..c7b43ef9567 100644 --- a/packages/flutter_tools/gradle/flutter.gradle +++ b/packages/flutter_tools/gradle/flutter.gradle @@ -824,7 +824,7 @@ abstract class BaseFlutterTask extends DefaultTask { args "-dExtraFrontEndOptions=${extraFrontEndOptions}" } if (extraGenSnapshotOptions != null) { - args "-dExtraGenSnapshotOptions=${extraGenSnapshotOptions}" + args "--ExtraGenSnapshotOptions=${extraGenSnapshotOptions}" } args ruleNames } diff --git a/packages/flutter_tools/lib/src/build_system/targets/android.dart b/packages/flutter_tools/lib/src/build_system/targets/android.dart index 1798d95e3a8..15a02816c98 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/android.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/android.dart @@ -208,6 +208,8 @@ class AndroidAot extends AotElfBase { if (!output.existsSync()) { output.createSync(recursive: true); } + final List extraGenSnapshotOptions = environment.defines[kExtraGenSnapshotOptions]?.split(',') + ?? const []; final BuildMode buildMode = getBuildModeForName(environment.defines[kBuildMode]); final int snapshotExitCode = await snapshotter.build( platform: targetPlatform, @@ -216,6 +218,7 @@ class AndroidAot extends AotElfBase { packagesPath: environment.projectDir.childFile('.packages').path, outputPath: output.path, bitcode: false, + extraGenSnapshotOptions: extraGenSnapshotOptions, ); if (snapshotExitCode != 0) { throw Exception('AOT snapshotter exited with code $snapshotExitCode'); diff --git a/packages/flutter_tools/lib/src/build_system/targets/dart.dart b/packages/flutter_tools/lib/src/build_system/targets/dart.dart index aa5b95fb0b5..800cd12e711 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/dart.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/dart.dart @@ -253,6 +253,8 @@ abstract class AotElfBase extends Target { if (environment.defines[kTargetPlatform] == null) { throw MissingDefineException(kTargetPlatform, 'aot_elf'); } + final List extraGenSnapshotOptions = environment.defines[kExtraGenSnapshotOptions]?.split(',') + ?? const []; final BuildMode buildMode = getBuildModeForName(environment.defines[kBuildMode]); final TargetPlatform targetPlatform = getTargetPlatformForName(environment.defines[kTargetPlatform]); final int snapshotExitCode = await snapshotter.build( @@ -262,6 +264,7 @@ abstract class AotElfBase extends Target { packagesPath: environment.projectDir.childFile('.packages').path, outputPath: outputPath, bitcode: false, + extraGenSnapshotOptions: extraGenSnapshotOptions, ); if (snapshotExitCode != 0) { throw Exception('AOT snapshotter exited with code $snapshotExitCode'); diff --git a/packages/flutter_tools/lib/src/commands/assemble.dart b/packages/flutter_tools/lib/src/commands/assemble.dart index 5265b5c89e9..0c6fbdaf704 100644 --- a/packages/flutter_tools/lib/src/commands/assemble.dart +++ b/packages/flutter_tools/lib/src/commands/assemble.dart @@ -77,6 +77,7 @@ class AssembleCommand extends FlutterCommand { 'files will be written. Must be either absolute or relative from the ' 'root of the current Flutter project.', ); + argParser.addOption(kExtraGenSnapshotOptions); argParser.addOption( 'resource-pool-size', help: 'The maximum number of concurrent tasks the build system will run.', @@ -150,17 +151,21 @@ class AssembleCommand extends FlutterCommand { return result; } - static Map _parseDefines(List values) { + Map _parseDefines(List values) { final Map results = {}; for (String chunk in values) { - final List parts = chunk.split('='); - if (parts.length != 2) { + final int indexEquals = chunk.indexOf('='); + if (indexEquals == -1) { throwToolExit('Improperly formatted define flag: $chunk'); } - final String key = parts[0]; - final String value = parts[1]; + final String key = chunk.substring(0, indexEquals); + final String value = chunk.substring(indexEquals + 1); results[key] = value; } + // Workaround for extraGenSnapshot formatting. + if (argResults.wasParsed(kExtraGenSnapshotOptions)) { + results[kExtraGenSnapshotOptions] = argResults[kExtraGenSnapshotOptions] as String; + } return results; } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart index f854dede195..fd7e09d86de 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart @@ -32,6 +32,18 @@ void main() { expect(testLogger.traceText, contains('build succeeded.')); }); + testbed.test('Can parse defines whose values contain =', () async { + when(buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) + .thenAnswer((Invocation invocation) async { + expect((invocation.positionalArguments[1] as Environment).defines, containsPair('FooBar', 'fizz=2')); + return BuildResult(success: true); + }); + final CommandRunner commandRunner = createTestCommandRunner(AssembleCommand()); + await commandRunner.run(['assemble', '-o Output', '-dFooBar=fizz=2', 'debug_macos_bundle_flutter_assets']); + + expect(testLogger.traceText, contains('build succeeded.')); + }); + testbed.test('Throws ToolExit if not provided with output', () async { when(buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/android_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/android_test.dart index 1906275a1e3..1b557000521 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/android_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/android_test.dart @@ -123,6 +123,41 @@ void main() { GenSnapshot: () => MockGenSnapshot(), }); + testbed.test('kExtraGenSnapshotOptions passes values to gen_snapshot', () async { + final Environment environment = Environment( + outputDir: fs.directory('out')..createSync(), + projectDir: fs.currentDirectory, + buildDir: fs.currentDirectory, + defines: { + kBuildMode: 'release', + kExtraGenSnapshotOptions: 'foo,bar,baz=2', + kTargetPlatform: 'android-arm', + } + ); + environment.buildDir.createSync(recursive: true); + environment.buildDir.childFile('app.dill').createSync(); + environment.projectDir.childFile('.packages') + .writeAsStringSync('sky_engine:file:///\n'); + + when(genSnapshot.run( + snapshotType: anyNamed('snapshotType'), + darwinArch: anyNamed('darwinArch'), + additionalArgs: captureAnyNamed('additionalArgs'), + )).thenAnswer((Invocation invocation) async { + expect(invocation.namedArguments[#additionalArgs], containsAll([ + 'foo', + 'bar', + 'baz=2', + ])); + return 0; + }); + + await const AndroidAot(TargetPlatform.android_arm64, BuildMode.release) + .build(environment); + }, overrides: { + GenSnapshot: () => MockGenSnapshot(), + }); + testbed.test('android aot bundle copies so from abi directory', () async { final Environment environment = Environment( outputDir: fs.directory('out')..createSync(), diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/dart_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/dart_test.dart index db0169f2dc0..82e56be73dc 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/dart_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/dart_test.dart @@ -413,10 +413,32 @@ flutter_tools:lib/'''); expect(androidEnvironment.outputDir.childFile('app.so').existsSync(), true); })); + + test('kExtraGenSnapshotOptions passes values to gen_snapshot', () => testbed.run(() async { + androidEnvironment.defines[kExtraGenSnapshotOptions] = 'foo,bar,baz=2'; + + when(genSnapshot.run( + snapshotType: anyNamed('snapshotType'), + darwinArch: anyNamed('darwinArch'), + additionalArgs: captureAnyNamed('additionalArgs'), + )).thenAnswer((Invocation invocation) async { + expect(invocation.namedArguments[#additionalArgs], containsAll([ + 'foo', + 'bar', + 'baz=2', + ])); + return 0; + }); + + + await const AotElfRelease().build(androidEnvironment); + }, overrides: { + GenSnapshot: () => MockGenSnapshot(), + })); } class MockProcessManager extends Mock implements ProcessManager {} - +class MockGenSnapshot extends Mock implements GenSnapshot {} class MockXcode extends Mock implements Xcode {} class FakeGenSnapshot implements GenSnapshot {