Feat/ios m1 improvements #32296

barbieri · 2021-09-29T17:05:22Z

Summary

Remove left over from a1c445a#commitcomment-57240925
Add a warning if running with Rosetta2 as per Fix: Xcode 12.5+ build of iPhone Simulator on Apple M1 #32284 (comment)

Changelog

[iOS] [Fixed] - Removed __apply_Xcode_12_5_M1_post_install_workaround
[iOS] [Changed] - Warn if Rosetta2 is being used (x86_64 on arm64)

Test Plan

Build on macOS Apple devices without any warnings during pod install

See facebook@a1c445a#commitcomment-57240925 This is producing a message that it will be deprecated, so we should just remove that.

Summary: Since Apple released its own silicon M1, an ARM64, the react-native build is broken or at least not as effective as it should. This PR stops excluding `arm64` simulator (this is not needed on the M1 neither on Intel devices) and removes the problematic `$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)` from `LIBRARY_SEARCH_PATHS`, since on Xcode 12.5 and 13.0 this folder contains only `i386/x86_64` binaries and will fail compilation. Instead this PR forces `$(SDKROOT)/usr/lib/swift` while it removes the incorrect directory. Ideally we could just remove `LIBRARY_SEARCH_PATHS` altogether if `$(inherited)` and `$(SDKROOT)/usr/lib/swift` were the only entries, but it would require us a **newer CocoaPods**, since that was fixed with `1.11` (see CocoaPods/CocoaPods@6985cbf). Since we don't enforce that, lets keep the `$(SDKROOT)/usr/lib/swift` and call it done. Last but not least, deprecate the `__apply_Xcode_12_5_M1_post_install_workaround()` as it's not needed anymore, at least with recent versions of the dependencies, no patching is required with RCT-Folly, neither we need to force `IPHONEOS_DEPLOYMENT_TARGET=11.0` ## Changelog [iOS] [Fixed] - Xcode 12.5+ build of iPhone Simulator on Apple M1 [iOS] [Changed] - Do not exclude the arm64 iphonesimulator [iOS] [Deprecated] - __apply_Xcode_12_5_M1_post_install_workaround() Pull Request resolved: #32284 Test Plan: * Build `packages/rn-tester` on M1 and see it still works properly * Run `pod install` on x86_64 and arm64 (m1) and see the `project.pbxproj` is not changed ## References: * Closes #31480 * The initial fix ac4ddec * Upgrading CocoaPods to 1.11 would bring us CocoaPods/CocoaPods@6985cbf and we could avoid adding `$(SDKROOT)/usr/lib/swift` ourselves Reviewed By: lunaleaps Differential Revision: D31248460 Pulled By: fkgozali fbshipit-source-id: 5a0d69593e889e296a2ba2e7b4387ecbd56fc08d

barbieri · 2021-09-29T17:06:48Z

@fkgozali take a look at this one please, related to a1c445a

fkgozali · 2021-09-29T17:11:51Z

scripts/react_native_pods.rb

@@ -16,6 +16,11 @@ def use_react_native! (options={})
  # Include Hermes dependencies
  hermes_enabled = options[:hermes_enabled] ||= false

+  # Running `pod install` from inside Rosetta2 is a bad sign
+  if `/usr/sbin/sysctl -n hw.optional.arm64 2>&1`.to_i == 1 && !RUBY_PLATFORM.start_with?('arm64')
+    Pod::UI.warn 'Do not use Rosetta2 with "pod install"'


what should people do instead? Maybe add a 2nd sentence pointing to a resource or list out the mitigation for them

Maybe also add comment that this may happen when using Apple Silicon machines?

Ok, will reword the message

If there's no existing resource/doc about this, you can explain them in comment (in a paragraph form). And/or pull this out to a helper function so you can isolate the documentation.

Anyway your call :)

fkgozali · 2021-09-29T17:17:10Z

I'll merge this by EOD US time (pending comment clarification mentioned above)

analysis-bot · 2021-09-29T17:35:20Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,714,421	+0
android	hermes	armeabi-v7a	7,243,144	+0
android	hermes	x86	8,134,133	+0
android	hermes	x86_64	8,099,499	+0
android	jsc	arm64-v8a	9,633,755	+0
android	jsc	armeabi-v7a	8,549,999	+0
android	jsc	x86	9,647,763	+0
android	jsc	x86_64	10,256,803	+0

Base commit: dd99475

Running `pod install` from inside Rosetta2 can lead to problems, try to avoid doing that by hinting users from use_react_native!() function. $ arch -x86_64 pod install Building RNTester with Fabric enabled. Building RNTester with Fabric enabled. Building RNTester with Fabric enabled. Analyzing dependencies Downloading dependencies Generating Pods project Integrating client project Pod installation complete! There are 63 dependencies from the Podfile and 50 total pods installed. [!] Do not use "pod install" from inside Rosetta2 (x86_64 emulation on arm64). [!] - Emulated x86_64 is slower than native arm64 [!] - May result in mixed architectures in rubygems (eg: ffi_c.bundle files may be x86_64 with an arm64 interpreter) [!] Run "env /usr/bin/arch -arm64 /bin/bash --login" then try again.

barbieri · 2021-09-29T17:48:10Z

@fkgozali improved the messaging.

Also, inside our company we noticed that even when running outside of Rosetta2, depending on the previous "mess", the ffi_c.bundle may be of an incorrect architecture. We could always show an info if running on arm64 such as:

  if `/usr/sbin/sysctl -n hw.optional.arm64 2>&1`.to_i == 1
    if !RUBY_PLATFORM.start_with?('arm64')
      Pod::UI.warn 'Do not use "pod install" from inside Rosetta2 (x86_64 emulation on arm64).'
      Pod::UI.warn ' - Emulated x86_64 is slower than native arm64'
      Pod::UI.warn ' - May result in mixed architectures in rubygems (eg: ffi_c.bundle files may be x86_64 with an arm64 interpreter)'
      Pod::UI.warn 'Run "env /usr/bin/arch -arm64 /bin/bash --login" then try again.'
    else
      Pod::UI.info 'If you get crashes related to ffi (eg: ffi_c.bundle), it may be related to mixed "bundle install" from different architectures (x86_64, arm64), please cleanup and try again. Its is NOT a bug with react-native'
    end
  end

However I did not add that as it would be disturbing to most people that did not have the mess... but if you do have some FAQ with:

If you face a problem with ffi and ffi_c.bundle, this is caused by mixing native (arm64) and rosetta2 (x86_64) environments when installing Ruby (even via rbenv or rvm) and its gems (bundler install or gem install cocoapods). Please uninstall everything related to Ruby and gems, then start again without rosetta2!

LoadError - dlopen(/path/to/gems/ffi-1.15.4/lib/ffi_c.bundle, 9): no suitable image found.  Did find:
	/path/to/gems/ffi-1.15.4/lib/ffi_c.bundle: mach-o, but wrong architecture
	/path/to/gems/ffi-1.15.4/lib/ffi_c.bundle: mach-o, but wrong architecture - /path/to/gems/ffi-1.15.4/lib/ffi_c.bundle
/path/to/gems/ffi-1.15.4/lib/ffi.rb:5:in `require'
/path/to/gems/ffi-1.15.4/lib/ffi.rb:5:in `rescue in <top (required)>'
/path/to/gems/ffi-1.15.4/lib/ffi.rb:2:in `<top (required)>'
/path/to/gems/ethon-0.14.0/lib/ethon.rb:3:in `require'
/path/to/gems/ethon-0.14.0/lib/ethon.rb:3:in `<top (required)>'
/path/to/gems/typhoeus-1.4.0/lib/typhoeus.rb:2:in `require'
/path/to/gems/typhoeus-1.4.0/lib/typhoeus.rb:2:in `<top (required)>'
/path/to/gems/cocoapods-core-1.11.2/lib/cocoapods-core/cdn_source.rb:440:in `require'

barbieri · 2021-09-29T18:06:09Z

build failures looks unrelated, but would be nice to confirm

fkgozali · 2021-09-29T23:35:00Z

Yeah test failure seems unrelated. Let's proceed.

facebook-github-bot · 2021-09-29T23:35:57Z

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

Remove leftover/workaround from template/Podfile

efa454b

See facebook@a1c445a#commitcomment-57240925 This is producing a message that it will be deprecated, so we should just remove that.

fkgozali reviewed Sep 29, 2021

View reviewed changes

barbieri force-pushed the feat/ios-m1-improvements branch from 8d49ab3 to 2175216 Compare September 29, 2021 17:37

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 Sep 29, 2021

facebook-github-bot closed this in 51bf557 Sep 30, 2021

barbieri deleted the feat/ios-m1-improvements branch September 30, 2021 19:06

mikehardy mentioned this pull request Nov 15, 2021

(:fire:) ARCHIVE FAILED The following build commands failed: PhaseScriptExecution [CP-User]\ [RNFB]\ Crashlytics\ Configuration invertase/react-native-firebase#5789

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/ios m1 improvements #32296

Feat/ios m1 improvements #32296

barbieri commented Sep 29, 2021

barbieri commented Sep 29, 2021

fkgozali Sep 29, 2021 •

edited

Loading

barbieri Sep 29, 2021

fkgozali Sep 29, 2021

fkgozali commented Sep 29, 2021

analysis-bot commented Sep 29, 2021

barbieri commented Sep 29, 2021

barbieri commented Sep 29, 2021

fkgozali commented Sep 29, 2021

facebook-github-bot commented Sep 29, 2021

Feat/ios m1 improvements #32296

Feat/ios m1 improvements #32296

Conversation

barbieri commented Sep 29, 2021

Summary

Changelog

Test Plan

barbieri commented Sep 29, 2021

fkgozali Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

barbieri Sep 29, 2021

Choose a reason for hiding this comment

fkgozali Sep 29, 2021

Choose a reason for hiding this comment

fkgozali commented Sep 29, 2021

analysis-bot commented Sep 29, 2021

barbieri commented Sep 29, 2021

barbieri commented Sep 29, 2021

fkgozali commented Sep 29, 2021

facebook-github-bot commented Sep 29, 2021

fkgozali Sep 29, 2021 •

edited

Loading