arm64 simulators & catalyst support #475

Arkkeeper · 2021-03-20T19:29:52Z

Summary

Edits build scripts to allow merging architectures into .xcframework instead of using lipo for building fat framework.
Adds compatibility with arm64 iOS Simulators on Apple M1 Macs and allows using Hermes with RN apps built for Mac Catalyst.

Fixes #460 and #468

facebook-github-bot · 2021-03-20T19:29:57Z

Hi @Arkkeeper!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot · 2021-03-20T19:49:07Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Huxpro · 2021-03-22T18:44:36Z

@Arkkeeper Thanks for contributing! While cc-ing @grabbou to review this, I noticed that test-apple-runtime in CircleCI is failing. Could you also take care of that?

The test-apple-runtime is setup here: https://github.com/facebook/hermes/blob/master/.circleci/config.yml#L202

Huxpro · 2021-03-22T18:45:45Z

hermes-engine.podspec

@@ -20,13 +20,13 @@ Pod::Spec.new do |spec|
  # The podspec would be serialized to JSON and people will download prebuilt binaries instead of the source.
  # TODO(use the hash field as a validation mechanism when the process is stable)
  spec.source      = ENV['hermes-artifact-url'] ? { http: ENV['hermes-artifact-url'] } : { git: "https://github.com/facebook/hermes.git", tag: "v#{spec.version}" }
-  spec.platforms   = { :osx => "10.13", :ios => "10.0" }
+  spec.platforms   = { :osx => "11.0", :ios => "10.0" }


Maybe because of this version bump?

I reverted that bump, I don't think that it's essential for Catalyst support.
Now CircleCI fails on Android tests, but my commits have nothing to do with the android part of code.

Yeah do not worry about the Android tests. The android emulator image is somehow missing.

andreialecu · 2021-04-08T11:30:48Z

Friendly ping @grabbou

For context why this is extremely important:

RN 0.64 now ships with iOS Hermes support, but lack of support for iOS Simulator arm64 arch makes it impossible to work with it.

Additionally, react-native-reanimated v2 has been released, which uses JSI/turbo modules.

RN apps using turbo modules can only be debugged with Flipper's Hermes Debugger.

andreialecu · 2021-04-08T11:32:25Z

@Arkkeeper is there any easy way to use this PR before it's merged?

Arkkeeper · 2021-04-10T16:44:48Z

You have to patch react-native/scripts/react_native_pods.rb.

First, add a variable to check whether it's ARM Mac, like:
is_arm = '/usr/sbin/sysctl -n hw.optional.arm64 2>&1'.to_i

Then modify definition of hermes-engine and refer to repo with my PR:

if is_arm == 1 then
pod 'hermes-engine', :git => 'https://github.com/Arkkeeper/hermes.git'
else
pod 'hermes-engine', '~> 0.7.2'
end

Make sure that you have cmake and ninja installed with homebrew.

Then run yarn and prepare to wait about 15-20 minutes. Building hermes-engine from sources for several archs is rather tedious.

andreialecu · 2021-04-10T17:30:51Z

Thank you. Some notes regarding your instructions, in case it helps others:

Ran into fatal: repository 'Arkkeeper/hermes.git' does not exist so I had to add it as:
pod 'hermes-engine', :git => 'git@github.com:Arkkeeper/hermes.git'.

Also, detecting arm64 didn't work in my case because I can only run cocoapods using arch -x86_64, due to other issues with it. So I just had to hardcode it without the check.

alloy

Awesome, I didn’t know somebody had already done this work! Most importantly we should make sure it still works for macOS too.

alloy · 2021-04-27T10:02:53Z

utils/build-apple-framework.sh

+  local catalyst="false"
+  local platform=$1
+
+  if [[ $1 == iphoneos || $1 == catalyst ]]; then


Perhaps also replace further use of $1 with the $platform alias here?

I would actually not use $platform here. Further explanation on line 62.

alloy · 2021-04-27T10:03:02Z

utils/build-apple-framework.sh

@@ -56,11 +59,16 @@ function configure_apple_framework {
  else
    build_cli_tools="false"
  fi
+  if [[ $1 == catalyst ]]; then


alloy · 2021-04-27T10:04:51Z

utils/build-apple-framework.sh

-  for platform in "${@:2}"; do
-    rm -r "$platform"
-  done
+  xcodebuild -create-xcframework -framework iphoneos/hermes.framework -framework iphonesimulator/hermes.framework -framework macosx/hermes.framework -output iphoneos/hermes.xcframework


The output should not assume the framework is just for iOS here, it should theoretically also be able to contain the macOS slices, no?

Yes. Also, it should also get the platforms from the platforms, like the lipo did, without hardcoding them. I just think it's nicer and doesn't require manual rm of every folder, like it's currently done on line 112-114.

utils/build-apple-framework.sh

-
-  for i in "${!platforms[@]}"; do
-    platforms[$i]="${platforms[$i]}/hermes.framework/hermes"
-  done


grabbou · 2021-04-27T12:24:23Z

Thanks for the PR! I left some comments to further optimise the scripts and the way we handle Catalyst, but I'd consider them minor tweaks, since overall, everything seems to be fine.

Arkkeeper · 2021-05-02T09:42:26Z

Thank you for your suggestions! I've pushed some optimizations for the build scripts. I hope that my PR won't delay Hermes 0.8.1 and RN 0.65.

garrylachman · 2021-05-23T09:29:19Z

Building hermes-engine from sources for several archs is rather tedious.

Building third time in row, over 1 hour each time. not cool :(

umang-simform · 2021-06-02T04:56:59Z

@Arkkeeper Any timeline on how soon it can get merged?

kelset · 2021-06-17T17:25:33Z

cc @grabbou remember to check this one ;)

grabbou

Thank you for addressing my previous comments. The PR looks really good! Rather than two tiny issues, it is good to go.

I will give it a shot and test it tomorrow, and if everything runs smoothly, I will merge it, reverting the deleted lines on top of your PR.

grabbou · 2021-06-24T16:42:57Z

utils/build-apple-framework.sh

-  # Once all was linked into a single framework, clean destroot
-  # from unused frameworks
-  for platform in "${@:2}"; do
-    rm -r "$platform"


Any particular reason why this has been removed? This should be reverted. We need to clean up destroot to only contain the artifacts that are used in production.

This is because CircleCI will package the entire destroot folder into a tarball that is distributed along the npm package. If we don't clean it up, we are going to ship one xcframework and every platform separately as well.

OK, I've reverted that removal

grabbou · 2021-06-24T16:43:14Z

utils/build-apple-framework.sh

  done

-  lipo -info "${platforms[0]}"


Any particular reason why this has been removed? I think this is a good add-on for debugging purposes on the CI.

Because lipo -info fails on xcframework with a fatal error:
can't map input file: iphoneos (Invalid argument)

grabbou · 2021-06-29T04:24:47Z

Code wise, it looks good. However, I just noticed that test-apple-runtime is currently failing.

Testing failed:
Undefined symbol: facebook::hermes::makeHermesRuntime(hermes::vm::RuntimeConfig const&)
Undefined symbol: facebook::hermes::HermesRuntime::debugJavaScript(std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator > const&, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator > const&, facebook::hermes::HermesRuntime::DebugFlags const&)
Testing cancelled because the build failed.

facebook-github-bot · 2021-06-29T18:45:51Z

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

Summary: Edits build scripts to allow merging architectures into .xcframework instead of using lipo for building fat framework. Adds compatibility with arm64 iOS Simulators on Apple M1 Macs and allows using Hermes with RN apps built for Mac Catalyst. Fixes facebook#460 and facebook#468 Pull Request resolved: facebook#475 Differential Revision: D29465045 Pulled By: Huxpro fbshipit-source-id: 6291aac9ee47db83d6f78822767019ec0277a899

TheSavior · 2021-07-08T17:15:58Z

I believe we should be closing this one in favor of #546. Thanks for the work so far!

Summary: This PR is #475 with adjustments to folder names. It also fixes caches and CI issues. Pull Request resolved: #546 Test Plan: CircleCI Reviewed By: mhorowitz Differential Revision: D29597379 Pulled By: Huxpro fbshipit-source-id: f278f9c5503965cb998872993a1ce38cfcdaadcc

arm64 simulators & catalyst support

88e6566

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 20, 2021

Huxpro reviewed Mar 22, 2021

View reviewed changes

.podspec macOS version downgrade

b63d240

alloy suggested changes Apr 27, 2021

View reviewed changes

grabbou reviewed Apr 27, 2021

View reviewed changes

grabbou mentioned this pull request Apr 30, 2021

Road to v0.65 Discussion react-native-community/releases#223

Closed

38 tasks

create_universal_framework improved

cee5fcf

This was referenced May 25, 2021

Hermes won't build on M1 Mac microsoft/react-native-macos#781

Closed

Hermes does not build for React Native macOS on ARM64 #514

Closed

kelset mentioned this pull request Jun 9, 2021

Road to v0.65.0 - RC2 phase react-native-community/releases#235

Closed

9 tasks

Huxpro mentioned this pull request Jun 9, 2021

What should be included in v0.8.1 #528

Closed

10 tasks

grabbou suggested changes Jun 24, 2021

View reviewed changes

build-apple-framework.sh reversion

bdd75a4

Huxpro mentioned this pull request Jun 30, 2021

arm64 simulators & catalyst support (#475) #543

Closed

Titozzz mentioned this pull request Jul 5, 2021

arm64 simulators & catalyst support (was #543 and #475) #546

Closed

TheSavior closed this Jul 8, 2021

This was referenced Jul 12, 2021

Support for Mac Catalyst #460

Closed

hermes.framework is incompatible with iOS Simulator on Apple M1 Macs #468

Closed

arm64 simulators & catalyst support #475

arm64 simulators & catalyst support #475

Conversation

Arkkeeper commented Mar 20, 2021

Summary

facebook-github-bot commented Mar 20, 2021

Action Required

Process

facebook-github-bot commented Mar 20, 2021

Huxpro commented Mar 22, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreialecu commented Apr 8, 2021

andreialecu commented Apr 8, 2021 • edited

Arkkeeper commented Apr 10, 2021 • edited

andreialecu commented Apr 10, 2021

alloy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

grabbou commented Apr 27, 2021

Arkkeeper commented May 2, 2021

garrylachman commented May 23, 2021

umang-simform commented Jun 2, 2021

kelset commented Jun 17, 2021

grabbou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grabbou commented Jun 29, 2021

facebook-github-bot commented Jun 29, 2021

TheSavior commented Jul 8, 2021

Huxpro commented Mar 22, 2021 •

edited

andreialecu commented Apr 8, 2021 •

edited

Arkkeeper commented Apr 10, 2021 •

edited