android: Build universal gen_snapshot for Android #164453
Conversation
This adopts the macOS/iOS rules for creating the `gen_snapshot_arm64` and `gen_snapshot_armv7` for both arm64 targets (`gen_snapshot_arm64`) and armv7 targets (`gen_snapshot_armv7`) on both arm64 and x64 macOS hosts. The macOS and iOS rules have already been updated to generate universal binaries for each of these that can be run on both Apple Silicon and Intel Mac hosts. The `create_arm_gen_snapshot` rule remains until I'm 100% convinced it's not used for anything else. Issue: flutter#152281 Issue: flutter#69157
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
github-actions
bot
added
platform-android
cbracken
force-pushed
the
universal-gen_snapshot-android
branch
from
503601d to
d087e55
Compare
cbracken
force-pushed
the
universal-gen_snapshot-android
branch
from
d087e55 to
9d72596
Compare
| rebase_path("${output_dir}/${gen_snapshot_target_name}"), | ||
| rebase_path( | ||
| "${root_out_dir}/artifacts_$host_cpu/gen_snapshot_${target_cpu}"), | ||
| "${root_out_dir}/artifacts_$host_cpu/gen_snapshot${gen_snapshot_suffix}"), |
There was a problem hiding this comment.
Just checking, is it intentional for this to now resolve to artifacts_arm/gen_snapshot_armv7 when the target_cpu is arm?
Looks like previously only create_arm_gen_snapshot task swapped arm to armv7
There was a problem hiding this comment.
Yep this is correct. Reason is that this target was never used for 32-bit targets until now, so this hadn't been a problem. Originally, it was just used for macOS (arm64, x64), then I later added iOS (arm64 only), but now that it's also used for 32-bit arm target CPUs, this needs to handle armv7 in addition to the other two.
There was a problem hiding this comment.
Worth noting that I could probably remove the target_os subexpression from the conditional, but it does help make clear to readers exactly which target platforms we're supporting here and add a bit of future-proofing in case we one day support cross-compile to 32-bit Raspberry Pis from macOS hosts or something 😛.
cbracken
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/164453, because - The status or check suite Linux android_java17_dependency_smoke_tests has failed. Please fix the issues identified (or deflake) before re-applying this label. |
cbracken
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
3 participants
This adopts the macOS/iOS rules for creating the
gen_snapshot_arm64andgen_snapshot_armv7for both arm64 targets (gen_snapshot_arm64) and armv7 targets (gen_snapshot_armv7) on both arm64 and x64 macOS hosts. The macOS and iOS rules have already been updated to generate universal binaries for each of these that can be run on both Apple Silicon and Intel Mac hosts.The
create_arm_gen_snapshotrule remains until I'm 100% convinced it's not used for anything else. Will send a follow-up patch removing it so as not to conflate the two.No test changes since this is covered by existing build/integration tests.
Fixes: #152281
Issue: #69157
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.