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

[iOS]Fix glog & Flipper-Glog build failed on Apple Silicon #32380

Closed
wants to merge 5 commits into from
Closed

[iOS]Fix glog & Flipper-Glog build failed on Apple Silicon #32380

wants to merge 5 commits into from

Conversation

tylinux
Copy link

@tylinux tylinux commented Oct 11, 2021

Summary

Because glog's config.stub && config.guess is too old, so execute pod install on Apple Silicon devices will failed, error message is Invalid configuration 'arm64-apple-darwin20.2.0': machine 'arm64-apple' not recognized.

So we need to update these 2 files from GNU to fix this problem.

Patch config.sub to recognize arm64-apple

Changelog

[iOS] [Fixed] - Fix glog & Flipper-Glog build failed on Apple Silicon

Test Plan

  1. Clone this repo on Apple Silicon device.
  2. Disable flipper in packages/rn-tester/Podfile(becase Flipper-Glog has this problem too).
  3. pod install, build and run tester app.

@tylinux tylinux changed the title Fix glog build failed on Apple Silicon Fix glog & Flipper-Glog build failed on Apple Silicon Oct 11, 2021
@analysis-bot
Copy link

analysis-bot commented Oct 11, 2021

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,717,940 -572,073
android hermes armeabi-v7a 7,246,007 -608,054
android hermes x86 8,137,533 -519,506
android hermes x86_64 8,103,013 -513,124
android jsc arm64-v8a 9,637,130 -154,235
android jsc armeabi-v7a 8,552,720 -199,505
android jsc x86 9,650,995 -89,243
android jsc x86_64 10,260,163 -80,905

Base commit: 61e1b6f

@react-native-bot react-native-bot added the No CLA Authors need to sign the CLA before a PR can be reviewed. label Oct 12, 2021
@tylinux tylinux changed the title Fix glog & Flipper-Glog build failed on Apple Silicon [iOS]Fix glog & Flipper-Glog build failed on Apple Silicon Oct 12, 2021
@analysis-bot
Copy link

analysis-bot commented Oct 12, 2021

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

Base commit: 61e1b6f

@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 13, 2021
@react-native-bot react-native-bot removed the No CLA Authors need to sign the CLA before a PR can be reviewed. label Oct 13, 2021
@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.

@@ -9,6 +9,28 @@ set -e
PLATFORM_NAME="${PLATFORM_NAME:-iphoneos}"
CURRENT_ARCH="${CURRENT_ARCH}"

# Fix build on Apple Silicon
if [ "$CURRENT_ARCH" == "arm64" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if [ "$CURRENT_ARCH" == "arm64" ]; then
if [ "${CURRENT_ARCH:-$(arch)}" == "arm64" ]; then

in my situation, I was finding that $CURRENT_ARCH was not set. not sure why, but this solves it and should potentially be hoisted to line 10 (but I'm not sure if that impacts later code)

Copy link
Contributor

Choose a reason for hiding this comment

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

alternatively, you can move this entire code beneath the if statement below (which handles the missing CURRENT_ARCH case)

facebook-github-bot pushed a commit that referenced this pull request Nov 3, 2021
Summary:
This is a fixed version of the #32380 PR. It solves a typo, prevents variable substitution in the patch file, and moves it to a better place in the script so that CURRENT_ARCH is actually detected before checking whether to patch.

The `config.sub` included in glog is too old and does not recognize `arm64-*` as a valid arch when building. This, combined with an out of date Flipper-Glog version, results in persistent build failures on Apple Silicon machines.

p.s. i assume all the podfile lock changes were caused by me running this on an Apple Silicon Mac, and thus all the pod checksums were run against the arm64 versions of those pods rather than the normal x86_64 versions. if this is an issue I can revert the changes to that file, but it would seem to be an inevitable issue in future PR diffs...

## Changelog

- [iOS] [Fixed] - Apple Silicon builds of glog & Flipper-Glog

Pull Request resolved: #32486

Test Plan:
- Clone this branch on both an Apple Silicon- & Intel-based Mac
- Run `pod install` in `packages/rn-tester`
- Confirm that build passes successfully

Reviewed By: cortinico

Differential Revision: D32080994

Pulled By: yungsters

fbshipit-source-id: 76a7c5bba20d91905455920609c890e92bb5b980
@sota000
Copy link
Contributor

sota000 commented Nov 4, 2021

This seems to be a dup work with #32486. Though it sounds like we have blocker to merge the diff. I'll close this one for now, but please re-open if this is still relevant.

@sota000 sota000 closed this Nov 4, 2021
facebook-github-bot pushed a commit that referenced this pull request Nov 9, 2021
Summary:
NOTE: Second attempt at merging #32486 (D32080994 (25d4cb9)).

This is a fixed version of the #32380 PR. It solves a typo, prevents variable substitution in the patch file, and moves it to a better place in the script so that CURRENT_ARCH is actually detected before checking whether to patch.

The `config.sub` included in glog is too old and does not recognize `arm64-*` as a valid arch when building. This, combined with an out of date Flipper-Glog version, results in persistent build failures on Apple Silicon machines.

p.s. i assume all the podfile lock changes were caused by me running this on an Apple Silicon Mac, and thus all the pod checksums were run against the arm64 versions of those pods rather than the normal x86_64 versions. if this is an issue I can revert the changes to that file, but it would seem to be an inevitable issue in future PR diffs...

## Changelog

- [iOS] [Fixed] - Apple Silicon builds of glog & Flipper-Glog

Pull Request resolved: #32486

Test Plan: See `react-native-oss-ios` Sandcastle job succeed.

Reviewed By: fkgozali

Differential Revision: D32256761

Pulled By: yungsters

fbshipit-source-id: c7f32b72287018f070910b26aad02aa0adf4a61f
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. 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.

6 participants