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

Fix Rosetta2 CocoaPods warning on Apple Silicon #32498

Closed
wants to merge 1 commit into from

Conversation

oblador
Copy link
Contributor

@oblador oblador commented Oct 28, 2021

Summary

The current warning assumes the ruby binary to be single arch, but the ruby version shipping with macOS is universal universal.arm64e-darwin20. This PR changes the check to search for arm64 in any position instead of just the beginning to fix false positives.

Changelog

[iOS] [Fixed] - Fix Rosetta2 CocoaPods warning on Apple Silicon

Test Plan

Before

On M1 Mac pod install using system ruby always yields a warning.

After

pod install does not yield a warning.
arch -x86_64 pod install yields a warning.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 28, 2021
@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Oct 28, 2021
@pull-bot
Copy link

PR build artifact for bbe32c7 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,290,013 +0
android hermes armeabi-v7a 7,854,061 +0
android hermes x86 8,657,039 +0
android hermes x86_64 8,616,137 +0
android jsc arm64-v8a 9,791,365 +0
android jsc armeabi-v7a 8,752,225 +0
android jsc x86 9,740,238 +0
android jsc x86_64 10,341,068 +0

Base commit: 61e1b6f
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 61e1b6f
Branch: main

@facebook-github-bot
Copy link
Contributor

@sota000 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sota000
Copy link
Contributor

sota000 commented Oct 29, 2021

@barbieri and @mikehardy, can you please take a look at this? @fkgozali suggested you have more context on this fix.

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Oh, interesting! Good catch+fix @oblador, thanks for the ping @sota000 - and while I will immediately defer to @barbieri if there were questions, this seems like it maintains the spirit of the original PR perfectly ("make sure ruby runs native not by translation") since a universal binary will run native if it can and there is a check for translation as the first clause.

@barbieri
Copy link
Contributor

makes total sense 👍

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 4, 2021
@facebook-github-bot
Copy link
Contributor

@sota000 merged this pull request in e918362.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants