Skip to content

Conversation

@jcesarmobile
Copy link
Member

When bundler support was added, it broke a lot of things.

First of all, Capacitor already had bundler support, but by using an executable that would run the bundle exec pod command instead of pod command.
The PR shouldn't have repurposed podPath as it was for pointing to the executable path, but in that PR it was changed to contain the command to run, which breaks some checks like isInstalled(podPath), and also doesn't really work if passed to runCommand as it only expects one command and that would pass bundle exec pod if using bundler. (no bug here since we were reading the value and manually calling just bundle and then exec and pod as params.

Also added a bunch of checkBundler() calls which cause issues and race conditions when not using bundler.

So, this PR adds a new packageManager property to the iOS config, which gets the value from a new determinePackageManager function with a similar logic to what determineGemfileOrCocoapodPath used to have (including a SPM check and only sets the value to bundler if installed.
The values can be bundler, Cocoapods or SPM
Then uses that property to only run checkBundler() if using bundler and only run checkCocoaPods() if using CocoaPods, and no checks at all if using SPM.
So it closes #8127

determineGemfileOrCocoapodPath has been deleted since it wasn't exported, so I don't think it's a breaking change.
Added a new determineCocoapodPath with the original logic that was removed on the mentioned bundler PR, so it only checks if the CAPACITOR_COCOAPODS_PATH env variable is defined or not.

Deprecated checkPackageManager and updateChecks since those were exported and removing them would be a breaking change.
We can delete them on main, but this should be cherry picked into 7.x and 8.x, so can't be deleted yet.

Changed some checks from == 'Cocoapods' to !== 'SPM' since bundler and Cocoapods are basically the same, except on a few places where I explicitly check for bundler.

Removes some isInstalled('bundler') and isInstalled('pod') since those are tested beforehand, and for pod it shouldn't check that but isInstalled(podPath) in case podPath was set to an executable path.

}
}

async function getPackageManager(config: Config, packageManager: any): Promise<string> {
Copy link

Choose a reason for hiding this comment

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

Return type could be narrowed down instead of Promise.

): Promise<string> {
if (process.env.CAPACITOR_COCOAPODS_PATH) {
return process.env.CAPACITOR_COCOAPODS_PATH;
): Promise<'Cocoapods' | 'bundler' | 'SPM'> {
Copy link

Choose a reason for hiding this comment

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

This return type is used several times, maybe it makes sense to create a determined type for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Capacitor finds unrelated Gemfile in an SPM project

3 participants