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

Remove HERMES_BUILD_FROM_SOURCE flag #35397

Closed
wants to merge 1 commit into from

Conversation

cipolleschi
Copy link
Contributor

Summary:
This Diff removes the HERMES_BUILD_FROM_SOURCE that was not always propagated to the original script. This lead to some cases where hermesC was built during pod install and then removed by the react_native_post_install's else branch.

Changelog:

[iOS][Changed] - Remove HERMES_BUILD_FROM_SOURCE flag

Reviewed By: cortinico

Differential Revision: D41373439

Summary:
This Diff removes the `HERMES_BUILD_FROM_SOURCE` that was not always propagated to the original script. This lead to some cases where hermesC was built during `pod install` and then removed by the `react_native_post_install`'s `else` branch.

## Changelog:
[iOS][Changed] - Remove `HERMES_BUILD_FROM_SOURCE` flag

Reviewed By: cortinico

Differential Revision: D41373439

fbshipit-source-id: eba5b0fa3166cb454eb08cadba88f08ab874c615
@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. p: Facebook Partner: Facebook Partner fb-exported labels Nov 18, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D41373439

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,102,137 +0
android hermes armeabi-v7a 6,470,209 +0
android hermes x86 7,519,643 +0
android hermes x86_64 7,378,296 +0
android jsc arm64-v8a 8,967,154 +0
android jsc armeabi-v7a 7,698,011 +0
android jsc x86 9,029,314 +0
android jsc x86_64 9,507,129 +0

Base commit: 95c5e18
Branch: main

@analysis-bot
Copy link

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

Base commit: c06323f
Branch: main

@pull-bot
Copy link

PR build artifact for 0617d5e is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for 0617d5e is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

cipolleschi added a commit that referenced this pull request Nov 22, 2022
Summary:
Pull Request resolved: #35397

This Diff removes the `HERMES_BUILD_FROM_SOURCE` that was not always propagated to the original script. This lead to some cases where hermesC was built during `pod install` and then removed by the `react_native_post_install`'s `else` branch.

Basically, when the Pods are installed the first time, everything run smoothly. Subsequent invocations of `pod install`, to install other dependencies, for example, will incur in this problem because:
1. Cocoapods will see that hermes-engine is already installed
2. the podspec is not executed, given that the pod has been fetched from the cache
3. The env var is not set (given that the podspec is not executed)
4. the main script sees the env var as not set, `ENV['HERMES_BUILD_FROM_SOURCE'] == "1"` return false
5. The `else` branch is executed, and it removes the `hermesc_build_dir` and the `copy Hermes framework` script phase.

[iOS][Changed] - Remove `HERMES_BUILD_FROM_SOURCE` flag

Reviewed By: cortinico, dmytrorykun

Differential Revision: D41373439

fbshipit-source-id: ea4aafd187c0ca3ff5c0d79f8aeaaa46ad50f499
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Pull Request resolved: facebook#35397

This Diff removes the `HERMES_BUILD_FROM_SOURCE` that was not always propagated to the original script. This lead to some cases where hermesC was built during `pod install` and then removed by the `react_native_post_install`'s `else` branch.

Basically, when the Pods are installed the first time, everything run smoothly. Subsequent invocations of `pod install`, to install other dependencies, for example, will incur in this problem because:
1. Cocoapods will see that hermes-engine is already installed
2. the podspec is not executed, given that the pod has been fetched from the cache
3. The env var is not set (given that the podspec is not executed)
4. the main script sees the env var as not set, `ENV['HERMES_BUILD_FROM_SOURCE'] == "1"` return false
5. The `else` branch is executed, and it removes the `hermesc_build_dir` and the `copy Hermes framework` script phase.

## Changelog:
[iOS][Changed] - Remove `HERMES_BUILD_FROM_SOURCE` flag

Reviewed By: cortinico, dmytrorykun

Differential Revision: D41373439

fbshipit-source-id: ea4aafd187c0ca3ff5c0d79f8aeaaa46ad50f499
@cipolleschi cipolleschi mentioned this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants