From 873ebd99a80b6e05f417f2ff146e3957e4de7432 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Mon, 21 Mar 2022 15:49:13 -0700 Subject: [PATCH] [macOS] Use arm64 snapshot in arm64 App.framework Previously, https://github.com/flutter/flutter/pull/100271 enabled building universal macOS binaries by default, but included a bug causing the arm64 App.framework to be built such that the TEXT section containing the app instructions built by gen_snapshot incorrectly contained x86_64 instructions rather than arm64 instructions. When building macOS (and iOS) apps, Flutter builds them in three components: * The Runner application: built by Xcode * The bundled App.framework: built from assembly code generated by gen_snapshot from the application's Dart sources. * The bundled FlutterMacOS.framework: built as part of the engine build and packaged by copying the distributed binary framework from our artifacts cache. Building App.framework consists of the following steps: * For each architecture, invoke gen_snapshot to generate architecture-specific assembly code, which is then built to object code and linked into an architecture-specific App.framework. * Use the `lipo` tool to generate a universal binary that includes both x86_64 and arm64 architectures. Previously, we were building architecture specific App.framework binaries. However, for all architectures we were (mistakenly) invoking the general `gen_snapshot` tool (which emitted x64 instructions, and which is now deprecated) instead of the architecture-specific `gen_snapshot_x86` and `gen_snapshot_arm64` builds which emit instructions for the correct architecture. This change introduces a small refactoring, which is to split the `getNameForDarwinArch` function into two functions: * `getDartNameForDarwinArch`: the name for the specified architecture as used in the Dart SDK, for example as the suffix of `gen_snapshot`. * `getNameForDarwinArch`: the name for the specified architecture as used in Apple tools, for example as an argument to `lipo`. For consistency, and to match developer expectations on Darwin platforms, this is also the name used in Flutter's build outputs. Issue: https://github.com/flutter/flutter/issues/100348 --- .../flutter_tools/lib/src/base/build.dart | 10 +++++--- .../flutter_tools/lib/src/build_info.dart | 25 +++++++++++++++++++ .../test/general.shard/build_info_test.dart | 12 +++++++++ .../build_system/targets/macos_test.dart | 4 +-- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/build.dart b/packages/flutter_tools/lib/src/base/build.dart index fecf5d310f77..674bf4eb9355 100644 --- a/packages/flutter_tools/lib/src/base/build.dart +++ b/packages/flutter_tools/lib/src/base/build.dart @@ -64,10 +64,12 @@ class GenSnapshot { String snapshotterPath = getSnapshotterPath(snapshotType); - // iOS has a separate gen_snapshot for armv7 and arm64 in the same, - // directory. So we need to select the right one. - if (snapshotType.platform == TargetPlatform.ios) { - snapshotterPath += '_${getNameForDarwinArch(darwinArch!)}'; + // iOS and macOS have separate gen_snapshot binaries for each target + // architecture (iOS: armv7, arm64; macOS: x86_64, arm64). Select the right + // one for the target architecture in question. + if (snapshotType.platform == TargetPlatform.ios || + snapshotType.platform == TargetPlatform.darwin) { + snapshotterPath += '_${getDartNameForDarwinArch(darwinArch!)}'; } return _processUtils.stream( diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index 119e3929876a..026078467095 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -570,6 +570,31 @@ List defaultIOSArchsForEnvironment( ]; } +// Returns the Dart SDK's name for the specified target architecture. +// +// When building for Darwin platforms, the tool invokes architecture-specific +// variants of `gen_snapshot`, one for each target architecture. The output +// instructions are then built into architecture-specific binaries, which are +// merged into a universal binary using the `lipo` tool. +String getDartNameForDarwinArch(DarwinArch arch) { + switch (arch) { + case DarwinArch.armv7: + return 'armv7'; + case DarwinArch.arm64: + return 'arm64'; + case DarwinArch.x86_64: + return 'x64'; + } +} + +// Returns Apple's name for the specified target architecture. +// +// When invoking Apple tools such as `xcodebuild` or `lipo`, the tool often +// passes one or more target architectures as paramters. The names returned by +// this function reflect Apple's name for the specified architecture. +// +// For consistency with developer expectations, Flutter outputs also use these +// architecture names in its build products for Darwin target platforms. String getNameForDarwinArch(DarwinArch arch) { switch (arch) { case DarwinArch.armv7: diff --git a/packages/flutter_tools/test/general.shard/build_info_test.dart b/packages/flutter_tools/test/general.shard/build_info_test.dart index ebfe7fbaf30b..96f3976a4291 100644 --- a/packages/flutter_tools/test/general.shard/build_info_test.dart +++ b/packages/flutter_tools/test/general.shard/build_info_test.dart @@ -80,6 +80,18 @@ void main() { }); }); + testWithoutContext('getDartNameForDarwinArch returns name used in Dart SDK', () { + expect(getDartNameForDarwinArch(DarwinArch.armv7), 'armv7'); + expect(getDartNameForDarwinArch(DarwinArch.arm64), 'arm64'); + expect(getDartNameForDarwinArch(DarwinArch.x86_64), 'x64'); + }); + + testWithoutContext('getNameForDarwinArch returns Apple names', () { + expect(getNameForDarwinArch(DarwinArch.armv7), 'armv7'); + expect(getNameForDarwinArch(DarwinArch.arm64), 'arm64'); + expect(getNameForDarwinArch(DarwinArch.x86_64), 'x86_64'); + }); + testWithoutContext('getNameForTargetPlatform on Darwin arches', () { expect(getNameForTargetPlatform(TargetPlatform.ios, darwinArch: DarwinArch.arm64), 'ios-arm64'); expect(getNameForTargetPlatform(TargetPlatform.ios, darwinArch: DarwinArch.armv7), 'ios-armv7'); diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart index 6dd648007262..ff8463760a4c 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart @@ -384,7 +384,7 @@ void main() { processManager.addCommands([ FakeCommand(command: [ - 'Artifact.genSnapshot.TargetPlatform.darwin.release', + 'Artifact.genSnapshot.TargetPlatform.darwin.release_arm64', '--deterministic', '--snapshot_kind=app-aot-assembly', '--assembly=${environment.buildDir.childFile('arm64/snapshot_assembly.S').path}', @@ -392,7 +392,7 @@ void main() { environment.buildDir.childFile('app.dill').path ]), FakeCommand(command: [ - 'Artifact.genSnapshot.TargetPlatform.darwin.release', + 'Artifact.genSnapshot.TargetPlatform.darwin.release_x64', '--deterministic', '--snapshot_kind=app-aot-assembly', '--assembly=${environment.buildDir.childFile('x86_64/snapshot_assembly.S').path}',