This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Generalise gen_snapshots.py to strip_bitcode.py #36496
Merged
Conversation
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
cbracken
commented
Sep 29, 2022
| # platform. | ||
| if (host_os == "mac" && (target_cpu == "arm" || target_cpu == "arm64")) { | ||
| if (host_os == "mac" && target_os != "mac" && | ||
| (target_cpu == "arm" || target_cpu == "arm64")) { |
Member
Author
There was a problem hiding this comment.
Note that this target:
- wasn't built for x64 macOS builds (correct)
- was built for arm64 macOS builds (incorrect - this target is used for iOS/Android)
The create_macos_gen_snapshots target:
- was built for both x64 and arm64 builds (correct)
In a following patch, I'll be merging create_arm_gen_snapshot and create_macos_gen_snapshots into a single target.
The purpose of these is just to generate gen_snapshot binaries suffixed with the target architecture, which is something we only do for macOS hosts at the moment.
cbracken
commented
Sep 29, 2022
| "root_out_dir") | ||
|
|
||
| input = "${host_output_dir}/gen_snapshot" | ||
| output = "${root_out_dir}/gen_snapshot_${target_cpu}" |
Member
Author
There was a problem hiding this comment.
This is the same output path where we've been historically emitted this binary. In a future patch, I'll make this consistent with the create_arm_gen_snapshot target, which emits it alongside the original in host_output_dir.
Not doing it in this patch since I'll need to verify expected paths used by the recipes first, and migrate stepwise if needed.
zanderso
reviewed
Sep 29, 2022
The gen_snapshots.py tool is used to copy an input gen_snapshot to an output path with an architecture-specific suffix. For example, to copy gen_snapshot to gen_snapshot_arm64. Bitcode segments, if any, are stripped. This moves the input/output filename hardcoding into the BUILD.gn file and generalises the logic to simply copy an input binary to an output path with bitcode segments stripped. Since the tool is no longer gen_snapshot specific, we rename it from gen_snapshots.py to strip_bitcode.py. This also renames the generalised `macos_gen_snapshots` rule to `strip_bitcode`. Since we're working on removing bitcode support from the engine, this script will eventually serve no purpose other than to copy the input binary to an output path, at which point this script, and the associated `strip_bitcode` template in `//flutter/sky/tools/macos_tools.gni` can be removed. Along with the TODO, renaming the script and the rule help ensure we'll spot this and remove it when bitcode support is removed from the engine. Finally, this fixes a dependency issue in the target //flutter/lib/snapshot:create_macos_gen_snapshots. Previously, it dependended on ":generate_snapshot_bin", but in fact, the only file it touches is gen_snapshot. This was built transitively as part of the ":generate_snapshot_bin" target, but is now depended on directly. This is pre-factoring for merging the iOS and macOS gen_snapshot creation build rules in `flutter/lib/snapshot/BUILD.gn`. Issue: flutter/flutter#103386 Issue: flutter/flutter#101138 Issue: flutter/flutter#107884
zanderso
approved these changes
Sep 29, 2022
a-wallen
approved these changes
Sep 29, 2022
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 29, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The
gen_snapshots.pytool is used to copy an inputgen_snapshotto an output path with an architecture-specific suffix. For example, to copygen_snapshottogen_snapshot_arm64. Bitcode segments, if any, are stripped.This moves the input/output filename hardcoding into the BUILD.gn file and generalises the logic to simply copy an input binary to an output path with bitcode segments stripped. Since the tool is no longer
gen_snapshotspecific, we rename it fromgen_snapshots.pytostrip_bitcode.py. This script is not used elsewhere in this repo or in our build recipes.This also renames the generalised
macos_gen_snapshotsrule tostrip_bitcode.Since we're working on removing bitcode support from the engine, this script will eventually serve no purpose other than to copy the input binary to an output path, at which point this script, and the associated
strip_bitcodetemplate in//flutter/sky/tools/macos_tools.gnican be removed.Along with the TODO, renaming the script and the rule help ensure we'll spot this and remove it when bitcode support is removed from the engine.
Finally, this fixes a dependency issue in the target
//flutter/lib/snapshot:create_macos_gen_snapshots. Previously, it depended on:generate_snapshot_bin, but in fact, the only file it touches isgen_snapshot. This was built transitively as part of the:generate_snapshot_bintarget, but is now depended on directly.This is pre-factoring for merging the iOS and macOS
gen_snapshotcreation build rules influtter/lib/snapshot/BUILD.gn.Issue: flutter/flutter#103386
Issue: flutter/flutter#101138
Issue: flutter/flutter#107884
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.