refactor cocoapods validator to detect broken install #38560

christopherfujino · 2019-08-14T19:38:34Z

Description

When CocoaPods are installed they are dependent on a specific version of ruby. If the pod script is invoked while a different version of ruby is set as the default, it will fail with an error message. The most common cause for this is upgrading macOS, which changes the system ruby version, however it can happen in other situations (the most annoying of which is the RVM situation). sudo gem install cocoapods will usually fix this, but I added a link to the issue which has various variations of fixes.

Related Issues

Fixes this issue.

Tests

I added the following tests:

Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See [Test Coverage].

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

…king

fluttergithubbot · 2019-08-14T19:38:37Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

…king

codecov · 2019-08-22T21:54:00Z

Codecov Report

Merging #38560 into master will increase coverage by 0.29%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master   #38560      +/-   ##
==========================================
+ Coverage   56.34%   56.63%   +0.29%     
==========================================
  Files         195      195              
  Lines       18355    18363       +8     
==========================================
+ Hits        10342    10400      +58     
+ Misses       8013     7963      -50

Flag	Coverage Δ
#flutter_tool	`56.63% <55.55%> (+0.29%)`	⬆️

Impacted Files	Coverage Δ
...ages/flutter_tools/lib/src/base/user_messages.dart	`49.05% <0%> (-0.47%)`	⬇️
...utter_tools/lib/src/macos/cocoapods_validator.dart	`81.81% <33.33%> (-7.66%)`	⬇️
...ackages/flutter_tools/lib/src/macos/cocoapods.dart	`85.98% <80%> (-1.4%)`	⬇️
packages/flutter_tools/lib/src/base/utils.dart	`85.64% <0%> (-7.43%)`	⬇️
packages/flutter_tools/lib/src/version.dart	`90.9% <0%> (-1.92%)`	⬇️
...ckages/flutter_tools/lib/src/base/file_system.dart	`67.85% <0%> (-1.79%)`	⬇️
packages/flutter_tools/lib/src/cache.dart	`43.02% <0%> (-0.73%)`	⬇️
packages/flutter_tools/lib/src/base/logger.dart	`82.06% <0%> (-0.39%)`	⬇️
packages/flutter_tools/lib/src/vmservice.dart	`38.07% <0%> (+0.76%)`	⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98ef956...548b091. Read the comment docs.

codecov · 2019-08-22T21:54:05Z

Codecov Report

Merging #38560 into master will decrease coverage by 0.22%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master   #38560      +/-   ##
==========================================
- Coverage   56.34%   56.12%   -0.23%     
==========================================
  Files         195      195              
  Lines       18355    18362       +7     
==========================================
- Hits        10342    10305      -37     
- Misses       8013     8057      +44

Flag	Coverage Δ
#flutter_tool	`56.12% <55.55%> (-0.23%)`	⬇️

Impacted Files	Coverage Δ
...ages/flutter_tools/lib/src/base/user_messages.dart	`47.16% <0%> (-2.36%)`	⬇️
...utter_tools/lib/src/macos/cocoapods_validator.dart	`81.81% <33.33%> (-7.66%)`	⬇️
...ackages/flutter_tools/lib/src/macos/cocoapods.dart	`85.98% <80%> (-1.4%)`	⬇️
.../flutter_tools/lib/src/android/android_studio.dart	`13.74% <0%> (-36.26%)`	⬇️
...ackages/flutter_tools/lib/src/commands/attach.dart	`47.05% <0%> (-27.28%)`	⬇️
.../flutter_tools/lib/src/runner/flutter_command.dart	`76.75% <0%> (-3.08%)`	⬇️
packages/flutter_tools/lib/src/version.dart	`90.9% <0%> (-1.92%)`	⬇️
packages/flutter_tools/lib/src/cache.dart	`43.02% <0%> (-0.73%)`	⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98ef956...10e98ee. Read the comment docs.

blasten · 2019-08-22T23:21:44Z

packages/flutter_tools/lib/src/macos/cocoapods_validator.dart

+      } else if (cocoaPodsStatus == CocoaPodsStatus.brokenInstall) {
+        status = ValidationType.missing;
+        messages.add(ValidationMessage.error(
+            userMessages.cocoaPodsBrokenInstall(brokenCocoaPodsConsequence, cocoaPodsInstallInstructions)));


minor nit: considering making cocoaPodsBrokenInstall if the consequence and instructions don't change.

blasten

LGTM

jmagman

LGTM with nit

jmagman · 2019-08-22T23:31:51Z

packages/flutter_tools/lib/src/macos/cocoapods.dart


+const String brokenCocoaPodsConsequence = '''
+  You appear to have CocoaPods installed but it is not working.
+  This can happen if the version of ruby that CocoaPods was installed with is different from the one being used to invoke it.


Ruby capitalized

* refactor cocoapods validator to detect broken install (#38560) * Skip pod initialization if version >= 1.8.0. (#41491)

refactor cocoapods validator to detect if it is installed but not wor…

4bc1133

…king

christopherfujino added the work in progress; do not review label Aug 14, 2019

fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 14, 2019

googlebot added the cla: yes label Aug 14, 2019

refactor cocoapods validator to detect if it is installed but not wor…

51561ef

…king

christopherfujino force-pushed the update-cocoapods-validator branch from 4bc1133 to 51561ef Compare August 22, 2019 20:35

update tests to new behavior

548b091

christopherfujino removed the work in progress; do not review label Aug 22, 2019

christopherfujino marked this pull request as ready for review August 22, 2019 22:07

update copy.

fe06146

blasten reviewed Aug 22, 2019

View reviewed changes

blasten approved these changes Aug 22, 2019

View reviewed changes

jmagman approved these changes Aug 22, 2019

View reviewed changes

christopherfujino added 2 commits August 23, 2019 09:16

merge

2b7c3df

code review

10e98ee

christopherfujino merged commit d1e0273 into flutter:master Aug 23, 2019

christopherfujino deleted the update-cocoapods-validator branch August 23, 2019 16:21

zanderso mentioned this pull request Oct 16, 2019

V1.9.1 hotfixes #42863

Merged

zanderso added a commit that referenced this pull request Oct 17, 2019

V1.9.1 hotfixes (#42863)

1aedbb1

* refactor cocoapods validator to detect broken install (#38560) * Skip pod initialization if version >= 1.8.0. (#41491)

jmagman mentioned this pull request Oct 18, 2019

Unhandled exception due to missing pod script #42782

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor cocoapods validator to detect broken install #38560

refactor cocoapods validator to detect broken install #38560

Uh oh!

christopherfujino commented Aug 14, 2019 •

edited

Loading

Uh oh!

fluttergithubbot commented Aug 14, 2019

Uh oh!

codecov bot commented Aug 22, 2019

Uh oh!

codecov bot commented Aug 22, 2019 •

edited

Loading

Uh oh!

blasten Aug 22, 2019

Uh oh!

blasten left a comment

Uh oh!

jmagman left a comment

Uh oh!

jmagman Aug 22, 2019

Uh oh!

christopherfujino Aug 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

refactor cocoapods validator to detect broken install #38560

refactor cocoapods validator to detect broken install #38560

Uh oh!

Conversation

christopherfujino commented Aug 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

fluttergithubbot commented Aug 14, 2019

Uh oh!

codecov bot commented Aug 22, 2019

Codecov Report

Uh oh!

codecov bot commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blasten Aug 22, 2019

Choose a reason for hiding this comment

Uh oh!

blasten left a comment

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

jmagman Aug 22, 2019

Choose a reason for hiding this comment

Uh oh!

christopherfujino Aug 23, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

christopherfujino commented Aug 14, 2019 •

edited

Loading

codecov bot commented Aug 22, 2019 •

edited

Loading