Skip to content

Commit

Permalink
Use RN Build Utils for Building Hermes Artifacts
Browse files Browse the repository at this point in the history
Summary:
We moved Hermes some build utils from [Hermes repo](https://github.com/facebook/hermes/tree/main/utils) to [React Navtie repo](https://github.com/facebook/react-native/tree/main/sdks/hermes-engine/utils) a while ago to have more control over them. However some paths on the CI were not updated. We continued to use old build scripts for Hermes prebuilds. Some unfortunate side effects are:
- `HERMES_ENABLE_DEBUGGER` is [hardcoded to true](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L65). That makes Hermes much slower in Release configuration.
- BUILD_TYPE is [set to Release](https://github.com/facebook/hermes/blob/main/utils/build-apple-framework.sh#L10) instead of `MinSizeRel` which inreases Hermes binary size.

This diff copies these build utils from RN to Hermes source directory before we perform Hermes build.

Changelog
[Internal]

Reviewed By: cipolleschi

Differential Revision: D44066721

fbshipit-source-id: f45ad6a31fb01c10199f69cc7bbcbbc83b793d34
  • Loading branch information
dmytrorykun authored and facebook-github-bot committed Mar 15, 2023
1 parent a871407 commit b07cf33
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,7 @@ jobs:
command: |
mkdir -p $HERMES_OSXBIN_ARTIFACTS_DIR ./sdks/hermes
cp -r $HERMES_WS_DIR/hermes/* ./sdks/hermes/.
cp -r ./sdks/hermes-engine/utils ./sdks/hermes/.
- brew_install:
package: cmake
- with_hermes_tarball_cache_span:
Expand Down

2 comments on commit b07cf33

@RohovDmytro
Copy link

@RohovDmytro RohovDmytro commented on b07cf33 Apr 17, 2023

Choose a reason for hiding this comment

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

Wow. Nice catch!

Does this have a change to resolve #36296 ?

@jaexplorer
Copy link

Choose a reason for hiding this comment

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

I can't see this in the change log, hopefully 72 fixes performance issues

Please sign in to comment.