Skip to content

Conversation

@christopherfujino
Copy link
Contributor

Fixes #88897

Previously we were calling cmake and catching ArgumentError for the case where cmake was not installed. However, per #88897, it was actually throwing a ProcessException. Instead, explicitly check processManager.canRun('cmake') (rather than catching all ProcessExceptions), and if it is not runnable give the user the same UserMessage the doctor validator would.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 13, 2022
Copy link
Contributor

@Jasguerrero Jasguerrero left a comment

Choose a reason for hiding this comment

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

LGTM

&& targetPlatform == TargetPlatform.linux_arm64;
int result;
if (!globals.processManager.canRun('cmake')) {
throwToolExit(globals.userMessages.cmakeMissing);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to remove the on ArgumentError block? Or at least make it throw the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good thinking

Copy link
Contributor Author

@christopherfujino christopherfujino May 14, 2022

Choose a reason for hiding this comment

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

I removed the try-catch, as I'm assuming if we get an ArgumentError that means it's a tool bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can tell I wrote this code when I was less familiar with Dart. Otherwise the fact that I was catching an Error type in the first place would have been a big red flag that the code was wrong...

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot merged commit 0ad0dc2 into flutter:master May 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 17, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProcessException: Failed to find "cmake" in the search path

6 participants