Skip to content

Conversation

@imWildCat
Copy link

@imWildCat imWildCat commented Jun 27, 2024

Summary:

Since 0.75-rc.x, I cannot run pod install because of an linking issue of react native firebase.

reactwg/react-native-releases#341 (comment)

Changelog:

[IOS][FIXED] Auto linking script of script phase

Test Plan:

Full demo of this fix: https://github.com/imWildCat-archived/react-native-075-rc2-regression-ios-linking-script-phase

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 27, 2024
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jun 27, 2024
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Hi @imWildCat, thanks for taking a stab at this and to create reproducers.
Tampering with the scriptPhases is not a good solution, it feels more like a patch.

Let's see if we can find a better solutions first!

@imWildCat
Copy link
Author

imWildCat commented Jun 27, 2024

Hi @imWildCat, thanks for taking a stab at this and to create reproducers. Tampering with the scriptPhases is not a good solution, it feels more like a patch.

Let's see if we can find a better solutions first!

yeah, sure!

please feel free to close this PR if a better solution comes

@imWildCat imWildCat marked this pull request as draft June 27, 2024 15:57
@blakef
Copy link
Contributor

blakef commented Jun 27, 2024

You're super close to the solution. This line needs adjusting:

iff --git a/packages/react-native/scripts/cocoapods/autolinking.rb b/packages/react-native/scripts/cocoapods/autolinking.rb
index fec826c541e..9508625fc4c 100644
--- a/packages/react-native/scripts/cocoapods/autolinking.rb
+++ b/packages/react-native/scripts/cocoapods/autolinking.rb
@@ -168,7 +168,7 @@ def link_native_modules!(config)

         # Support passing in a path relative to the root of the package
         if phase["path"]
-          phase["script"] = File.read(File.expand_path(phase["path"], package["root"]))
+          phase["script"] = File.read(File.expand_path(phase["path"], package[:root]))
           phase.delete("path")
         end

Do you want to update your diff?

@blakef blakef self-assigned this Jun 27, 2024
@imWildCat imWildCat marked this pull request as ready for review June 27, 2024 17:04
@imWildCat
Copy link
Author

@blakef done. thank you so much!

@imWildCat
Copy link
Author

imWildCat commented Jun 27, 2024

Update: I did some test runs on my repro repo

This does not seem to work (https://github.com/imWildCat-archived/react-native-075-rc2-regression-ios-linking-script-phase/actions/runs/9700745283/job/26772863511):

diff --git a/before.txt b/after.txt
index fec826c..9508625 100644
--- a/before.txt
+++ b/after.txt
@@ -168,7 +168,7 @@ def link_native_modules!(config)
 
         # Support passing in a path relative to the root of the package
         if phase["path"]
-          phase["script"] = File.read(File.expand_path(phase["path"], package["root"]))
+          phase["script"] = File.read(File.expand_path(phase["path"], package[:root]))
           phase.delete("path")
         end
ios git:(main) pod install --repo-update
RNFBAnalytics: Using default Firebase/Analytics with Ad Ids. May require App Tracking Transparency. Not allowed for Kids apps.
RNFBAnalytics: You may set variable `$RNFirebaseAnalyticsWithoutAdIdSupport=true` in Podfile to use analytics without ad ids.
Found 3 modules for target `AwesomeProject7`
link_native_modules! {:ios_packages=>[{:configurations=>[], :name=>"@react-native-firebase/analytics", :path=>"../node_modules/@react-native-firebase/analytics", :podspec_path=>"/Volumes/QuickMac/temp/202406/AwesomeProject7/node_modules/@react-native-firebase/analytics/RNFBAnalytics.podspec", :script_phases=>[]}, {:configurations=>[], :name=>"@react-native-firebase/app", :path=>"../node_modules/@react-native-firebase/app", :podspec_path=>"/Volumes/QuickMac/temp/202406/AwesomeProject7/node_modules/@react-native-firebase/app/RNFBApp.podspec", :script_phases=>[{"name"=>"[RNFB] Core Configuration", "path"=>"./ios_config.sh", "execution_position"=>"after_compile", "input_files"=>["$(BUILT_PRODUCTS_DIR)/$(INFOPLIST_PATH)"]}]}, {:configurations=>[], :name=>"react-native-svg", :path=>"../node_modules/react-native-svg", :podspec_path=>"/Volumes/QuickMac/temp/202406/AwesomeProject7/node_modules/react-native-svg/RNSVG.podspec", :script_phases=>[]}], :ios_project_root_path=>"/Volumes/QuickMac/temp/202406/AwesomeProject7/ios", :react_native_path=>"../node_modules/react-native"}
RNFBAnalytics: Using default Firebase/Analytics with Ad Ids. May require App Tracking Transparency. Not allowed for Kids apps.
RNFBAnalytics: You may set variable `$RNFirebaseAnalyticsWithoutAdIdSupport=true` in Podfile to use analytics without ad ids.
Adding a custom script phase for Pod RNFBApp: [RNFB] Core Configuration

[!] Invalid `Podfile` file: No such file or directory @ rb_sysopen - /Volumes/QuickMac/temp/202406/AwesomeProject7/ios/ios_config.sh.

 #  from /Volumes/QuickMac/temp/202406/AwesomeProject7/ios/Podfile:18
 #  -------------------------------------------
 #  target 'AwesomeProject7' do
 >    config = use_native_modules!
 #  
 #  -------------------------------------------

@cipolleschi
Copy link
Contributor

Yup, I was testing the same.
The right fix is not :root but :path.

This works:

if phase["path"]
-  phase["script"] = File.read(File.expand_path(phase["path"], package[:root]))
+  phase["script"] = File.read(File.expand_path(phase["path"], package[:path]))
  phase.delete("path")
end

@cipolleschi
Copy link
Contributor

sorry for the back and forth

@imWildCat
Copy link
Author

imWildCat commented Jun 27, 2024

no worries at all! thanks!

https://github.com/imWildCat-archived/react-native-075-rc2-regression-ios-linking-script-phase/actions/runs/9700897360/job/26773347289

test run (green)

@imWildCat imWildCat changed the title resolve absolute path of the script phase fix the path of the script phase Jun 27, 2024
@imWildCat imWildCat changed the title fix the path of the script phase [iOS][FIXED] fix the path of the script phase Jun 27, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 27, 2024
@facebook-github-bot
Copy link
Contributor

@blakef merged this pull request in e320ab4.

@github-actions
Copy link

This pull request was successfully merged by imWildCat in e320ab4.

When will my fix make it into a release? | How to file a pick request?

@cipolleschi
Copy link
Contributor

committed @ e320ab4

@mikehardy
Copy link
Contributor

@imWildCat thank you so much for spotting this and more importantly the PR with fix 🎉

@imWildCat
Copy link
Author

@imWildCat thank you so much for spotting this and more importantly the PR with fix 🎉

My pleasure!

Thanks a lot to the maintainers for the suggestions and prompt review!

cortinico pushed a commit that referenced this pull request Jul 1, 2024
Summary:
Since 0.75-rc.x, I cannot run pod install because of an linking issue of react native firebase.

reactwg/react-native-releases#341 (comment)

## Changelog:

[IOS][FIXED] Auto linking script of script phase

Pull Request resolved: #45208

Test Plan: Full demo of this fix: <https://github.com/imWildCat-archived/react-native-075-rc2-regression-ios-linking-script-phase>

Reviewed By: christophpurrer

Differential Revision: D59125585

Pulled By: blakef

fbshipit-source-id: be96d3b207eff67c5e0d777203e7fc0d10103fc0
facebook-github-bot pushed a commit that referenced this pull request Aug 23, 2024
Summary:
A previous attempt at fixing this issue used a relative path (#45208), this doesn't work if the user runs bundle install outside of the `ios/`
folder, using the `--project-directory=ios` argument.

## Changelog:
[iOS][Fixed] support bundle install from outside the ios folder using --project-directory

Pull Request resolved: #46186

Test Plan:
Ran the command in a project with `react-native-firebase/app` using the
`--project-directory`, confirmed that it's fixed when using the absolute
path.

closes: reactwg/react-native-releases#341

Reviewed By: cipolleschi

Differential Revision: D61719821

Pulled By: blakef

fbshipit-source-id: d83429dd29c9e8cc066ab9843ad95fdfc0af8dea
cipolleschi pushed a commit that referenced this pull request Sep 11, 2024
Summary:
A previous attempt at fixing this issue used a relative path (#45208), this doesn't work if the user runs bundle install outside of the `ios/`
folder, using the `--project-directory=ios` argument.

## Changelog:
[iOS][Fixed] support bundle install from outside the ios folder using --project-directory

Pull Request resolved: #46186

Test Plan:
Ran the command in a project with `react-native-firebase/app` using the
`--project-directory`, confirmed that it's fixed when using the absolute
path.

closes: reactwg/react-native-releases#341

Reviewed By: cipolleschi

Differential Revision: D61719821

Pulled By: blakef

fbshipit-source-id: d83429dd29c9e8cc066ab9843ad95fdfc0af8dea
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. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants