-
Notifications
You must be signed in to change notification settings - Fork 929
use pod installed by bundler if possible #2669
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
use pod installed by bundler if possible #2669
Conversation
| export async function execaPod(args: string[], options?: execa.Options) { | ||
| let podType: 'system' | 'bundle' = 'system'; | ||
| try { | ||
| await execa('bundle', ['exec', 'pod', '--version'], options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be quicker to run bundle show cocoapods. That way it doesn't need to initialise cocoapods, it just asks bundler if it can resolve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether it remains as an bundle exec pod --version or becomes a bundle show cocoapods;
Does this need to be more robust?
I'm asking because currently, it'd fail if bundle install hasn't been run and would then fall back to the system pod even if it's meant to be using bundler.
If the first command fails you could potentially check the output of bundle check to see if cocoapods is listed as a missing gem?
But, this may be more complex than it needs to be? I'll defer to a project maintainer on that one. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm asking because currently, it'd fail if bundle install hasn't been run and would then fall back to the system pod even if it's meant to be using bundler.
Nice catch. This would move more users to installing cocoapods globally, but it seems this is not the intended way since the template that is used by the cli is installing cocoapods with bundler anyway (https://github.com/react-native-community/template/blob/main/template/Gemfile#L7).
It would be a breaking change in but I would love to have only the bundler way.
| try { | ||
| await execa('pod', ['--version'], options); | ||
| podType = 'system'; | ||
| } catch (systemPodError) { | ||
| throw new Error('cocoapods not installed'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only want to fall back to pod when bundle is not installed, which you can check separately by trying to run bundle install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed some changes. It will now try bundle install first before falling back to pod. Can you please have another look?
|
@buschco this is great start. Mind following up with a bit more robust logic please? |
e1a0ffe to
d85ce6c
Compare
|
I'm running into the same problem. It would be awesome to get this in! |
Summary
see #2668 and #2663 (should fix both).
This change ensures that local installed cocoapods (
bundle exec pod) is used before system installedpod.Test Plan
A: Bundled Version:
gem uninstall cocoapodsnpx @react-native-community/cli@latest init MyAppnfor cocoa pods questioncd MyAppbundle installNow
bundle exec pod --versionhas an 0 exit code.npx react-native run-ios --force-podswill not fail nor install cocoapods globally (podwill have exit code > 0)B: Global Version (someone could argue this should never be supported anyway):
gem install cocoapodsnpx @react-native-community/cli@latest init MyAppnfor cocoa pods questioncd MyAppcocoapodsfromGemfilebundle installNow
bundle exec pod --versionhas an non 0 exit code.npx react-native run-ios --force-podswill not fail and install cocoapods globally (podwill have non 0 exit code)Checklist
react-nativecheckout (instructions).