fix: CLI breaks process when pod install has not failed #3943
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fatme
reviewed
Sep 27, 2018
lib/services/cocoapods-service.ts
Outdated
| this.$errors.failWithoutHelp("CocoaPods or ruby gem 'xcodeproj' is not installed. Run `sudo gem install cocoapods` and try again."); | ||
| } | ||
|
|
||
| await this.$xcprojService.verifyXcproj(true); |
There was a problem hiding this comment.
boolean variable passed as param 🙀 🙀 🙀 😶
| await this.prepareCocoapods(pluginPlatformsFolderPath, pluginData, projectData); | ||
|
|
||
| const projectRoot = this.getPlatformData(projectData).projectRoot; | ||
| await this.$cocoapodsService.applyPluginPodfileToProject(pluginData, projectData, projectRoot); |
There was a problem hiding this comment.
This code was included in prepareCocoapods method
if (opts && opts.executePodInstall && this.$fs.exists(pluginPodFilePath)) {
await this.executePodInstall(projectData);
}
but I can't see to call it in the new implementation. Maybe I'm missing something?
There was a problem hiding this comment.
opts were never passed, so I just removed this code
The CocoapodsService is designed to execute all pod related operations, so move the `executePodInstall` method from `IosProjectService` to `CocoapodsService`.
In some cases `pod install` command prints data on the `stderr` (for example when setup the repo for the first time or when updating it). CLI detects the data on stderr and fails the operation. However these are not real errors.
Fix this by passing `{ throwError: false }` to `spawnFromEvent` method. This way it will not fail in any case. Check the exit code of the operation and if it is not 0, just fail.
Pipe the stderr of the `pod install` process to stdout of CLI, this way we'll not polute CLI's stderr with some unrelevant info.
NOTE: `pod install` command prints the real errors on stdout, not stderr.
Add missing test for #3935
rosen-vladimirov
force-pushed
the
vladimirov/cocoapods-errors-parsing
branch
from
a970cbe to
2db98d0
Compare
Fatme
approved these changes
Sep 27, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First commit:
refactor: move executePodInstall method to CocoapodsService
The CocoapodsService is designed to execute all pod related operations, so move the
executePodInstallmethod fromIosProjectServicetoCocoapodsService.fix: CLI breaks process when pod install has not failed
The fix and related unit tests are in the second commit:
In some cases
pod installcommand prints data on thestderr(for example when setup the repo for the first time or when updating it). CLI detects the data on stderr and fails the operation. However these are not real errors.Fix this by passing
{ throwError: false }tospawnFromEventmethod. This way it will not fail in any case. Check the exit code of the operation and if it is not 0, just fail.Pipe the stderr of the
pod installprocess to stdout of CLI, this way we'll not polute CLI's stderr with some unrelevant info.NOTE:
pod installcommand prints the real errors on stdout, not stderr.PR Checklist
What is the current behavior?
When
pod installmethod prints some information on stderr (for example when this is the first execution of the command, it setups the master repo and prints progress indicator on stderr), CLI process stops with errorpod install failed....What is the new behavior?
CLI continues the execution in the mentioned scenario. CLI will stop only when
pod installcommand exits with non-zero exit code.Fixes issue #3686