Unverified Commit fd3c34c9 authored by Chris Bracken's avatar Chris Bracken Committed by GitHub

[macOS] Use arm64 snapshot in arm64 App.framework (#100504)

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
parent 69a4b82b
......@@ -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(
......
......@@ -570,6 +570,31 @@ List<DarwinArch> 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:
......
......@@ -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');
......
......@@ -384,7 +384,7 @@ void main() {
processManager.addCommands(<FakeCommand>[
FakeCommand(command: <String>[
'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: <String>[
'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}',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment