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

Disable hermes in RNTester Podfile #780

Merged
merged 11 commits into from
May 25, 2021

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented May 21, 2021

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

RNTester-macOS did not build on an m1 Mac for arm64. The build errors implied there was a problem with Hermes. Disabling Hermes leads to a successful build.

Changelog

[macOS] [Fixed] - Fixed building RNTester on ARM64 by disabling hermes

Test Plan

Built and ran RNTester-macOS on my m1 Mac mini with arm64 and not rosetta.

@Saadnajmi Saadnajmi requested a review from alloy as a code owner May 21, 2021 20:27
@Saadnajmi
Copy link
Collaborator Author

The type of errors seen without disabling:
e6abbbf1-2a8a-49b6-8819-a8e983aa8bbd

@harrieshin
Copy link

can we make sure we have a ticket to track Hermes would need to support m1 machines properly?

@Saadnajmi
Copy link
Collaborator Author

can we make sure we have a ticket to track Hermes would need to support m1 machines properly?

Would that be an issue in this repo, or the hermes repo? Not sure if hermes explicitly supports RN-macOS or we just kinda got it for free. @HeyImChris in case he knows best place to track

@Saadnajmi Saadnajmi merged commit b771b8c into microsoft:master May 25, 2021
@Saadnajmi
Copy link
Collaborator Author

Filed #781 to track publicly

@Saadnajmi Saadnajmi deleted the disable-hermes branch May 25, 2021 02:40
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Jul 29, 2022
Summary:
**Context**

On Core RN, Hermes for iOS can be enabled by setting a flag in the Podfile
https://reactnative.dev/docs/hermes#ios

| Since React Native 0.64, Hermes also runs on iOS. To enable Hermes for iOS, edit your ios/Podfile file and make the change illustrated below:
```
   use_react_native!(
     :path => config[:reactNativePath],
     # to enable hermes on iOS, change `false` to `true` and then install pods
     # By default, Hermes is disabled on Old Architecture, and enabled on New Architecture.
     # You can enable/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`.
     :hermes_enabled => true
   )
```
In the RNTester Podfile, Hermes is enabled using envvar:
https://github.com/facebook/react-native/blob/main/packages/rn-tester/Podfile#L27
```
  # Hermes is now enabled by default.
  # The following line will only disable Hermes if the USE_HERMES envvar is SET to a value other than 1 (e.g. USE_HERMES=0).
  hermes_enabled = !ENV.has_key?('USE_HERMES') || ENV['USE_HERMES'] == '1'
```
Build command: `USE_HERMES=1 bundle exec pod install`

This will install the Hermes runtime Pod (not build it from scratch) & thus enable the `RCT_USE_HERMES` macro.

https://www.internalfb.com/code/fbsource/[9f57823a75a40d3f8559c8f1b7ae0add8e95d6dc]/xplat/js/react-native-github/packages/rn-tester/RNTester/AppDelegate.mm?lines=10-16

---

The documentation for enabling Hermes on RN Desktop macOS are outdated:
https://microsoft.github.io/react-native-windows/docs/hermes#hermes-on-macos

> Install the npm package yarn add 'hermes-engine-darwin@^0.4.3'

* `hermes-engine-darwin` is no longer required

> Add (or uncomment) the following pod dependencies to your macOS target in your Podfile:
pod 'React-Core/Hermes', :path => '../node_modules/react-native-macos/'
pod 'hermes', :path => '../node_modules/hermes-engine-darwin'
pod 'libevent', :podspec => '../node_modules/react-native-macos/third-party-podspecs/libevent.podspec'

* Setting `USE_HERMES=1` during `pod install= replaces all of this

> Copy
Run pod install
Be sure to set your target's deployment target to at least 10.14 before running pod install

* `USE_HERMES=1 bundle exec pod install --verbose`

---

On RN Desktop, the Hermes flag was [set to false](microsoft#780) due to M1 build reasons which have since been resolved.
- microsoft#952
- microsoft#781

Curiously, the `RNTester-macOS` target AppDelegate was never updated to import & use Hermes when  `RCT_USE_HERMES` was `true`. Only the `RNTester` for mobile had the correct Hermes usage.

**RNTester-macOS:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester-macOS/AppDelegate.mm

**RNTester:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm

**Change**

* Remove `pods(:hermes_enabled => true)` in favor of passing `USE_HERMES=1` to `pod install` (This is how it's done on RNTester iOS)
* Copy Hermes support to `RNTester-macOS` AppDelegate

Test Plan: **Install from scratch**

Differential Revision: https://phabricator.intern.facebook.com/D38277077
Saadnajmi pushed a commit that referenced this pull request Aug 2, 2022
Summary:
**Context**

On Core RN, Hermes for iOS can be enabled by setting a flag in the Podfile
https://reactnative.dev/docs/hermes#ios

| Since React Native 0.64, Hermes also runs on iOS. To enable Hermes for iOS, edit your ios/Podfile file and make the change illustrated below:
```
   use_react_native!(
     :path => config[:reactNativePath],
     # to enable hermes on iOS, change `false` to `true` and then install pods
     # By default, Hermes is disabled on Old Architecture, and enabled on New Architecture.
     # You can enable/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`.
     :hermes_enabled => true
   )
```
In the RNTester Podfile, Hermes is enabled using envvar:
https://github.com/facebook/react-native/blob/main/packages/rn-tester/Podfile#L27
```
  # Hermes is now enabled by default.
  # The following line will only disable Hermes if the USE_HERMES envvar is SET to a value other than 1 (e.g. USE_HERMES=0).
  hermes_enabled = !ENV.has_key?('USE_HERMES') || ENV['USE_HERMES'] == '1'
```
Build command: `USE_HERMES=1 bundle exec pod install`

This will install the Hermes runtime Pod (not build it from scratch) & thus enable the `RCT_USE_HERMES` macro.

https://www.internalfb.com/code/fbsource/[9f57823a75a40d3f8559c8f1b7ae0add8e95d6dc]/xplat/js/react-native-github/packages/rn-tester/RNTester/AppDelegate.mm?lines=10-16

---

The documentation for enabling Hermes on RN Desktop macOS are outdated:
https://microsoft.github.io/react-native-windows/docs/hermes#hermes-on-macos

> Install the npm package yarn add 'hermes-engine-darwin@^0.4.3'

* `hermes-engine-darwin` is no longer required

> Add (or uncomment) the following pod dependencies to your macOS target in your Podfile:
pod 'React-Core/Hermes', :path => '../node_modules/react-native-macos/'
pod 'hermes', :path => '../node_modules/hermes-engine-darwin'
pod 'libevent', :podspec => '../node_modules/react-native-macos/third-party-podspecs/libevent.podspec'

* Setting `USE_HERMES=1` during `pod install= replaces all of this

> Copy
Run pod install
Be sure to set your target's deployment target to at least 10.14 before running pod install

* `USE_HERMES=1 bundle exec pod install --verbose`

---

On RN Desktop, the Hermes flag was [set to false](#780) due to M1 build reasons which have since been resolved.
- #952
- #781

Curiously, the `RNTester-macOS` target AppDelegate was never updated to import & use Hermes when  `RCT_USE_HERMES` was `true`. Only the `RNTester` for mobile had the correct Hermes usage.

**RNTester-macOS:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester-macOS/AppDelegate.mm

**RNTester:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm

**Change**

* Remove `pods(:hermes_enabled => true)` in favor of passing `USE_HERMES=1` to `pod install` (This is how it's done on RNTester iOS)
* Copy Hermes support to `RNTester-macOS` AppDelegate

Test Plan: **Install from scratch**

Differential Revision: https://phabricator.intern.facebook.com/D38277077

Co-authored-by: Shawn Dempsey <shawndempsey@fb.com>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
Summary:
**Context**

On Core RN, Hermes for iOS can be enabled by setting a flag in the Podfile
https://reactnative.dev/docs/hermes#ios

| Since React Native 0.64, Hermes also runs on iOS. To enable Hermes for iOS, edit your ios/Podfile file and make the change illustrated below:
```
   use_react_native!(
     :path => config[:reactNativePath],
     # to enable hermes on iOS, change `false` to `true` and then install pods
     # By default, Hermes is disabled on Old Architecture, and enabled on New Architecture.
     # You can enable/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`.
     :hermes_enabled => true
   )
```
In the RNTester Podfile, Hermes is enabled using envvar:
https://github.com/facebook/react-native/blob/main/packages/rn-tester/Podfile#L27
```
  # Hermes is now enabled by default.
  # The following line will only disable Hermes if the USE_HERMES envvar is SET to a value other than 1 (e.g. USE_HERMES=0).
  hermes_enabled = !ENV.has_key?('USE_HERMES') || ENV['USE_HERMES'] == '1'
```
Build command: `USE_HERMES=1 bundle exec pod install`

This will install the Hermes runtime Pod (not build it from scratch) & thus enable the `RCT_USE_HERMES` macro.

https://www.internalfb.com/code/fbsource/[9f57823a75a40d3f8559c8f1b7ae0add8e95d6dc]/xplat/js/react-native-github/packages/rn-tester/RNTester/AppDelegate.mm?lines=10-16

---

On RN Desktop, the Hermes flag was [set to false](microsoft#780) due to M1 build reasons which have since been resolved.
- microsoft#952
- microsoft#781

Curiously, the `RNTester-macOS` target AppDelegate was never updated to import & use Hermes when  `RCT_USE_HERMES` was `true`. Only the `RNTester` for mobile had the correct Hermes usage.

**RNTester-macOS:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester-macOS/AppDelegate.mm

**RNTester:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm

--

The documentation for enabling Hermes on RN Desktop macOS are outdated:
https://microsoft.github.io/react-native-windows/docs/hermes#hermes-on-macos

> Install the npm package yarn add 'hermes-engine-darwin@^0.4.3'

* `hermes-engine-darwin` npm package is no longer required

> Add (or uncomment) the following pod dependencies to your macOS target in your Podfile:
pod 'React-Core/Hermes', :path => '../node_modules/react-native-macos/'
pod 'hermes', :path => '../node_modules/hermes-engine-darwin'
pod 'libevent', :podspec => '../node_modules/react-native-macos/third-party-podspecs/libevent.podspec'

* Setting `USE_HERMES=1` during `pod install` replaces these manual steps

> Copy
Run pod install
Be sure to set your target's deployment target to at least 10.14 before running pod install

* `USE_HERMES=1 bundle exec pod install --verbose`

---

**Change**

* Remove `pods(:hermes_enabled => true)` in favor of passing `USE_HERMES=1` to `pod install` (This is how it's done on RNTester iOS)
* Copy Hermes support to `RNTester-macOS` AppDelegate

Test Plan:
## Hermes Enabled

```
cd packages/rn-tester

# Ensure fresh pod install & `RCT_USE_HERMES` works
rm -rf Pods 

# Install pods
USE_HERMES=1 bundle exec pod install --verbose

# Note the installing of hermes-engine

-> Installing hermes-engine (0.11.0)
  > Copying hermes-engine from `/Users/shawndempsey/Library/Caches/CocoaPods/Pods/Release/hermes-engine/0.11.0-84e3a` to `Pods/hermes-engine`

# Open in xcode
open RNTesterPods.xcworkspace
```

`React-hermes` should be installed
{F756792783}

|Connect to Hermes w/ Flipper|
|https://pxl.cl/28RTj|

---


## Hermes disabled

```
bundle exec pod install --verbose
```

`React-hermes` is **not** installed
{F756795691}

|Herms is not active|
|https://pxl.cl/28RWK|

Reviewers: lyahdav, ericroz, chpurrer, #seller_expansion

Reviewed By: lyahdav

Differential Revision: https://phabricator.intern.facebook.com/D38277077

Tasks: T127659495

Tags: marketplace, marketplace_seller_expansion
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
Summary:
**Context**

On Core RN, Hermes for iOS can be enabled by setting a flag in the Podfile
https://reactnative.dev/docs/hermes#ios

| Since React Native 0.64, Hermes also runs on iOS. To enable Hermes for iOS, edit your ios/Podfile file and make the change illustrated below:
```
   use_react_native!(
     :path => config[:reactNativePath],
     # to enable hermes on iOS, change `false` to `true` and then install pods
     # By default, Hermes is disabled on Old Architecture, and enabled on New Architecture.
     # You can enable/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`.
     :hermes_enabled => true
   )
```
In the RNTester Podfile, Hermes is enabled using envvar:
https://github.com/facebook/react-native/blob/main/packages/rn-tester/Podfile#L27
```
  # Hermes is now enabled by default.
  # The following line will only disable Hermes if the USE_HERMES envvar is SET to a value other than 1 (e.g. USE_HERMES=0).
  hermes_enabled = !ENV.has_key?('USE_HERMES') || ENV['USE_HERMES'] == '1'
```
Build command: `USE_HERMES=1 bundle exec pod install`

This will install the Hermes runtime Pod (not build it from scratch) & thus enable the `RCT_USE_HERMES` macro.

https://www.internalfb.com/code/fbsource/[9f57823a75a40d3f8559c8f1b7ae0add8e95d6dc]/xplat/js/react-native-github/packages/rn-tester/RNTester/AppDelegate.mm?lines=10-16

---

On RN Desktop, the Hermes flag was [set to false](microsoft#780) due to M1 build reasons which have since been resolved.
- microsoft#952
- microsoft#781

Curiously, the `RNTester-macOS` target AppDelegate was never updated to import & use Hermes when  `RCT_USE_HERMES` was `true`. Only the `RNTester` for mobile had the correct Hermes usage.

**RNTester-macOS:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester-macOS/AppDelegate.mm

**RNTester:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm

--

The documentation for enabling Hermes on RN Desktop macOS are outdated:
https://microsoft.github.io/react-native-windows/docs/hermes#hermes-on-macos

> Install the npm package yarn add 'hermes-engine-darwin@^0.4.3'

* `hermes-engine-darwin` npm package is no longer required

> Add (or uncomment) the following pod dependencies to your macOS target in your Podfile:
pod 'React-Core/Hermes', :path => '../node_modules/react-native-macos/'
pod 'hermes', :path => '../node_modules/hermes-engine-darwin'
pod 'libevent', :podspec => '../node_modules/react-native-macos/third-party-podspecs/libevent.podspec'

* Setting `USE_HERMES=1` during `pod install` replaces these manual steps

> Copy
Run pod install
Be sure to set your target's deployment target to at least 10.14 before running pod install

* `USE_HERMES=1 bundle exec pod install --verbose`

---

**Change**

* Remove `pods(:hermes_enabled => true)` in favor of passing `USE_HERMES=1` to `pod install` (This is how it's done on RNTester iOS)
* Copy Hermes support to `RNTester-macOS` AppDelegate

Test Plan:
## Hermes Enabled

```
cd packages/rn-tester

# Ensure fresh pod install & `RCT_USE_HERMES` works
rm -rf Pods 

# Install pods
USE_HERMES=1 bundle exec pod install --verbose

# Note the installing of hermes-engine

-> Installing hermes-engine (0.11.0)
  > Copying hermes-engine from `/Users/shawndempsey/Library/Caches/CocoaPods/Pods/Release/hermes-engine/0.11.0-84e3a` to `Pods/hermes-engine`

# Open in xcode
open RNTesterPods.xcworkspace
```

`React-hermes` should be installed
{F756792783}

|Connect to Hermes w/ Flipper|
|https://pxl.cl/28RTj|

---


## Hermes disabled

```
bundle exec pod install --verbose
```

`React-hermes` is **not** installed
{F756795691}

|Herms is not active|
|https://pxl.cl/28RWK|

Reviewers: lyahdav, ericroz, chpurrer, #seller_expansion

Reviewed By: lyahdav

Differential Revision: https://phabricator.intern.facebook.com/D38277077

Tasks: T127659495

Tags: marketplace, marketplace_seller_expansion
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.

2 participants