Skip to content

[flutter_tools] process exception during linux_doctor is handled#100159

Merged
fluttergithubbot merged 3 commits intoflutter:masterfrom
Jasguerrero:doctor_fails_missing_clang
Mar 17, 2022
Merged

[flutter_tools] process exception during linux_doctor is handled#100159
fluttergithubbot merged 3 commits intoflutter:masterfrom
Jasguerrero:doctor_fails_missing_clang

Conversation

@Jasguerrero
Copy link
Contributor

@Jasguerrero Jasguerrero commented Mar 15, 2022 •
edited by christopherfujino
Loading

Fixes #99944

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 15, 2022
@Jasguerrero Jasguerrero marked this pull request as ready for review March 16, 2022 18:30
christopherfujino
christopherfujino reviewed Mar 16, 2022
View reviewed changes
packages/flutter_tools/test/general.shard/linux/linux_doctor_test.dart Outdated
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io' show ProcessException;
Copy link
Contributor

@christopherfujino christopherfujino Mar 16, 2022

Choose a reason for hiding this comment

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

Importing dart:io is banned in the tool (I'm pretty sure analysis will catch this). You can import this class via: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/base/io.dart#L92

Copy link
Contributor

@christopherfujino christopherfujino Mar 16, 2022

Choose a reason for hiding this comment

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

Interesting, analysis passed. I wonder if we only validate that for non-test code?

@christopherfujino
Copy link
Contributor

christopherfujino commented Mar 16, 2022

Note, if you put "Fixes #99944" in your description, GitHub will link the two, and once the bot merges the PR GitHub will auto-close the issue.

@Jasguerrero Jasguerrero changed the title catch ProcessException when binary fails #99944 catch ProcessException when binary fails Fixes #99944 Mar 16, 2022
@Jasguerrero Jasguerrero changed the title catch ProcessException when binary fails Fixes #99944 Fixes #99944 Mar 16, 2022
christopherfujino
christopherfujino reviewed Mar 16, 2022
View reviewed changes
packages/flutter_tools/lib/src/linux/linux_doctor.dart Outdated
} on ArgumentError {
// ignore error.
}
on ProcessException {
Copy link
Contributor

@christopherfujino christopherfujino Mar 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

nit this should be on the previous line

@christopherfujino
Copy link
Contributor

christopherfujino commented Mar 16, 2022

Note, if you put "Fixes #99944" in your description, GitHub will link the two, and once the bot merges the PR GitHub will auto-close the issue.

I see you updated the title. It looks like only the description gets the special GitHub parsing magic.

@christopherfujino christopherfujino changed the title Fixes #99944 [flutter_tools] process exception during linux_doctor is handled Mar 16, 2022
christopherfujino
christopherfujino approved these changes Mar 16, 2022
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM barring the style nit

@fluttergithubbot fluttergithubbot merged commit 03b4f2b into flutter:master Mar 17, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 18, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 18, 2022
@engine-flutter-autoroll
03b4f2b [flutter_tools] process exception during linux_doctor is hand…
01914d2 
…led (flutter/flutter#100159)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 18, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2022
@engine-flutter-autoroll
03b4f2b [flutter_tools] process exception during linux_doctor is hand…
bfa2105 
…led (flutter/flutter#100159)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 18, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 18, 2022
@engine-flutter-autoroll
03b4f2b [flutter_tools] process exception during linux_doctor is hand…
8fd3ffd 
…led (flutter/flutter#100159)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 18, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 18, 2022
@engine-flutter-autoroll
03b4f2b [flutter_tools] process exception during linux_doctor is hand…
4f2fdfe 
…led (flutter/flutter#100159)
@Jasguerrero Jasguerrero deleted the doctor_fails_missing_clang branch April 11, 2022 22:19
@christopherfujino christopherfujino mentioned this pull request May 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yjbanov yjbanov Awaiting requested review from yjbanov

1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[flutter_tools] when the doctor fails to find clang++, it crashes

3 participants

@Jasguerrero @christopherfujino @fluttergithubbot