Add copy_gen_snapshots.py tool #10430
Merged
Conversation
This file contains 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
Copies and renames the architecture-dependent iOS variants of gen_snapshot to a destination directory. Now that the armv7-emitting variant of gen_snapshot is 64-bit we can no longer bundle up gen_snapshot as a fat binary with the x86 armv7 variant merged with the x86_64 arm64 variant.
|
/cc @chinmaygarde |
liamappelbe
approved these changes
Aug 1, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 2, 2019
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Aug 2, 2019
[email protected]:flutter/engine.git/compare/bf9288597586...eac7ef0 git log bf92885..eac7ef0 --no-merges --oneline 2019-08-01 [email protected] Add copy_gen_snapshots.py tool (flutter/engine#10430) 2019-08-01 [email protected] Fix mac gen_snapshot uploader (flutter/engine#10423) 2019-08-01 [email protected] Make kernel compiler use host toolchain (flutter/engine#10419) 2019-08-01 [email protected] Use simarm_x64 when targeting arm (flutter/engine#10010) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
cbracken
added a commit
to cbracken/flutter_engine
that referenced
this pull request
Aug 2, 2019
cbracken
added a commit
that referenced
this pull request
Aug 2, 2019
cfontas
pushed a commit
to cfontas/engine
that referenced
this pull request
Aug 8, 2019
Copies and renames the architecture-dependent iOS variants of gen_snapshot to a destination directory. Now that the armv7-emitting variant of gen_snapshot is 64-bit we can no longer bundle up gen_snapshot as a fat binary with the x86 armv7 variant merged with the x86_64 arm64 variant.
cfontas
pushed a commit
to cfontas/engine
that referenced
this pull request
Aug 8, 2019
cbracken
added a commit
to cbracken/flutter
that referenced
this pull request
Apr 17, 2024
When performing artifact lookups for Artifact.genSnapshot, a TargetPlatform is used to determine the name of the tool, typically `gen_snapshot_$TARGET_ARCH`. Formerly, this tool was always named `gen_snapshot`. The astute reader may ask "but Chris, didn't we support TWO target architectures on iOS and therefore need TWO gen_snapshot binaries?" Yes, we did support both armv7 and arm64 target architectures on iOS. No, we didn't have two gen_snapshot binaries. At the time, the bitness of the gen_snapshot tool needed to match the bitness of the target architecture. As such we did *build* two gen_snapshots: * A 32-bit x86 binary that emitted armv7 AOT code * A 64-bit x64 binary that emitted arm64 AOT code However, to avoid having to do a lot of work plumbing through suffixed gen_snapshot names, the author of that work elected to "cleverly" lipo the two together into a single multi-architecture macOS binary. See: flutter/engine#4948 This was later remediated over the course of several patches, including: See: flutter#37445 See: flutter/engine#10430 See: flutter/engine#22818 However, there were still cases (notably --local-engine workflows in the tool) where we weren't computing the target platform and thus referenced the generic gen_snapshot tool. See: flutter#38933 Fixed in: flutter/engine#28345 The test removed in this PR, which ensured that null SnapshotType.platform was supported was introduced in flutter#11924 as a followup to flutter#11820 when the snapshotting logic was originally extracted to the `GenSnapshot` class. Since there are no longer any cases where TargetPlatform isn't passed when looking up Artifacts.genSnapshot, we can safely make the platform non-nullable and remove the test. This is pre-factoring towrards the removal of the generic gen_snapshot artifact, which is pre-factoring towards the goal of building gen_snapshot binaries with an arm64 host architecture, and eliminate the need to use Rosetta during iOS and macOS Flutter builds. Issue: flutter#101138 Issue: flutter#103386
9 tasks
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Apr 18, 2024
When performing artifact lookups for `Artifact.genSnapshot` for macOS desktop builds, a `TargetPlatform` is used to determine the name of the tool, typically `gen_snapshot_$TARGET_ARCH`. Formerly, this tool was always named `gen_snapshot`. The astute reader may ask "but Chris, didn't we support TWO target architectures on iOS and therefore need TWO `gen_snapshot` binaries?" Yes, we did support both armv7 and arm64 target architectures on iOS. But no, we didn't initially have two `gen_snapshot` binaries. We did *build* two `gen_snapshots`: * A 32-bit x86 binary that emitted armv7 AOT code * A 64-bit x64 binary that emitted arm64 AOT code At the time, the bitness of the `gen_snapshot` tool needed to match the bitness of the target architecture, and to avoid having to do a lot of work plumbing through suffixed `gen_snapshot` names, the author of that work (who, as evidenced by this patch, is still paying for his code crimes) elected to "cleverly" lipo the two together into a single multi-architecture macOS binary still named `gen_snapshot`. See: flutter/engine#4948 This was later remediated over the course of several patches, including: * flutter/engine#10430 * flutter/engine#22818 * #37445 However, there were still cases (notably `--local-engine` workflows in the tool) where we weren't computing the target platform and thus referenced the generic `gen_snapshot` tool. See: #38933 Fixed in: flutter/engine#28345 The test removed in this PR, which ensured that null `SnapshotType.platform` was supported was introduced in #11924 as a followup to #11820 when the snapshotting logic was originally extracted to the `GenSnapshot` class, and most invocations still passed a null target platform. Since there are no longer any cases where `TargetPlatform` isn't passed when looking up `Artifact.genSnapshot`, we can safely make the platform non-nullable and remove the test. This is pre-factoring towards the removal of the generic `gen_snapshot` artifact from the macOS host binaries (which are currently unused since we never pass a null `TargetPlatform`), which is pre-factoring towards the goal of building `gen_snapshot` binaries with an arm64 host architecture, and eliminate the need to use Rosetta during iOS and macOS Flutter builds. Part of: #101138 Umbrella issue: #103386 Umbrella issue: #69157 No new tests since the behaviour is enforced by the compiler.
gilnobrega
pushed a commit
to gilnobrega/flutter
that referenced
this pull request
Apr 22, 2024
When performing artifact lookups for `Artifact.genSnapshot` for macOS desktop builds, a `TargetPlatform` is used to determine the name of the tool, typically `gen_snapshot_$TARGET_ARCH`. Formerly, this tool was always named `gen_snapshot`. The astute reader may ask "but Chris, didn't we support TWO target architectures on iOS and therefore need TWO `gen_snapshot` binaries?" Yes, we did support both armv7 and arm64 target architectures on iOS. But no, we didn't initially have two `gen_snapshot` binaries. We did *build* two `gen_snapshots`: * A 32-bit x86 binary that emitted armv7 AOT code * A 64-bit x64 binary that emitted arm64 AOT code At the time, the bitness of the `gen_snapshot` tool needed to match the bitness of the target architecture, and to avoid having to do a lot of work plumbing through suffixed `gen_snapshot` names, the author of that work (who, as evidenced by this patch, is still paying for his code crimes) elected to "cleverly" lipo the two together into a single multi-architecture macOS binary still named `gen_snapshot`. See: flutter/engine#4948 This was later remediated over the course of several patches, including: * flutter/engine#10430 * flutter/engine#22818 * flutter#37445 However, there were still cases (notably `--local-engine` workflows in the tool) where we weren't computing the target platform and thus referenced the generic `gen_snapshot` tool. See: flutter#38933 Fixed in: flutter/engine#28345 The test removed in this PR, which ensured that null `SnapshotType.platform` was supported was introduced in flutter#11924 as a followup to flutter#11820 when the snapshotting logic was originally extracted to the `GenSnapshot` class, and most invocations still passed a null target platform. Since there are no longer any cases where `TargetPlatform` isn't passed when looking up `Artifact.genSnapshot`, we can safely make the platform non-nullable and remove the test. This is pre-factoring towards the removal of the generic `gen_snapshot` artifact from the macOS host binaries (which are currently unused since we never pass a null `TargetPlatform`), which is pre-factoring towards the goal of building `gen_snapshot` binaries with an arm64 host architecture, and eliminate the need to use Rosetta during iOS and macOS Flutter builds. Part of: flutter#101138 Umbrella issue: flutter#103386 Umbrella issue: flutter#69157 No new tests since the behaviour is enforced by the compiler.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Copies and renames the architecture-dependent iOS variants of
gen_snapshot to a destination directory. Now that the armv7-emitting
variant of gen_snapshot is 64-bit we can no longer bundle up
gen_snapshot as a fat binary with the x86 armv7 variant merged with the
x86_64 arm64 variant.