Skip to content
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(cli): add support for 'pod install' in VM based environments #5144

Merged
merged 13 commits into from
Oct 27, 2021

Conversation

thomasvidas
Copy link
Contributor

Fixes #5069

Directly check for pod executable rather than just failing on not Mac OS. Allows for use of capacitor if running the cli from within a vm container like docker or vagrant.

@Ionitron Ionitron added this to Needs review 🤔 in Capacitor Engineering ⚡️ Oct 15, 2021
@jcesarmobile
Copy link
Member

The OS check as there more because of xcodebuild than because of pod install. I don’t think you can run xcodebuild on Linux. I have not tried the change on a vm, but it will probably fail because of that.
If we want to add this, then xcodebuild shouldn’t be run.

@thomasvidas
Copy link
Contributor Author

I added a function to check if xcodebuild is installed. In the future we should refactor to have a "commandExists" function or something to check if a command is able to be run, but I think that is outside the scope of this PR

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

Added three comments.

We in fact have an isInstalled function already, so should use that instead of which (it uses which internally) and instead of having two functions for checking.

cli/src/ios/update.ts Outdated Show resolved Hide resolved
cli/src/ios/update.ts Outdated Show resolved Hide resolved
cli/src/ios/update.ts Outdated Show resolved Hide resolved
thomasvidas and others added 5 commits October 19, 2021 13:44
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
@thomasvidas
Copy link
Contributor Author

Finally made the changes to this 😄

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

I've removed some unused imports, the platform check since the update file is the iOS file, so the platform is always iOS, and also moved the pod check right before running pod install since users might still want the Podfile to be updated despite they don't have CocoaPods installed in case they run update on windows/linux but build on mac without running update there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

bug: Capacitor 3.x sync command stopped running on vagrant (or any other virtualization platform)
2 participants