Skip to content
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] Only bundle gen_snapshot_${target_arch} #53971

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cbracken
Copy link
Member

On most platforms, we support only a single host architecture and a single target architecture per build. macOS supports both arm64 and x86_64 host platforms via multi-arch (universal) binaries generated with the lipo tool and as such, we have two gen_snapshot binaries, one for each target architecture. These binaries are universal binaries named gen_snapshot_${target_arch}. The unqualified gen_snapshot binary used for other platforms is unused on macOS.

This eliminates the unqualified binary from the uploaded bundles used by the tool.

Cleanup related to:
Issue: flutter/flutter#101138
Issue: flutter/flutter#69157

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C , Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

On most platforms, we support only a single host architecture and a
single target architecture per build. macOS supports both arm64 and
x86_64 host platforms via multi-arch (universal) binaries generated with
the `lipo` tool and as such, we have two gen_snapshot binaries, one for
each target architecture. These binaries are universal binaries named
`gen_snapshot_${target_arch}`. The unqualified `gen_snapshot` binary
used for other platforms is unused on macOS.

This eliminates the unqualified binary from the uploaded bundles used by
the tool.
On most platforms, we support only a single host architecture and a
single target architecture per build. macOS supports both arm64 and
x86_64 host platforms via multi-arch (universal) binaries generated with
the `lipo` tool and as such, we have two gen_snapshot binaries, one for
each target architecture. These binaries are universal binaries named
`gen_snapshot_${target_arch}`. The unqualified `gen_snapshot` binary
used for other platforms is unused on macOS.

This eliminates the unqualified binary from the uploaded bundles used by
the tool.
@cbracken cbracken force-pushed the mac-remove-unqualified-gen_snapshot branch from b9b216f to 95bb3f4 Compare July 17, 2024 21:49
Copy link
Member

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20 days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

@cbracken cbracken marked this pull request as draft August 6, 2024 22:04
@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

@Piinks
Copy link
Contributor

Piinks commented Oct 22, 2024

(PR triage): @cbracken is this still on your radar?

@cbracken
Copy link
Member Author

I'll file a proper issue for this, self-assign and then close this once I've collected up the error messages. Thanks for the reminder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants