From 3969e5b47b39073a72293d3874237e6cf341a698 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Mon, 1 Mar 2021 15:45:02 -0800 Subject: [PATCH] Move iOS Flutter.framework thinning into copy assemble build target (#77007) --- packages/flutter_tools/bin/xcode_backend.sh | 24 +--- .../lib/src/build_system/targets/ios.dart | 136 ++++++++---------- .../lib/src/commands/assemble.dart | 1 - .../build_system/targets/ios_test.dart | 112 ++++++++------- .../ios_content_validation_test.dart | 22 +-- 5 files changed, 130 insertions(+), 165 deletions(-) diff --git a/packages/flutter_tools/bin/xcode_backend.sh b/packages/flutter_tools/bin/xcode_backend.sh index 1f6ca64557a..5d87a23f00b 100755 --- a/packages/flutter_tools/bin/xcode_backend.sh +++ b/packages/flutter_tools/bin/xcode_backend.sh @@ -204,19 +204,6 @@ is set to release or run \"flutter build ios --release\", then re-run Archive fr return 0 } -# Destructively thins the Flutter and App frameworks to include only the specified -# architectures. -ThinAppFrameworks() { - RunCommand "${FLUTTER_ROOT}/bin/flutter" \ - ${verbose_flag} \ - assemble \ - --no-version-check \ - --output="${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}" \ - -dTargetPlatform=ios \ - -dIosArchs="${ARCHS}" \ - "thin_ios_application_frameworks" -} - # Adds the App.framework as an embedded binary and the flutter_assets as # resources. EmbedFlutterFrameworks() { @@ -273,11 +260,6 @@ AddObservatoryBonjourService() { fi } -EmbedAndThinFrameworks() { - EmbedFlutterFrameworks - ThinAppFrameworks -} - # Main entry point. if [[ $# == 0 ]]; then # Named entry points were introduced in Flutter v0.0.7. @@ -288,11 +270,13 @@ else "build") BuildApp ;; "thin") - ThinAppFrameworks ;; + # No-op, thinning is handled during the bundle asset assemble build target. + ;; "embed") EmbedFlutterFrameworks ;; "embed_and_thin") - EmbedAndThinFrameworks ;; + # Thinning is handled during the bundle asset assemble build target, so just embed. + EmbedFlutterFrameworks ;; "test_observatory_bonjour_service") # Exposed for integration testing only. AddObservatoryBonjourService ;; diff --git a/packages/flutter_tools/lib/src/build_system/targets/ios.dart b/packages/flutter_tools/lib/src/build_system/targets/ios.dart index 60ad4977dea..428d838069a 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/ios.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/ios.dart @@ -277,6 +277,14 @@ abstract class UnpackIOS extends Target { if (environment.defines[kSdkRoot] == null) { throw MissingDefineException(kSdkRoot, name); } + if (environment.defines[kIosArchs] == null) { + throw MissingDefineException(kIosArchs, name); + } + await _copyFramework(environment); + await _thinFramework(environment); + } + + Future _copyFramework(Environment environment) async { final Directory sdkRoot = environment.fileSystem.directory(environment.defines[kSdkRoot]); final EnvironmentType environmentType = environmentTypeFromSdkroot(sdkRoot); final String basePath = environment.artifacts.getArtifactPath( @@ -302,6 +310,59 @@ abstract class UnpackIOS extends Target { ); } } + + /// Destructively thin Flutter.framework to include only the specified architectures. + Future _thinFramework(Environment environment) async { + final Directory frameworkDirectory = environment.outputDir; + + final File flutterFramework = frameworkDirectory.childDirectory('Flutter.framework').childFile('Flutter'); + final String binaryPath = flutterFramework.path; + if (!flutterFramework.existsSync()) { + throw Exception('Binary $binaryPath does not exist, cannot thin'); + } + final String archs = environment.defines[kIosArchs]; + final List archList = archs.split(' ').toList(); + final ProcessResult infoResult = environment.processManager.runSync([ + 'lipo', + '-info', + binaryPath, + ]); + final String lipoInfo = infoResult.stdout as String; + + final ProcessResult verifyResult = environment.processManager.runSync([ + 'lipo', + binaryPath, + '-verify_arch', + ...archList + ]); + + if (verifyResult.exitCode != 0) { + throw Exception('Binary $binaryPath does not contain $archs. Running lipo -info:\n$lipoInfo'); + } + + // Skip thinning for non-fat executables. + if (lipoInfo.startsWith('Non-fat file:')) { + environment.logger.printTrace('Skipping lipo for non-fat file $binaryPath'); + return; + } + + // Thin in-place. + final ProcessResult extractResult = environment.processManager.runSync([ + 'lipo', + '-output', + binaryPath, + for (final String arch in archList) + ...[ + '-extract', + arch, + ], + ...[binaryPath], + ]); + + if (extractResult.exitCode != 0) { + throw Exception('Failed to extract $archs for $binaryPath.\n${extractResult.stderr}\nRunning lipo -info:\n$lipoInfo'); + } + } } /// Unpack the release prebuilt engine framework. @@ -534,78 +595,3 @@ Future createStubAppFramework(File outputFile, String sdkRoot, } } } - -/// Destructively thins the Flutter.framework to include only the specified architectures. -/// -/// This target is not fingerprinted and will always run. -class ThinIosApplicationFrameworks extends Target { - const ThinIosApplicationFrameworks(); - - @override - String get name => 'thin_ios_application_frameworks'; - - @override - List get dependencies => const []; - - @override - List get inputs => const []; - - @override - List get outputs => const []; - - @override - Future build(Environment environment) async { - if (environment.defines[kIosArchs] == null) { - throw MissingDefineException(kIosArchs, 'thin_ios_application_frameworks'); - } - final Directory frameworkDirectory = environment.outputDir; - - final File flutterFramework = frameworkDirectory.childDirectory('Flutter.framework').childFile('Flutter'); - final String binaryPath = flutterFramework.path; - if (!flutterFramework.existsSync()) { - throw Exception('Binary $binaryPath does not exist, cannot thin'); - } - final String archs = environment.defines[kIosArchs]; - final List archList = archs.split(' ').toList(); - final ProcessResult infoResult = environment.processManager.runSync([ - 'lipo', - '-info', - binaryPath, - ]); - final String lipoInfo = infoResult.stdout as String; - - final ProcessResult verifyResult = environment.processManager.runSync([ - 'lipo', - binaryPath, - '-verify_arch', - ...archList - ]); - - if (verifyResult.exitCode != 0) { - throw Exception('Binary $binaryPath does not contain $archs. Running lipo -info:\n$lipoInfo'); - } - - // Skip this step for non-fat executables. - if (lipoInfo.startsWith('Non-fat file:')) { - environment.logger.printTrace('Skipping lipo for non-fat file $binaryPath'); - return; - } - - // Thin in-place. - final ProcessResult extractResult = environment.processManager.runSync([ - 'lipo', - '-output', - binaryPath, - for (final String arch in archList) - ...[ - '-extract', - arch, - ], - ...[binaryPath], - ]); - - if (extractResult.exitCode != 0) { - throw Exception('Failed to extract $archs for $binaryPath.\n${extractResult.stderr}\nRunning lipo -info:\n$lipoInfo'); - } - } -} diff --git a/packages/flutter_tools/lib/src/commands/assemble.dart b/packages/flutter_tools/lib/src/commands/assemble.dart index 22f79327f29..792a1f47741 100644 --- a/packages/flutter_tools/lib/src/commands/assemble.dart +++ b/packages/flutter_tools/lib/src/commands/assemble.dart @@ -66,7 +66,6 @@ const List _kDefaultTargets = [ DebugIosApplicationBundle(), ProfileIosApplicationBundle(), ReleaseIosApplicationBundle(), - ThinIosApplicationFrameworks(), // Windows targets UnpackWindows(), DebugBundleWindowsAssets(), diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/ios_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/ios_test.dart index 4f29fd0deff..2479e40b36d 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/ios_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/ios_test.dart @@ -239,10 +239,26 @@ void main() { Platform: () => macPlatform, }); - group('copy engine Flutter.framework', () { - testWithoutContext('iphonesimulator', () async { + group('copy and thin engine Flutter.framework', () { + Directory outputDir; + FakeCommand copyPhysicalFrameworkCommand; + + setUp(() { final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('output'); + outputDir = fileSystem.directory('output'); + copyPhysicalFrameworkCommand = FakeCommand(command: [ + 'rsync', + '-av', + '--delete', + '--filter', + '- .DS_Store/', + 'Artifact.flutterFramework.TargetPlatform.ios.debug.EnvironmentType.physical', + outputDir.path, + ]); + }); + + testWithoutContext('iphonesimulator', () async { + final File binary = outputDir.childDirectory('Flutter.framework').childFile('Flutter'); final Environment environment = Environment.test( fileSystem.currentDirectory, processManager: processManager, @@ -251,6 +267,7 @@ void main() { fileSystem: fileSystem, outputDir: outputDir, defines: { + kIosArchs: 'x86_64', kSdkRoot: 'path/to/iPhoneSimulator.sdk', }, ); @@ -264,47 +281,34 @@ void main() { '- .DS_Store/', 'Artifact.flutterFramework.TargetPlatform.ios.debug.EnvironmentType.simulator', outputDir.path, - ]), - ); - - await const DebugUnpackIOS().build(environment); - }); - - testWithoutContext('iphoneos', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('output'); - final Environment environment = Environment.test( - fileSystem.currentDirectory, - processManager: processManager, - artifacts: artifacts, - logger: logger, - fileSystem: fileSystem, - outputDir: outputDir, - defines: { - kSdkRoot: 'path/to/iPhoneOS.sdk', - }, + ], + onRun: () => binary.createSync(recursive: true), + ), ); processManager.addCommand( FakeCommand(command: [ - 'rsync', - '-av', - '--delete', - '--filter', - '- .DS_Store/', - 'Artifact.flutterFramework.TargetPlatform.ios.debug.EnvironmentType.physical', - outputDir.path, - ]), + 'lipo', + '-info', + binary.path, + ], stdout: 'Non-fat file:'), ); + processManager.addCommand( + FakeCommand(command: [ + 'lipo', + binary.path, + '-verify_arch', + 'x86_64', + ]), + ); await const DebugUnpackIOS().build(environment); - }); - }); - group('thin frameworks', () { - testWithoutContext('fails when frameworks missing', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('Runner.app').childDirectory('Frameworks'); + expect(logger.traceText, contains('Skipping lipo for non-fat file output/Flutter.framework/Flutter')); + expect(processManager.hasRemainingExpectations, isFalse); + }); + + testWithoutContext('thinning fails when frameworks missing', () async { final Environment environment = Environment.test( fileSystem.currentDirectory, processManager: processManager, @@ -314,10 +318,12 @@ void main() { outputDir: outputDir, defines: { kIosArchs: 'arm64', + kSdkRoot: 'path/to/iPhoneOS.sdk', }, ); + processManager.addCommand(copyPhysicalFrameworkCommand); expect( - const ThinIosApplicationFrameworks().build(environment), + const DebugUnpackIOS().build(environment), throwsA(isA().having( (Exception exception) => exception.toString(), 'description', @@ -325,9 +331,7 @@ void main() { ))); }); - testWithoutContext('fails when requested archs missing from framework', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('Runner.app').childDirectory('Frameworks')..createSync(recursive: true); + testWithoutContext('thinning fails when requested archs missing from framework', () async { final File binary = outputDir.childDirectory('Flutter.framework').childFile('Flutter')..createSync(recursive: true); final Environment environment = Environment.test( @@ -339,9 +343,11 @@ void main() { outputDir: outputDir, defines: { kIosArchs: 'arm64 armv7', + kSdkRoot: 'path/to/iPhoneOS.sdk', }, ); + processManager.addCommand(copyPhysicalFrameworkCommand); processManager.addCommand( FakeCommand(command: [ 'lipo', @@ -361,7 +367,7 @@ void main() { ); expect( - const ThinIosApplicationFrameworks().build(environment), + const DebugUnpackIOS().build(environment), throwsA(isA().having( (Exception exception) => exception.toString(), 'description', @@ -369,9 +375,7 @@ void main() { ))); }); - testWithoutContext('fails when lipo extract fails', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('Runner.app').childDirectory('Frameworks')..createSync(recursive: true); + testWithoutContext('thinning fails when lipo extract fails', () async { final File binary = outputDir.childDirectory('Flutter.framework').childFile('Flutter')..createSync(recursive: true); final Environment environment = Environment.test( @@ -383,9 +387,11 @@ void main() { outputDir: outputDir, defines: { kIosArchs: 'arm64 armv7', + kSdkRoot: 'path/to/iPhoneOS.sdk', }, ); + processManager.addCommand(copyPhysicalFrameworkCommand); processManager.addCommand( FakeCommand(command: [ 'lipo', @@ -419,17 +425,15 @@ void main() { ); expect( - const ThinIosApplicationFrameworks().build(environment), + const DebugUnpackIOS().build(environment), throwsA(isA().having( (Exception exception) => exception.toString(), 'description', - contains('Failed to extract arm64 armv7 for Runner.app/Frameworks/Flutter.framework/Flutter.\nlipo error\nRunning lipo -info:\nArchitectures in the fat file:'), + contains('Failed to extract arm64 armv7 for output/Flutter.framework/Flutter.\nlipo error\nRunning lipo -info:\nArchitectures in the fat file:'), ))); }); testWithoutContext('skips thin frameworks', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('Runner.app').childDirectory('Frameworks')..createSync(recursive: true); final File binary = outputDir.childDirectory('Flutter.framework').childFile('Flutter')..createSync(recursive: true); final Environment environment = Environment.test( @@ -441,9 +445,11 @@ void main() { outputDir: outputDir, defines: { kIosArchs: 'arm64', + kSdkRoot: 'path/to/iPhoneOS.sdk', }, ); + processManager.addCommand(copyPhysicalFrameworkCommand); processManager.addCommand( FakeCommand(command: [ 'lipo', @@ -460,16 +466,14 @@ void main() { 'arm64', ]), ); - await const ThinIosApplicationFrameworks().build(environment); + await const DebugUnpackIOS().build(environment); - expect(logger.traceText, contains('Skipping lipo for non-fat file Runner.app/Frameworks/Flutter.framework/Flutter')); + expect(logger.traceText, contains('Skipping lipo for non-fat file output/Flutter.framework/Flutter')); expect(processManager.hasRemainingExpectations, isFalse); }); testWithoutContext('thins fat frameworks', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - final Directory outputDir = fileSystem.directory('Runner.app').childDirectory('Frameworks')..createSync(recursive: true); final File binary = outputDir.childDirectory('Flutter.framework').childFile('Flutter')..createSync(recursive: true); final Environment environment = Environment.test( @@ -481,9 +485,11 @@ void main() { outputDir: outputDir, defines: { kIosArchs: 'arm64 armv7', + kSdkRoot: 'path/to/iPhoneOS.sdk', }, ); + processManager.addCommand(copyPhysicalFrameworkCommand); processManager.addCommand( FakeCommand(command: [ 'lipo', @@ -515,7 +521,7 @@ void main() { ]), ); - await const ThinIosApplicationFrameworks().build(environment); + await const DebugUnpackIOS().build(environment); expect(processManager.hasRemainingExpectations, isFalse); }); }); diff --git a/packages/flutter_tools/test/integration.shard/ios_content_validation_test.dart b/packages/flutter_tools/test/integration.shard/ios_content_validation_test.dart index 2cc46a26fa3..a31562c2919 100644 --- a/packages/flutter_tools/test/integration.shard/ios_content_validation_test.dart +++ b/packages/flutter_tools/test/integration.shard/ios_content_validation_test.dart @@ -23,6 +23,7 @@ void main() { Directory buildPath; Directory outputApp; + Directory frameworkDirectory; Directory outputFlutterFramework; File outputFlutterFrameworkBinary; Directory outputAppFramework; @@ -70,22 +71,11 @@ void main() { outputApp = buildPath.childDirectory('Runner.app'); - outputFlutterFramework = fileSystem.directory( - fileSystem.path.join( - outputApp.path, - 'Frameworks', - 'Flutter.framework', - ), - ); - + frameworkDirectory = outputApp.childDirectory('Frameworks'); + outputFlutterFramework = frameworkDirectory.childDirectory('Flutter.framework'); outputFlutterFrameworkBinary = outputFlutterFramework.childFile('Flutter'); - outputAppFramework = fileSystem.directory(fileSystem.path.join( - outputApp.path, - 'Frameworks', - 'App.framework', - )); - + outputAppFramework = frameworkDirectory.childDirectory('App.framework'); outputAppFrameworkBinary = outputAppFramework.childFile('App'); }); @@ -94,6 +84,8 @@ void main() { }); testWithoutContext('flutter build ios builds a valid app', () { + // Should only contain Flutter.framework and App.framework. + expect(frameworkDirectory.listSync().length, 2); expect(outputAppFramework.childFile('App'), exists); final File vmSnapshot = fileSystem.file(fileSystem.path.join( @@ -195,8 +187,6 @@ void main() { 'VERBOSE_SCRIPT_LOGGING': '1', 'FLUTTER_BUILD_MODE': 'release', 'ACTION': 'install', - 'ARCHS': 'arm64 armv7', - 'FLUTTER_ROOT': flutterRoot, // Skip bitcode stripping since we just checked that above. }, );