-
Notifications
You must be signed in to change notification settings - Fork 26.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[macOS] Use arm64 snapshot in arm64 App.framework #100504
Conversation
Gold has detected about 1 new digest(s) on patchset 1. |
e75c099
to
09a77c7
Compare
Gold has detected about 1 new digest(s) on patchset 1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change LGTM.
Recommend an integration test to validate the macOS test runs in release mode.
Suggest a macOS version of https://github.com/flutter/flutter/blob/master/dev/devicelab/bin/tasks/run_release_test.dart
See https://github.com/flutter/flutter/blob/master/dev/devicelab/bin/tasks/hot_mode_dev_cycle_macos_target__benchmark.dart for example of macOS test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
+1 |
Previously, flutter#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: flutter#100348
09a77c7
to
873ebd9
Compare
Sounds good; I'm going to land this to get |
Integration test added in #100526. |
Adds a test that invokes flutter run in release mode on macOS desktop, waits for successful launch and the flutter command list, then sends the 'q' command to quit the running app. This adds an integration test for flutter#100504. Issue: flutter#100348
Adds a test that invokes flutter run in release mode on macOS desktop, waits for successful launch and the flutter command list, then sends the 'q' command to quit the running app. This adds an integration test for #100504. Issue: #100348 (fix) Issue: #97978 (partial fix) Issue: #97977 (partial fix) Umbrella issue: #60113
Previously, #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:
gen_snapshot from the application's Dart sources.
and packaged by copying the distributed binary framework from our
artifacts cache.
Building App.framework consists of the following steps:
architecture-specific assembly code, which is then built to object
code and linked into an architecture-specific App.framework.
lipo
tool to generate a universal binary that includes bothx86_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, andwhich is now deprecated) instead of the architecture-specific
gen_snapshot_x86
andgen_snapshot_arm64
builds which emitinstructions 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 asused in the Dart SDK, for example as the suffix of
gen_snapshot
.getNameForDarwinArch
: the name for the specified architectureas used in Apple tools, for example as an argument to
lipo
. Forconsistency, and to match developer expectations on Darwin platforms,
this is also the name used in Flutter's build outputs.
Issue: #100348
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.