Skip to content

Commit

Permalink
fix: Correctly assign the hermes-engine pod tag when installing pod…
Browse files Browse the repository at this point in the history
…s from a different folder (#38754)

Summary:
This PR aims to fix an issue where installing iOS pods from a different directory causes the `hermes-engine` `:tag:` not to be properly resolved.

Ever since #37148 was merged, the `setup_hermes` script tries to resolve the `node_modules/react-native/sdks/.hermesversion` file and use its content to generate the pod tag, in order to verify when it changes over time.
This works perfectly when installing pods within the `ios` folder, as the `react_native_path` should point to the correct relative folder (`../node_modules/react-native` by default).

However, when installing pods from a different directory (the project root, for example) and leveraging the `--project-directory` flag, the file fails to resolve, as the current working directory is considered when resolving the `.hermesversion` file.

### Quick Example:

- `react_native_path`: `../node_modules/react-native` (the default config)

**Installing pods from the `ios` folder:**
- `cd ios`
- `bundle exec pod install`
- `hermestag_file` resolved path: `$project_root/node_modules/react-native` ✅
- `hermes-engine` `:tag:`: `hermes-2023-03-20-RNv0.72.0-49794cfc7c81fb8f69fd60c3bbf85a7480cc5a77` ✅

**Installing pods from the `$project_root` folder**
- `bundle exec pod install --project-directory=ios`
- `hermestag_file` resolved path: `$parent_folder/$project_root/node_modules/react-native` ❌
- `hermes-engine` `:tag:`: `''` ❌

### The fix

Turns out that the same file had a resolved reference to the `react-native` folder, assigned to the `react_native_dir` variable:
```ruby
react_native_dir = Pod::Config.instance.installation_root.join(react_native_path)
```

By resolving the `.hermesversion` using that folder, we guarantee that the relative path will always reference the directory where the `Podfile` is defined, which is the expected behaviour.

## Changelog:

[Internal] - Fix an issue where installing pods from a different directory would fail to resolve `hermes-engine` tags correctly.

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: #38754

Test Plan:
- Init a new `react-native` repo
- Remove the generated `Podfile.lock` file
- Navigate to the project root folder
- `bundle exec pod install -project-directory=ios`
- Check that the `hermes-engine` entry has a properly populated `:tag:` attribute:

**Before:**
```
hermes-engine:
    :podspec: "../node_modules/react-native/sdks/hermes-engine/hermes-engine.podspec"
    :tag: ''
```

**After:**
```
hermes-engine:
    :podspec: "../node_modules/react-native/sdks/hermes-engine/hermes-engine.podspec"
    :tag: hermes-2023-03-20-RNv0.72.0-49794cfc7c81fb8f69fd60c3bbf85a7480cc5a77
```

Reviewed By: cortinico

Differential Revision: D48029413

Pulled By: cipolleschi

fbshipit-source-id: 82d465abd5c888eeb9eacd32858fa4ecf4f8c217
  • Loading branch information
gvarandas authored and Luna Wei committed Aug 7, 2023
1 parent 3350dd8 commit a601b22
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion packages/react-native/scripts/cocoapods/jsengine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def setup_hermes!(react_native_path: "../node_modules/react-native", fabric_enab
pod 'React-jsi', :path => "#{react_native_path}/ReactCommon/jsi"
# This `:tag => hermestag` below is only to tell CocoaPods to update hermes-engine when React Native version changes.
# We have custom logic to compute the source for hermes-engine. See sdks/hermes-engine/*
hermestag_file = File.join(react_native_path, "sdks", ".hermesversion")
hermestag_file = File.join(react_native_dir, "sdks", ".hermesversion")
hermestag = File.exist?(hermestag_file) ? File.read(hermestag_file).strip : ''

pod 'hermes-engine', :podspec => "#{react_native_path}/sdks/hermes-engine/hermes-engine.podspec", :tag => hermestag
Expand Down

0 comments on commit a601b22

Please sign in to comment.