-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat(ios): Add Bundler support #5205
Conversation
I didn't dare to touch the iOS project template just yet. Once this change is accepted, it might make sense to do that as well to better isolate iOS builds from the host machine used. |
9471caf
to
1a600e1
Compare
I addressed the eslint issues but now need a workflow approval / re-run. TIA. |
fdc46a3
to
5b3f441
Compare
5b3f441
to
c9a3c3c
Compare
c9a3c3c
to
b7ac582
Compare
Any remarks or comments? |
b7ac582
to
4dae0fe
Compare
Once again, any news, decisions or comments on the matter? |
It would be nice if this could get some feedback, I just rebased onto most recent |
This is awesome 👍🏽 hope this can be merged as fast as possible. |
4dae0fe
to
2184698
Compare
Hey, bump on this one, does the Ionic team see any value in bringing this into the project? |
Let me take a look - I do know that bundler is popular along side cocoapods - I know I used it. |
Hey, please update this with the latest from main as well. As best as you can anyway - I know we're coming fast and furious with commits - but I don't like to update other peoples forks unless I'm about to merge any minute. |
2184698
to
3fbb292
Compare
And so I keep it current even though I already gave up hope that contributing to CapacitorJS is a thing at all. |
Now that some attention is on this one, may I shamelessly point you guys to #5195 as well? It's equally important when trying to use latest Android SDK versions |
Sure - but I'm more iOS leaning so I'll have to pull in some help - we are bumping the target to 33 in Cap5 (now in alpha!) |
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.
Looks good, only thing we might want to pull is the CTA about installing Bundler - but I'll have to ask the team.
Let's see wait they say. I'd be happy to adjust the default project template afterwards. |
@saschpe You have a conflict here since I removed the deletion of the |
I'm currently rebasing all my open pull requests... |
3fbb292
to
69d109f
Compare
I haven't forgotten about this - I'm going to see if we can get this into the 5-rc or if not that, the next point release. |
Please see my new comment: |
Bundler isolates RubGems dependencies from the host operating system and creates a consistent environment. Gem dependencies can be pinned to exact versions by using a `Gemfile`. CocoaPods is modeled after Bundler. This ensures all developers and CI machines use the same CocoaPods version. It's also conveniently used to install CocoaPods plugins, e.g. `cocoapods-art`, or additional iOS development tools, such as `xcpretty`. Multiple cases are now handled correctly. If a `Gemfile` is found: - At the Git repository root, in case of a multi-app project - At or below the app directory, in case of a single-app project Then: 1. Use Bundler to install RubyGems, including CocoaPods: 1. If Bundler is outdated, update first by executing `gem install bundler` 2. If RubyGems bundle is outdated, update first by executing `bundle install` 3. Install CocoaPods and other gems by executing `bundle exec pod install` Or, just like before, if: 4. CocoaPods is available on PATH, execute `pod install` 5. Neither global CocoaPods nor Gemfile are found Skip CocoaPods install entirely Implements ionic-team#5177
19dfb8f
to
08b2666
Compare
Is there a way to tell Capacitor 5 to ignore or opt-out of the bundler integration? We have a Gemfile for automation purposes, but don't want it used for development at all (at least not yet). |
I'm trying to figure out if I'm seeing an issue with this change. Running
The update code appears to confusingly think that it's not using bundler, despite the earlier message. Tracing down further, I think the issue is that it's using single quotes in the test to determine if Cocoapods is referenced in the Gemfile, but it's valid to use double quotes here and our file does. |
Double quoted strings allow interpolation which is a bit unusual in the case of gem names. They're rather fixed. Then again, Ruby has another string literal variant, percent strings like While it's straightforward to expand the test to check for double-quoted strings, I am not sure we'd want to support percent strings as well. I can already see someone who is using regex strings :-) |
Yes, how do I opt out of this? We have a Gemfile for fastlane but this is trying to install bundler on xcode cloud build? |
You're using Fastlane and Xcode Cloud? There aren't many reasons left these days to still use Fastlane. Almost everything can be achieved with |
You’re using bundled to install cocoa pods? |
Bundler isolates RubGems dependencies from the host operating system and creates a consistent environment. Gem dependencies can be pinned to exact versions by using a
Gemfile
. CocoaPods is modeled after Bundler.This ensures all developers and CI machines use the same CocoaPods version. It's also conveniently used to install CocoaPods plugins, e.g.
cocoapods-art
, or additional iOS development tools, such asxcpretty
.Multiple cases are now handled correctly. If a
Gemfile
is found:Then:
gem install bundler
bundle install
bundle exec pod install
Or, just like before, if:
pod install
Skip CocoaPods install entirely
Implements #5177
Remarks
The following screenshots demonstrate the CLI output and actions that now occur (in reverse order but with increasing coolness):
Case 5
Case 4
Case 1.2 and 1.3
Case 1.1 and 1.3