[beta ] Revert "Ship gen_snapshot for linux-arm64 hosts targeting Android"#186818
Merged
auto-submit[bot] merged 1 commit intoMay 21, 2026
Conversation
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes support for linux-arm64 Android engine artifacts, including CI targets, build definitions, and artifact caching logic in the Flutter tool. Review feedback identifies a regression in artifacts.dart where an injected dependency was replaced by a global function and descriptive documentation was lost. Additionally, formatting changes in the test suite were flagged for violating the 80-character line limit and standard Dart formatting conventions.
Comment on lines
+603
to
+606
| // TODO(cbracken): Build Android gen_snapshot as Arm64 binary to run | ||
| // natively on Apple Silicon. See: | ||
| // https://github.com/flutter/flutter/issues/152281 | ||
| HostPlatform hostPlatform = getCurrentHostPlatform(); |
Contributor
There was a problem hiding this comment.
This change reverts the use of the injected _operatingSystemUtils back to the global getCurrentHostPlatform() function. In the Flutter tool, we prefer using injected dependencies to improve testability. Additionally, the descriptive comment about macOS universal binaries is more informative than the stale TODO it replaces (as noted in the original PR, the binary is already universal). Please consider keeping these improvements as they are unrelated to the breakage being addressed by this revert.
Suggested change
| // TODO(cbracken): Build Android gen_snapshot as Arm64 binary to run | |
| // natively on Apple Silicon. See: | |
| // https://github.com/flutter/flutter/issues/152281 | |
| HostPlatform hostPlatform = getCurrentHostPlatform(); | |
| // macOS gen_snapshot ships as a universal binary under the darwin-x64 | |
| // directory name, so remap darwin-arm64 to darwin-x64 for path resolution. | |
| HostPlatform hostPlatform = _operatingSystemUtils.hostPlatform; |
References
- Documentation should be useful: Explain the why and the how. The descriptive comment provides context that the TODO lacks. (link)
Comment on lines
+263
to
+278
| testUsingContext('should skip EngineCachedArtifacts when local engine is provided', () async { | ||
| final artifact1 = FakeSecondaryCachedArtifact()..upToDate = false; | ||
| final fileSystem = MemoryFileSystem.test(); | ||
| final cache = Cache.test(fileSystem: fileSystem, processManager: FakeProcessManager.any()); | ||
| final artifact2 = FakeEngineCachedArtifact(cache)..upToDate = false; | ||
|
|
||
| final cacheWithArtifacts = Cache.test( | ||
| fileSystem: fileSystem, | ||
| artifacts: <CachedArtifact>[artifact1, artifact2], | ||
| processManager: FakeProcessManager.any(), | ||
| ); | ||
| final cacheWithArtifacts = Cache.test( | ||
| fileSystem: fileSystem, | ||
| artifacts: <CachedArtifact>[artifact1, artifact2], | ||
| processManager: FakeProcessManager.any(), | ||
| ); | ||
|
|
||
| await cacheWithArtifacts.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal}); | ||
| expect(artifact1.didUpdate, true); | ||
| expect(artifact2.didUpdate, false); | ||
| }, | ||
| overrides: <Type, Generator>{Artifacts: () => FakeLocalEngineArtifacts()}, | ||
| ); | ||
| await cacheWithArtifacts.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal}); | ||
| expect(artifact1.didUpdate, true); | ||
| expect(artifact2.didUpdate, false); | ||
| }, overrides: <Type, Generator>{Artifacts: () => FakeLocalEngineArtifacts()}); |
Contributor
There was a problem hiding this comment.
This formatting change appears to violate the standard 80-character line limit and the idiomatic formatting for testUsingContext in this repository. Please restore the previous multi-line formatting which is more readable and adheres to dart format standards.
testUsingContext(
'should skip EngineCachedArtifacts when local engine is provided',
() async {
final artifact1 = FakeSecondaryCachedArtifact()..upToDate = false;
final fileSystem = MemoryFileSystem.test();
final cache = Cache.test(fileSystem: fileSystem, processManager: FakeProcessManager.any());
final artifact2 = FakeEngineCachedArtifact(cache)..upToDate = false;
final cacheWithArtifacts = Cache.test(
fileSystem: fileSystem,
artifacts: <CachedArtifact>[artifact1, artifact2],
processManager: FakeProcessManager.any(),
);
await cacheWithArtifacts.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal});
expect(artifact1.didUpdate, true);
expect(artifact2.didUpdate, false);
},
overrides: <Type, Generator>{Artifacts: () => FakeLocalEngineArtifacts()},
);References
- All Dart code is formatted using dart format. This is enforced by CI and typically follows an 80-character line limit. (link)
Comment on lines
+280
to
+294
| testUsingContext('should skip engine_stamp artifact when local engine is provided', () async { | ||
| final artifact1 = FakeSecondaryCachedArtifact()..upToDate = false; | ||
| final fileSystem = MemoryFileSystem.test(); | ||
| final artifact2 = FakeEngineStampArtifact()..upToDate = false; | ||
|
|
||
| final cacheWithArtifacts = Cache.test( | ||
| fileSystem: fileSystem, | ||
| artifacts: <CachedArtifact>[artifact1, artifact2], | ||
| processManager: FakeProcessManager.any(), | ||
| ); | ||
| final cacheWithArtifacts = Cache.test( | ||
| fileSystem: fileSystem, | ||
| artifacts: <CachedArtifact>[artifact1, artifact2], | ||
| processManager: FakeProcessManager.any(), | ||
| ); | ||
|
|
||
| await cacheWithArtifacts.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal}); | ||
| expect(artifact1.didUpdate, true); | ||
| expect(artifact2.didUpdate, false); | ||
| }, | ||
| overrides: <Type, Generator>{Artifacts: () => FakeLocalEngineArtifacts()}, | ||
| ); | ||
| await cacheWithArtifacts.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal}); | ||
| expect(artifact1.didUpdate, true); | ||
| expect(artifact2.didUpdate, false); | ||
| }, overrides: <Type, Generator>{Artifacts: () => FakeLocalEngineArtifacts()}); |
Contributor
There was a problem hiding this comment.
This formatting change appears to violate the standard 80-character line limit and the idiomatic formatting for testUsingContext in this repository. Please restore the previous multi-line formatting.
testUsingContext(
'should skip engine_stamp artifact when local engine is provided',
() async {
final artifact1 = FakeSecondaryCachedArtifact()..upToDate = false;
final fileSystem = MemoryFileSystem.test();
final artifact2 = FakeEngineStampArtifact()..upToDate = false;
final cacheWithArtifacts = Cache.test(
fileSystem: fileSystem,
artifacts: <CachedArtifact>[artifact1, artifact2],
processManager: FakeProcessManager.any(),
);
await cacheWithArtifacts.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal});
expect(artifact1.didUpdate, true);
expect(artifact2.didUpdate, false);
},
overrides: <Type, Generator>{Artifacts: () => FakeLocalEngineArtifacts()},
);References
- All Dart code is formatted using dart format. This is enforced by CI and typically follows an 80-character line limit. (link)
reidbaker
approved these changes
May 20, 2026
Contributor
|
autosubmit label was removed for flutter/flutter/186818, because - The status or check suite Flutter Presubmits has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Contributor
Author
|
Presubmits are failing because the beta branch cut occurred when This is blocked until that lands. |
13 tasks
…lutter#186693) Reverts: [Ship gen_snapshot for linux-arm64 hosts targeting Android](flutter#182552) Initiated by: @eyebrowsoffire Reason for reverting: We do not have any bots capable of servicing this builder, and it is breaking our releases as well as `flutter precache --all` Original PR Author: @dbebawy Reviewed By: @reidbaker The original PR description is provided below: ## Summary Enable Android **profile** and **release** builds on ARM64 Linux hosts (AWS Graviton, GCP Tau T2A, self-hosted ARM64 runners, etc.), which currently fail because `gen_snapshot` is only shipped for `linux-x64` hosts. Supersedes flutter#182275 and flutter#182276 — combined into a single atomic commit per reviewer feedback. ### Engine changes - **`BUILD.gn`**: Make `gen_snapshot` and `analyze_snapshot` zip output names host-architecture-aware (`linux-arm64.zip` vs `linux-x64.zip`) using explicit `if (host_os == "linux" && host_cpu == "arm64")` branching, matching the existing pattern for mac/windows. - **`linux_arm64_android_aot_engine.json`** (new): CI builder config following the secondary builder pattern (matching `mac_android_aot_engine.json`). Only produces host-specific archives — shared artifacts (`artifacts.zip`, `symbols.zip`, embedding/abi jars) continue to be produced by the existing x64 Linux builder. - **`.ci.yaml`**: New builder entry with `bringup: true`, `os=Linux`, `cpu=arm64`. ### Framework changes - **`flutter_cache.dart`**: Convert `_linuxBinaryDirs` from a hardcoded `linux-x64` const list to a function parameterized by host architecture, using the existing `cache.getHostPlatformArchName()` pattern from `LinuxEngineArtifacts`. - **`artifacts.dart`**: Clean up stale TODO comment — replace with accurate explanation of why `darwin-arm64` is remapped to `darwin-x64` (universal binary). No Linux remapping needed since Linux ships native binaries per host architecture. - **Tests**: Add tests for both x64 and arm64 host artifact resolution in `cache_test.dart` and `artifacts_test.dart`, following the existing `LinuxEngineArtifacts` test pattern. ### Infrastructure notes - The builder is set to `bringup: true` to allow stabilization before becoming a blocking builder. - Requires ARM64 Linux machines in the LUCI swarming pool (`os=Linux` + `cpu=arm64`). - 8 build configurations: {arm, arm64, x64, riscv64} × {profile, release}. This matches the x64 Linux builder's target coverage. The macOS equivalent has 6 builds (no riscv64). - Uses RBE (`--rbe`) for remote compilation. Fixes flutter#75864 Fixes flutter#168980 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md ## Test plan ### Verified by PR CI (x64 hosts) - [x] `linux_android_aot_engine` (x64) passed — `BUILD.gn` change still produces `linux-x64.zip` correctly - [x] `mac_android_aot_engine` passed — `darwin-x64.zip` unaffected - [x] `windows_android_aot_engine` passed — `windows-x64.zip` unaffected - [x] `ci.yaml validation` passed — builder JSON and `.ci.yaml` entry are structurally valid - [x] Archive paths are unique across all builder configs (verified locally) - [x] Unit tests pass for `AndroidGenSnapshotArtifacts.getBinaryDirs()` returning `linux-x64` entries on x64 hosts and `linux-arm64` entries on arm64 hosts - [x] Unit tests pass for `getArtifactPath` resolving `gen_snapshot` to correct host-architecture path on both x64 and arm64 Linux ### Requires ARM64 Linux hardware (post-bringup) The new `linux_arm64_android_aot_engine` builder is `bringup: true` and does not run on PR checks. The `BUILD.gn` code path (`host_cpu == "arm64"`) is only exercised on ARM64 hosts. The following need to be validated once the builder runs on ARM64 infrastructure: - [ ] `gen_snapshot` compiles and links on ARM64 Linux - [ ] CI builder produces `linux-arm64.zip` archives for all Android target CPUs (arm, arm64, x64, riscv64) in both profile and release modes - [ ] `analyze_snapshot` produces `analyze-snapshot-linux-arm64.zip` for 64-bit targets - [ ] Artifacts download correctly on an ARM64 Linux host via `flutter precache` --------- Co-authored-by: Jackson Gardner <jacksongardner@google.com>
6923c92 to
41757a7
Compare
0a9f488
into
flutter:flutter-3.45-candidate.0
8 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a cherry-pick of this revert, which is needed because the change that was reverted caused a breakage in our release build.
Reverts: Ship gen_snapshot for linux-arm64 hosts targeting Android
Initiated by: @eyebrowsoffire
Reason for reverting: We do not have any bots capable of servicing this builder, and it is breaking our releases as well as
flutter precache --allOriginal PR Author: @dbebawy
Reviewed By: @reidbaker
The original PR description is provided below:
Summary
Enable Android profile and release builds on ARM64 Linux hosts (AWS Graviton, GCP Tau T2A, self-hosted ARM64 runners, etc.), which currently fail because
gen_snapshotis only shipped forlinux-x64hosts.Supersedes #182275 and #182276 — combined into a single atomic commit per reviewer feedback.
Engine changes
BUILD.gn: Makegen_snapshotandanalyze_snapshotzip output names host-architecture-aware (linux-arm64.zipvslinux-x64.zip) using explicitif (host_os == "linux" && host_cpu == "arm64")branching, matching the existing pattern for mac/windows.linux_arm64_android_aot_engine.json(new): CI builder config following the secondary builder pattern (matchingmac_android_aot_engine.json). Only produces host-specific archives — shared artifacts (artifacts.zip,symbols.zip, embedding/abi jars) continue to be produced by the existing x64 Linux builder..ci.yaml: New builder entry withbringup: true,os=Linux,cpu=arm64.Framework changes
flutter_cache.dart: Convert_linuxBinaryDirsfrom a hardcodedlinux-x64const list to a function parameterized by host architecture, using the existingcache.getHostPlatformArchName()pattern fromLinuxEngineArtifacts.artifacts.dart: Clean up stale TODO comment — replace with accurate explanation of whydarwin-arm64is remapped todarwin-x64(universal binary). No Linux remapping needed since Linux ships native binaries per host architecture.cache_test.dartandartifacts_test.dart, following the existingLinuxEngineArtifactstest pattern.Infrastructure notes
bringup: trueto allow stabilization before becoming a blocking builder.os=Linux+cpu=arm64).--rbe) for remote compilation.Fixes #75864 Fixes #168980
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Contributor Guide:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md test-exempt:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests Flutter Style Guide:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md Features we expect every widget to implement:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement CLA: https://cla.developers.google.com/
flutter/tests: https://github.com/flutter/tests
breaking change policy:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes Discord:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md Data Driven Fixes:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Test plan
Verified by PR CI (x64 hosts)
linux_android_aot_engine(x64) passed —BUILD.gnchange still produceslinux-x64.zipcorrectlymac_android_aot_enginepassed —darwin-x64.zipunaffectedwindows_android_aot_enginepassed —windows-x64.zipunaffectedci.yaml validationpassed — builder JSON and.ci.yamlentry are structurally validAndroidGenSnapshotArtifacts.getBinaryDirs()returninglinux-x64entries on x64 hosts andlinux-arm64entries on arm64 hostsgetArtifactPathresolvinggen_snapshotto correct host-architecture path on both x64 and arm64 LinuxRequires ARM64 Linux hardware (post-bringup)
The new
linux_arm64_android_aot_enginebuilder isbringup: trueand does not run on PR checks. TheBUILD.gncode path (host_cpu == "arm64") is only exercised on ARM64 hosts. The following need to be validated once the builder runs on ARM64 infrastructure:gen_snapshotcompiles and links on ARM64 Linuxlinux-arm64.ziparchives for all Android target CPUs (arm, arm64, x64, riscv64) in both profile and release modesanalyze_snapshotproducesanalyze-snapshot-linux-arm64.zipfor 64-bit targetsflutter precacheReplace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.