From 8b8d368d2b4b38afc4b05981cf25c98217425c35 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Tue, 8 May 2018 17:28:53 -0700 Subject: [PATCH] Replace --prefer-shared-library with --build-shared-library (#17394) This replaces the --prefer-shared-library flag, which falls back to regular (non-shared-lib) compile if the NDK is not found, with the --build-shared-library flag, which exits with an error message if the NDK is not found. This simplifies the set of allowed code paths through AOT compile, resulting in better testability and easier-to-follow logic. It also results in more predictable behaviour for continuous integration and other scenarios. --- .../flutter_tools/lib/src/base/build.dart | 41 +++++++++++++------ .../lib/src/commands/build_aot.dart | 18 ++++---- .../flutter_tools/test/base/build_test.dart | 32 +++++++++++++-- 3 files changed, 67 insertions(+), 24 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/build.dart b/packages/flutter_tools/lib/src/base/build.dart index ead0361f5f..402699dd16 100644 --- a/packages/flutter_tools/lib/src/base/build.dart +++ b/packages/flutter_tools/lib/src/base/build.dart @@ -137,27 +137,35 @@ class AOTSnapshotter { @required String packagesPath, @required String outputPath, @required bool previewDart2, - @required bool preferSharedLibrary, + @required bool buildSharedLibrary, IOSArch iosArch, List extraGenSnapshotOptions: const [], }) async { if (!_isValidAotPlatform(platform, buildMode)) { printError('${getNameForTargetPlatform(platform)} does not support AOT compilation.'); - return -1; + return 1; } // TODO(cbracken): replace IOSArch with TargetPlatform.ios_{armv7,arm64}. assert(platform != TargetPlatform.ios || iosArch != null); - final bool compileToSharedLibrary = preferSharedLibrary && androidSdk.ndkCompiler != null; - if (preferSharedLibrary && !compileToSharedLibrary) { - printStatus('Could not find NDK compiler. Not building in shared library mode.'); + // buildSharedLibrary is ignored for iOS builds. + if (platform == TargetPlatform.ios) + buildSharedLibrary = false; + + if (buildSharedLibrary && androidSdk.ndkCompiler == null) { + printError( + 'Could not find NDK in Android SDK at ${androidSdk.directory}.\n' + 'Unable to build with --build-shared-library\n' + 'To install the NDK, see instructions at https://developer.android.com/ndk/guides/' + ); + return 1; } final PackageMap packageMap = new PackageMap(packagesPath); final String packageMapError = packageMap.checkValid(); if (packageMapError != null) { printError(packageMapError); - return -2; + return 1; } final Directory outputDir = fs.directory(outputPath); @@ -198,7 +206,7 @@ class AOTSnapshotter { } final String assembly = fs.path.join(outputDir.path, 'snapshot_assembly.S'); - if (compileToSharedLibrary || platform == TargetPlatform.ios) { + if (buildSharedLibrary || platform == TargetPlatform.ios) { // Assembly AOT snapshot. outputPaths.add(assembly); genSnapshotArgs.add('--snapshot_kind=app-aot-assembly'); @@ -230,7 +238,7 @@ class AOTSnapshotter { final Iterable missingInputs = inputPaths.where((String p) => !fs.isFileSync(p)); if (missingInputs.isNotEmpty) { printError('Missing input files: $missingInputs from $inputPaths'); - return -3; + return 1; } // If inputs and outputs have not changed since last run, skip the build. @@ -242,7 +250,7 @@ class AOTSnapshotter { 'targetPlatform': platform.toString(), 'entryPoint': mainPath, 'dart2': previewDart2.toString(), - 'sharedLib': compileToSharedLibrary.toString(), + 'sharedLib': buildSharedLibrary.toString(), 'extraGenSnapshotOptions': extraGenSnapshotOptions.join(' '), }, depfilePaths: [depfilePath], @@ -261,7 +269,7 @@ class AOTSnapshotter { ); if (genSnapshotExitCode != 0) { printError('Dart snapshot generator failed with exit code $genSnapshotExitCode'); - return -4; + return genSnapshotExitCode; } // Write path to gen_snapshot, since snapshots have to be re-generated when we roll @@ -274,10 +282,12 @@ class AOTSnapshotter { final RunResult result = await _buildIosFramework(iosArch: iosArch, assemblyPath: assembly, outputPath: outputDir.path); if (result.exitCode != 0) return result.exitCode; - } else if (compileToSharedLibrary) { + } else if (buildSharedLibrary) { final RunResult result = await _buildAndroidSharedLibrary(assemblyPath: assembly, outputPath: outputDir.path); - if (result.exitCode != 0) + if (result.exitCode != 0) { + printError('Failed to build AOT snapshot. Compiler terminated with exit code ${result.exitCode}'); return result.exitCode; + } } // Compute and record build fingerprint. @@ -298,8 +308,10 @@ class AOTSnapshotter { final String assemblyO = fs.path.join(outputPath, 'snapshot_assembly.o'); final RunResult compileResult = await xcode.cc(commonBuildOptions.toList()..addAll(['-c', assemblyPath, '-o', assemblyO])); - if (compileResult.exitCode != 0) + if (compileResult.exitCode != 0) { + printError('Failed to compile AOT snapshot. Compiler terminated with exit code ${compileResult.exitCode}'); return compileResult; + } final String frameworkDir = fs.path.join(outputPath, 'App.framework'); fs.directory(frameworkDir).createSync(recursive: true); @@ -313,6 +325,9 @@ class AOTSnapshotter { assemblyO, ]); final RunResult linkResult = await xcode.clang(linkArgs); + if (linkResult.exitCode != 0) { + printError('Failed to link AOT snapshot. Linker terminated with exit code ${compileResult.exitCode}'); + } return linkResult; } diff --git a/packages/flutter_tools/lib/src/commands/build_aot.dart b/packages/flutter_tools/lib/src/commands/build_aot.dart index d67c2a6def..12f8839bc2 100644 --- a/packages/flutter_tools/lib/src/commands/build_aot.dart +++ b/packages/flutter_tools/lib/src/commands/build_aot.dart @@ -33,11 +33,16 @@ class BuildAotCommand extends BuildSubCommand { hide: !verboseHelp, help: 'Preview Dart 2.0 functionality.', ) + ..addFlag('build-shared-library', + negatable: false, + defaultsTo: false, + help: 'Compile to a *.so file (requires NDK when building for Android).' + ) ..addMultiOption('ios-arch', splitCommas: true, defaultsTo: defaultIOSArchs.map(getNameForIOSArch), allowed: IOSArch.values.map(getNameForIOSArch), - help: 'iOS architectures to build', + help: 'iOS architectures to build.', ) ..addMultiOption(FlutterOptions.kExtraFrontEndOptions, splitCommas: true, @@ -46,10 +51,7 @@ class BuildAotCommand extends BuildSubCommand { ..addMultiOption(FlutterOptions.kExtraGenSnapshotOptions, splitCommas: true, hide: true, - ) - ..addFlag('prefer-shared-library', - negatable: false, - help: 'Whether to prefer compiling to a *.so file (android only).'); + ); } @override @@ -115,7 +117,7 @@ class BuildAotCommand extends BuildSubCommand { packagesPath: PackageMap.globalPackagesPath, outputPath: outputPath, previewDart2: previewDart2, - preferSharedLibrary: false, + buildSharedLibrary: false, extraGenSnapshotOptions: argResults[FlutterOptions.kExtraGenSnapshotOptions], ).then((int buildExitCode) { if (buildExitCode != 0) @@ -142,16 +144,18 @@ class BuildAotCommand extends BuildSubCommand { packagesPath: PackageMap.globalPackagesPath, outputPath: outputPath, previewDart2: previewDart2, - preferSharedLibrary: argResults['prefer-shared-library'], + buildSharedLibrary: argResults['build-shared-library'], extraGenSnapshotOptions: argResults[FlutterOptions.kExtraGenSnapshotOptions], ); if (snapshotExitCode != 0) { printError('Snapshotting exited with non-zero exit code: $snapshotExitCode'); + return; } } } on String catch (error) { // Catch the String exceptions thrown from the `runCheckedSync` methods below. printError(error); + return; } status?.stop(); diff --git a/packages/flutter_tools/test/base/build_test.dart b/packages/flutter_tools/test/base/build_test.dart index 88b81b54dd..ea84410e75 100644 --- a/packages/flutter_tools/test/base/build_test.dart +++ b/packages/flutter_tools/test/base/build_test.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:convert' show json; import 'package:file/memory.dart'; +import 'package:flutter_tools/src/android/android_sdk.dart'; import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/base/build.dart'; @@ -21,6 +22,7 @@ import 'package:test/test.dart'; import '../src/context.dart'; class MockFlutterVersion extends Mock implements FlutterVersion {} +class MockAndroidSdk extends Mock implements AndroidSdk {} class MockArtifacts extends Mock implements Artifacts {} class MockXcode extends Mock implements Xcode {} @@ -287,6 +289,7 @@ void main() { _FakeGenSnapshot genSnapshot; MemoryFileSystem fs; AOTSnapshotter snapshotter; + MockAndroidSdk mockAndroidSdk; MockArtifacts mockArtifacts; MockXcode mockXcode; @@ -308,6 +311,7 @@ void main() { genSnapshot = new _FakeGenSnapshot(); snapshotter = new AOTSnapshotter(); + mockAndroidSdk = new MockAndroidSdk(); mockArtifacts = new MockArtifacts(); mockXcode = new MockXcode(); for (BuildMode mode in BuildMode.values) { @@ -320,6 +324,7 @@ void main() { }); final Map contextOverrides = { + AndroidSdk: () => mockAndroidSdk, Artifacts: () => mockArtifacts, FileSystem: () => fs, GenSnapshot: () => genSnapshot, @@ -334,7 +339,7 @@ void main() { mainPath: 'main.dill', packagesPath: '.packages', outputPath: outputPath, - preferSharedLibrary: false, + buildSharedLibrary: false, previewDart2: true, ), isNot(equals(0))); }, overrides: contextOverrides); @@ -347,7 +352,7 @@ void main() { mainPath: 'main.dill', packagesPath: '.packages', outputPath: outputPath, - preferSharedLibrary: false, + buildSharedLibrary: false, previewDart2: true, ), isNot(0)); }, overrides: contextOverrides); @@ -373,7 +378,7 @@ void main() { mainPath: 'main.dill', packagesPath: '.packages', outputPath: outputPath, - preferSharedLibrary: false, + buildSharedLibrary: false, previewDart2: true, iosArch: IOSArch.arm64, ); @@ -420,7 +425,7 @@ void main() { mainPath: 'main.dill', packagesPath: '.packages', outputPath: outputPath, - preferSharedLibrary: false, + buildSharedLibrary: false, previewDart2: true, iosArch: IOSArch.arm64, ); @@ -443,5 +448,24 @@ void main() { 'main.dill', ]); }, overrides: contextOverrides); + + testUsingContext('returns failure if buildSharedLibrary is true but no NDK is found', () async { + final String outputPath = fs.path.join('build', 'foo'); + + when(mockAndroidSdk.ndkCompiler).thenReturn(null); + + final int genSnapshotExitCode = await snapshotter.build( + platform: TargetPlatform.android_arm, + buildMode: BuildMode.release, + mainPath: 'main.dill', + packagesPath: '.packages', + outputPath: outputPath, + buildSharedLibrary: true, + previewDart2: true, + ); + + expect(genSnapshotExitCode, isNot(0)); + expect(genSnapshot.callCount, 0); + }, overrides: contextOverrides); }); }