Add basic desktop linux checks by jonahwilliams · Pull Request #31873 · flutter/flutter

jonahwilliams · 2019-04-30T21:59:50Z

Description

Adds doctor validation checks for gcc and gtk for linux desktop:

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel master, v1.5.9-pre.62, on Linux, locale en_US.UTF-8)
[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[✓] Linux toolchain - develop for Linux desktop applications
[!] Android Studio (version 3.3)
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
    ✗ Dart plugin not installed; this adds Dart specific functionality.
[✓] IntelliJ IDEA Ultimate Edition (version 2018.2)
[!] IntelliJ IDEA Ultimate Edition (version 2018.3)
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
[!] VS Code (version 1.27.1)
    ✗ Flutter extension not installed; install from
      https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter
[✓] Connected device (1 available)

With -v

[✓] Linux toolchain - develop for Linux desktop applications
    • clang++ 4.0.1
    • GNU Make 4.1

#31368

stuartmorgan-g · 2019-04-30T23:16:47Z

+    final List<ValidationMessage> messages = <ValidationMessage>[];
+    /// Check for some version of GCC.
+    final ProcessResult gccResult = await processManager.run(const <String>[
+      'g++',


The choice of GCC vs clang is specific to the FDE example (and might be something we want to change before templatizing); it doesn't seem like we should have doctor check for things that aren't required by flutter_tool.

How about checking for make, since that's what the tooling actually expects at this point?

The doctor could check make and then for both clang/gcc as a hint? Longer term, I would like to break out each doctor validation check to make it platform/template specific (see go/flutter-tool-extensions).

I'm not familiar yet with the gradations of checks that the doctor support, but an error for not having make and a warning for not having gcc or clang definitely seems reasonable as a starting point. (Substituting whatever the right terminology is here for 'error' and 'warning')

stuartmorgan-g · 2019-04-30T23:18:11Z

+
+    /// Check for the correct version of gtk.
+    final ProcessResult gtkResult = await processManager.run(const <String>[
+      'pkg-config',


We'd need to make sure pkg-config was present first. But as with GCC, I don't think we should actually be requiring that. The shell does have a runtime dependency on GTK, but that could be satisfied without pkg-config being present.

These need not be required, but it could be a hint. Is there a better method to detect whether gtk is there at runtime?

To be clear, when I say it's a runtime dependency I mean that end users of the built application need GTK; it's not a developer-specific thing. That's why I don't think doctor should check for it.

Is there a better method to detect whether gtk is there

It's Linux, so as always the answer seems to be "it depends on your distribution!"

Honestly I think that without a bunch of fallback logic and iteration with different distros we're likely to create a lot of false-positives (i.e., whatever we use to check of the presence of GTK is going to be missing more often than GTK), and that we're better off not doing this for now, and seeing if there's really a need for a GTK check in practice based on feedback.

stuartmorgan-g

Sorry, I totally forget about this PR :(

stuartmorgan-g · 2019-05-08T18:03:36Z

+    final List<ValidationMessage> messages = <ValidationMessage>[];
+    /// Check for some version of GCC.
+    final ProcessResult gccResult = await processManager.run(const <String>[
+      'g++',


I'm not familiar yet with the gradations of checks that the doctor support, but an error for not having make and a warning for not having gcc or clang definitely seems reasonable as a starting point. (Substituting whatever the right terminology is here for 'error' and 'warning')

stuartmorgan-g · 2019-05-08T18:11:15Z

+
+    /// Check for the correct version of gtk.
+    final ProcessResult gtkResult = await processManager.run(const <String>[
+      'pkg-config',


To be clear, when I say it's a runtime dependency I mean that end users of the built application need GTK; it's not a developer-specific thing. That's why I don't think doctor should check for it.

Is there a better method to detect whether gtk is there

It's Linux, so as always the answer seems to be "it depends on your distribution!"

Honestly I think that without a bunch of fallback logic and iteration with different distros we're likely to create a lot of false-positives (i.e., whatever we use to check of the presence of GTK is going to be missing more often than GTK), and that we're better off not doing this for now, and seeing if there's really a need for a GTK check in practice based on feedback.

jonahwilliams · 2019-05-21T17:19:02Z

Closing this for now until I have some more time

stuartmorgan-g · 2019-06-12T05:02:51Z

The template switched to Clang, which I intend to be a permanent change, so when this gets picked up again the initial version that covers everything needed for the simple example app should be:

check for make
check for clang++ (if we want to be fancy, version 3.4+, since we use C++14)

jonahwilliams · 2019-06-17T22:51:03Z

This is back on my radar

jonahwilliams · 2019-06-18T19:24:49Z

Updated to check for clang++ greater than version 3.4 and make

stuartmorgan-g

One more test note, but LGTM

…o desktop_doctor

jonahwilliams · 2019-06-19T18:52:43Z

cc @dnfield, do you know why this change would be failing the release_smoke_tests ? I don't know how to run it locally to debug, but this should have no effect on an Android build

dnfield · 2019-06-19T19:38:52Z

It looks like Gradle is failing to build in release mode with these changes.

The command that's failing is ../../../bin/flutter build appbundle --target-platform android-arm,android-arm64 from dev/integration_tests/release_smoke_test. Is that passing locally?

jonahwilliams · 2019-06-19T19:56:01Z

Works fine locally, though I'm also on a mac

dnfield · 2019-06-19T20:11:50Z

Ah. I think this is the gradle emoji bug.

Can you try replacing:

        - ./dev/bots/firebase_testlab.sh

with

        - echo "$CIRRUS_CHANGE_MESSAGE" > /tmp/cirrus_change_message.txt
        - echo "$CIRRUS_COMMIT_MESSAGE" > /tmp/cirrus_commit_message.txt
        - export CIRRUS_CHANGE_MESSAGE=""
        - export CIRRUS_COMMIT_MESSAGE=""
        - ./dev/bots/firebase_testlab.sh
        - export CIRRUS_CHANGE_MESSAGE=`cat /tmp/cirrus_change_message.txt`
        - export CIRRUS_COMMIT_MESSAGE=`cat /tmp/cirrus_commit_message.txt`

in cirrus.yml?

jonahwilliams · 2019-06-19T21:01:55Z

Didn't work, can we submit on red for this case after I roll back the cirrus config?

tvolkert · 2019-06-19T23:02:34Z

Looks like this is causing the following bot failures:

[✓] Flutter (Channel master, v1.7.6-pre.19, on Linux, locale en_US.UTF-8)
    • Flutter version 1.7.6-pre.19 at /tmp/flutter sdk
    • Framework revision f221ad1f92 (3 minutes ago), 2019-06-19 15:51:21 -0700
    • Engine revision 54f88ab5da
    • Dart version 2.4.0
Invalid argument(s): Cannot find executable for clang++.
#0      _getExecutable (package:process/src/interface/local_process_manager.dart:113:5)
#1      LocalProcessManager.run (package:process/src/interface/local_process_manager.dart:62:7)
#2      LinuxDoctorValidator.validate (package:flutter_tools/src/linux/linux_doctor.dart:22:60)
<asynchronous suspension>
#3      Doctor.startValidatorTasks (package:flutter_tools/src/doctor.dart:152:52)
#4      Doctor.diagnose (package:flutter_tools/src/doctor.dart:223:41)
#5      _AsyncAwaitCompleter.start (dart:async-patch/async_patch.dart:49:6)
#6      Doctor.diagnose (package:flutter_tools/src/doctor.dart:213:24)
#7      DoctorCommand.runCommand (package:flutter_tools/src/commands/doctor.dart:48:39)

This reverts commit f221ad1.

jonahwilliams · 2019-06-19T23:03:45Z

Not sure why CI didn't catch this - possibly some interaction between the PRs I landed today

This reverts commit f221ad1.

add basic desktop linux checks

3c57b93

googlebot added the cla: yes label Apr 30, 2019

Update linux_doctor.dart

461c853

jonahwilliams requested a review from stuartmorgan-g April 30, 2019 22:57

stuartmorgan-g requested changes Apr 30, 2019

View reviewed changes

goderbauer added the tool Affects the "flutter" command-line tool. See also t: labels. label May 1, 2019

stuartmorgan-g reviewed May 8, 2019

View reviewed changes

jonahwilliams closed this May 21, 2019

jonahwilliams reopened this Jun 17, 2019

jonahwilliams added 2 commits June 18, 2019 11:57

Merge branch 'master' of github.com:flutter/flutter into desktop_doctor

a120722

update linux doctor

db53b22

jonahwilliams force-pushed the desktop_doctor branch from 461c853 to db53b22 Compare June 18, 2019 19:23

dont forget copywrite

6120a93

stuartmorgan-g requested changes Jun 18, 2019

View reviewed changes

address comments and fix bugs

1f5d7e5

stuartmorgan-g approved these changes Jun 18, 2019

View reviewed changes

Comment thread packages/flutter_tools/test/linux/linux_doctor_test.dart

jonahwilliams added 4 commits June 18, 2019 15:19

add more tests

e0c5a3c

Merge branch 'master' of github.com:flutter/flutter into desktop_doctor

9a07b4f

Merge branch 'master' of github.com:flutter/flutter into desktop_doctor

29332e0

Merge branch 'desktop_doctor' of github.com:jonahwilliams/flutter int…

04b7de8

…o desktop_doctor

fix cirrus config

d7f1c2b

Jonah Williams added 2 commits June 19, 2019 14:02

Update .cirrus.yml

26bc5ba

add it back

709e27e

jonahwilliams merged commit f221ad1 into flutter:master Jun 19, 2019

jonahwilliams pushed a commit that referenced this pull request Jun 19, 2019

Revert "Add basic desktop linux checks (#31873)"

472b1cf

This reverts commit f221ad1.

jonahwilliams mentioned this pull request Jun 19, 2019

Revert "Add basic desktop linux checks" #34753

Merged

jonahwilliams pushed a commit that referenced this pull request Jun 19, 2019

Revert "Add basic desktop linux checks (#31873)" (#34753)

830fb38

This reverts commit f221ad1.

jonahwilliams mentioned this pull request Jun 20, 2019

Add linux doctor implementation #34755

Merged

johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019

Add basic desktop linux checks (flutter#31873)

da0a814

johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019

Revert "Add basic desktop linux checks (flutter#31873)" (flutter#34753)

10de59c

This reverts commit f221ad1.

github-actions Bot locked as resolved and limited conversation to collaborators Aug 6, 2021

Conversation

jonahwilliams commented Apr 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented May 21, 2019

Uh oh!

stuartmorgan-g commented Jun 12, 2019

Uh oh!

jonahwilliams commented Jun 17, 2019

Uh oh!

jonahwilliams commented Jun 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonahwilliams commented Jun 19, 2019

Uh oh!

dnfield commented Jun 19, 2019

Uh oh!

jonahwilliams commented Jun 19, 2019

Uh oh!

dnfield commented Jun 19, 2019

Uh oh!

jonahwilliams commented Jun 19, 2019

Uh oh!

tvolkert commented Jun 19, 2019

Uh oh!

jonahwilliams commented Jun 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jonahwilliams commented Apr 30, 2019 •

edited

Loading