[flutter_tools] remove globals from compilers by jonahwilliams · Pull Request #59184 · flutter/flutter

jonahwilliams · 2020-06-10T18:51:50Z

Description

Refactors KernelCompiler and ResidentCompiler to no longer use globals (except as a fallback for g3 migration). Improves the compilation error when running flutter test on a package without a flutter_test dependency.

Fixes #59510

jonahwilliams · 2020-06-10T18:52:37Z

 }
+
+
+class _TestArtifacts implements Artifacts {


I got tired of mocking Artifacts when I don't actually care about the contents. This should trivially generate unique-ish artifact paths for testing

jonahwilliams · 2020-06-10T18:53:24Z

    this.pid = 1,
    Future<int> exitCode,
-    Stream<List<int>> stdin,
+    IOSink stdin,


this type was wrong and the case is bogus

jonahwilliams · 2020-06-10T18:53:58Z

        // Override the filesystem scheme so that the frontend_server can find
        // the generated entrypoint code.
        fileSystemScheme: 'org-dartlang-app',
-        compilerMessageConsumer:


@annagrin I need to remove this because it prevents regular compiler errors from showing up on the web

I need to understand this better. I added this to communicate compiler errors to the debugger during evaluation, so something might break there...

This is only printing trace messages, how is the debugger using printed messages?

For expression evaluation, the debugger does not need messages printed to the console, but instead is handing them specially, depending on the context (watch window, hover over, expression evaluation box, conditional compilation all need their own error handling). Not sure how the debugger is getting the error messages though, will find it out and update.

I tried the change and figured out that if we suppress the compilation messages in compile.dart :

Future<CompilerOutput> _compileExpressionToJs(_CompileExpressionToJsRequest request) async { _stdoutHandler.reset(suppressCompilerMessages: **false**, expectSources: false);

then we get the desired effect - the error messages are still propagating to the debugger but the printing is gone.

Ahh, awesome - I will make that change!

Figured out how debugger gets compilation errors:

flutter/packages/flutter_tools/lib/src/build_runner/devfs_web.dart

Line 85 in 96711b0

return ExpressionCompilationResult(

So as long as the generator writes them to the file, the debugger should get them:

https://github.com/dart-lang/sdk/blob/ecc7a2110667284d792946775f471f4a7f409afb/pkg/frontend_server/lib/frontend_server.dart#L964

And the errors table is filled by:

https://github.com/dart-lang/sdk/blob/ecc7a2110667284d792946775f471f4a7f409afb/pkg/frontend_server/lib/frontend_server.dart#L360

This a bit too complicated, may need revisiting and simplification...

…ms/flutter into resident_runner_dependencies

jonahwilliams · 2020-06-11T16:14:51Z

    }
  }
-
-  void _reportCompilerMessage(String message, {bool emphasis, TerminalColor color}) {


Rather than printing this message after a bunch of compilation failures, the new test compiler checks for a dependency before compiling and exits early. This gives a much clearer instruction on how to resolve the error too

jonahwilliams · 2020-06-11T17:09:59Z

 <<stderr>>
 <<skip until matching line>>
-Failed to load test harness\. +Are you missing a dependency on flutter_test\?
+Error: cannot run without a dependency on "package:flutter_test". Ensure the following lines are present in your pubspec.yaml:


By checking the package config we know definitely there is a missing dependency. Added instructions on how to fix

jmagman

nit: CompilerMessageConsumer typedef is unused, remove.
LGTM

…ner_dependencies

zanderso

lgtm w/ question.

zanderso · 2020-06-16T15:55:29Z

@@ -68,10 +76,6 @@ void main() {
    verifyNoMoreInteractions(mockFrontendServerStdIn);


Can any of these use testWithoutContext?

zanderso · 2020-06-16T15:55:49Z

      verify(residentCompiler.shutdown()).called(1);
    }));
+
+    test('Reports an error when there is no dependency on flutter_test', () => testbed.run(() async {


not until these tests are re-written.

…ner_dependencies

jonahwilliams · 2020-06-18T21:09:15Z

Fixes #59510

Refactors KernelCompiler and ResidentCompiler to no longer use globals (except as a fallback for g3 migration). Improves the compilation error when running flutter test on a package without a flutter_test dependency. Updates machine mode to output trace text to stderr

jonahwilliams and others added 2 commits June 10, 2020 11:50

[flutter_tools] remove globals from compilers

a918036

Update artifacts.dart

fed135b

jonahwilliams commented Jun 10, 2020

View reviewed changes

goderbauer added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 10, 2020

jonahwilliams added 2 commits June 11, 2020 09:13

print error message before attempting to compile

902d857

Merge branch 'resident_runner_dependencies' of github.com:jonahwillia…

e47066e

…ms/flutter into resident_runner_dependencies

googlebot added the cla: yes label Jun 11, 2020

jonahwilliams commented Jun 11, 2020

View reviewed changes

jonahwilliams and others added 2 commits June 11, 2020 09:49

fix tests

9bb5555

Delete flutter_01.sksl.json

e6e7ada

jonahwilliams commented Jun 11, 2020

View reviewed changes

jonahwilliams requested review from jmagman and zanderso June 11, 2020 18:38

Update artifacts.dart

6ac04fd

jonahwilliams mentioned this pull request Jun 15, 2020

Flutter tool needs to perform additional validation on local engine paths for windows #59438

Open

jmagman approved these changes Jun 15, 2020

View reviewed changes

jonahwilliams added 2 commits June 16, 2020 07:31

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

09045d4

…ner_dependencies

remove CompilerMessageConsumer

fcd6174

zanderso approved these changes Jun 16, 2020

View reviewed changes

jonahwilliams and others added 5 commits June 16, 2020 09:25

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

da8fd4a

…ner_dependencies

remove testWithContext

2570a60

silence errors for JS eval

70f04ab

move messages to trace text

39b06f7

output verbose daemon logs to stderr

a56e95f

jonahwilliams commented Jun 17, 2020

View reviewed changes

Comment thread packages/flutter_tools/lib/src/commands/daemon.dart

jonahwilliams requested a review from annagrin June 17, 2020 15:29

annagrin approved these changes Jun 17, 2020

View reviewed changes

jonahwilliams merged commit 447e3d3 into flutter:master Jun 18, 2020

jonahwilliams deleted the resident_runner_dependencies branch June 18, 2020 17:33

fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Jun 18, 2020

jonahwilliams mentioned this pull request Jun 18, 2020

Dart SDK error hidden as warning by flutter tool #59510

Closed

github-actions Bot locked as resolved and limited conversation to collaborators Jul 30, 2021

		@@ -68,10 +76,6 @@ void main() {
		verifyNoMoreInteractions(mockFrontendServerStdIn);

Conversation

jonahwilliams commented Jun 10, 2020 • 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!

annagrin Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

zanderso 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonahwilliams commented Jun 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jonahwilliams commented Jun 10, 2020 •

edited

Loading

annagrin Jun 16, 2020 •

edited

Loading