diff --git a/packages/flutter_tools/bin/fuchsia_attach.dart b/packages/flutter_tools/bin/fuchsia_attach.dart index 59e00fe3030..09582d60f4e 100644 --- a/packages/flutter_tools/bin/fuchsia_attach.dart +++ b/packages/flutter_tools/bin/fuchsia_attach.dart @@ -98,7 +98,7 @@ Future main(List args) async { Cache.disableLocking(); // ignore: invalid_use_of_visible_for_testing_member await runner.run( command, - [ + () => [ _FuchsiaAttachCommand(), _FuchsiaDoctorCommand(), // If attach fails the tool will attempt to run doctor. ], diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart index 08905a7f1fc..bf994bea3bb 100644 --- a/packages/flutter_tools/lib/executable.dart +++ b/packages/flutter_tools/lib/executable.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'runner.dart' as runner; import 'src/base/context.dart'; +import 'src/base/logger.dart'; import 'src/base/template.dart'; // The build_runner code generation is provided here to make it easier to // avoid introducing the dependency into google3. Not all build* packages @@ -64,8 +65,9 @@ Future main(List args) async { (args.isNotEmpty && args.first == 'help') || (args.length == 1 && verbose); final bool muteCommandLogging = help || doctor; final bool verboseHelp = help && verbose; + final bool daemon = args.contains('daemon'); - await runner.run(args, [ + await runner.run(args, () => [ AnalyzeCommand( verboseHelp: verboseHelp, fileSystem: globals.fs, @@ -122,5 +124,14 @@ Future main(List args) async { WebRunnerFactory: () => DwdsWebRunnerFactory(), // The mustache dependency is different in google3 TemplateRenderer: () => const MustacheTemplateRenderer(), + if (daemon) + Logger: () => NotifyingLogger(verbose: verbose) + else if (verbose) + Logger: () => VerboseLogger(StdoutLogger( + timeoutConfiguration: timeoutConfiguration, + stdio: globals.stdio, + terminal: globals.terminal, + outputPreferences: globals.outputPreferences, + )) }); } diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index cb7f957edd7..d40eda237f6 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart @@ -23,9 +23,12 @@ import 'src/runner/flutter_command.dart'; import 'src/runner/flutter_command_runner.dart'; /// Runs the Flutter tool with support for the specified list of [commands]. +/// +/// [commands] must be either `List` or `List Function()`. +// TODO(jonahwilliams): update command type once g3 has rolled. Future run( List args, - List commands, { + dynamic commands, { bool muteCommandLogging = false, bool verbose = false, bool verboseHelp = false, @@ -39,12 +42,17 @@ Future run( args = List.of(args); args.removeWhere((String option) => option == '-v' || option == '--verbose'); } - - final FlutterCommandRunner runner = FlutterCommandRunner(verboseHelp: verboseHelp); - commands.forEach(runner.addCommand); + List Function() commandGenerator; + if (commands is List) { + commandGenerator = () => commands; + } else { + commandGenerator = commands as List Function(); + } return runInContext(() async { reportCrashes ??= !await globals.isRunningOnBot; + final FlutterCommandRunner runner = FlutterCommandRunner(verboseHelp: verboseHelp); + commandGenerator().forEach(runner.addCommand); // Initialize the system locale. final String systemLocale = await intl_standalone.findSystemLocale(); diff --git a/packages/flutter_tools/lib/src/commands/analyze.dart b/packages/flutter_tools/lib/src/commands/analyze.dart index 969a2c5fc82..36f314aea5a 100644 --- a/packages/flutter_tools/lib/src/commands/analyze.dart +++ b/packages/flutter_tools/lib/src/commands/analyze.dart @@ -11,7 +11,6 @@ import '../base/file_system.dart'; import '../base/logger.dart'; import '../base/platform.dart'; import '../base/terminal.dart'; -import '../globals.dart' as globals; import '../runner/flutter_command.dart'; import 'analyze_continuously.dart'; import 'analyze_once.dart'; @@ -113,9 +112,7 @@ class AnalyzeCommand extends FlutterCommand { runner.getRepoRoots(), runner.getRepoPackages(), fileSystem: _fileSystem, - // TODO(jonahwilliams): determine a better way to inject the logger, - // since it is constructed on-demand. - logger: _logger ?? globals.logger, + logger: _logger, platform: _platform, processManager: _processManager, terminal: _terminal, @@ -128,9 +125,7 @@ class AnalyzeCommand extends FlutterCommand { runner.getRepoPackages(), workingDirectory: workingDirectory, fileSystem: _fileSystem, - // TODO(jonahwilliams): determine a better way to inject the logger, - // since it is constructed on-demand. - logger: _logger ?? globals.logger, + logger: _logger, platform: _platform, processManager: _processManager, terminal: _terminal, diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index e6d201ef634..33eacafb871 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -200,7 +200,7 @@ class AttachCommand extends FlutterCommand { ? Daemon( stdinCommandStream, stdoutCommandResponse, - notifyingLogger: NotifyingLogger(), + notifyingLogger: NotifyingLogger(verbose: globals.logger.isVerbose), logToStdout: true, ) : null; diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 6147d961d20..1650d87070e 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -52,27 +52,16 @@ class DaemonCommand extends FlutterCommand { Future runCommand() async { globals.printStatus('Starting device daemon...'); isRunningFromDaemon = true; - - final NotifyingLogger notifyingLogger = NotifyingLogger(); - Cache.releaseLockEarly(); - await context.run( - body: () async { - final Daemon daemon = Daemon( - stdinCommandStream, stdoutCommandResponse, - notifyingLogger: notifyingLogger, - ); - - final int code = await daemon.onExit; - if (code != 0) { - throwToolExit('Daemon exited with non-zero exit code: $code', exitCode: code); - } - }, - overrides: { - Logger: () => notifyingLogger, - }, + final Daemon daemon = Daemon( + stdinCommandStream, stdoutCommandResponse, + notifyingLogger: globals.logger as NotifyingLogger, ); + final int code = await daemon.onExit; + if (code != 0) { + throwToolExit('Daemon exited with non-zero exit code: $code', exitCode: code); + } return FlutterCommandResult.success(); } } @@ -927,7 +916,22 @@ dynamic _toJsonable(dynamic obj) { } class NotifyingLogger extends Logger { - final StreamController _messageController = StreamController.broadcast(); + NotifyingLogger({ @required this.verbose }) { + _messageController = StreamController.broadcast( + onListen: _onListen, + ); + } + + final bool verbose; + final List messageBuffer = []; + StreamController _messageController; + + void _onListen() { + if (messageBuffer.isNotEmpty) { + messageBuffer.forEach(_messageController.add); + messageBuffer.clear(); + } + } Stream get onMessage => _messageController.stream; @@ -941,7 +945,7 @@ class NotifyingLogger extends Logger { int hangingIndent, bool wrap, }) { - _messageController.add(LogMessage('error', message, stackTrace)); + _sendMessage(LogMessage('error', message, stackTrace)); } @override @@ -954,12 +958,15 @@ class NotifyingLogger extends Logger { int hangingIndent, bool wrap, }) { - _messageController.add(LogMessage('status', message)); + _sendMessage(LogMessage('status', message)); } @override void printTrace(String message) { - // This is a lot of traffic to send over the wire. + if (!verbose) { + return; + } + _sendMessage(LogMessage('trace', message)); } @override @@ -979,6 +986,13 @@ class NotifyingLogger extends Logger { ); } + void _sendMessage(LogMessage logMessage) { + if (_messageController.hasListener) { + return _messageController.add(logMessage); + } + messageBuffer.add(logMessage); + } + void dispose() { _messageController.close(); } diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index a5125a8f6f1..2458a73d626 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -407,7 +407,7 @@ class RunCommand extends RunCommandBase { final Daemon daemon = Daemon( stdinCommandStream, stdoutCommandResponse, - notifyingLogger: NotifyingLogger(), + notifyingLogger: NotifyingLogger(verbose: globals.logger.isVerbose), logToStdout: true, ); AppInstance app; diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 59f5b25f9f4..8612b5bf55a 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -103,6 +103,7 @@ class DeviceManager { fileSystem: globals.fs, platform: globals.platform, processManager: globals.processManager, + logger: globals.logger, ), ]); diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart index 6730f7accdc..21be933e103 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart @@ -15,7 +15,6 @@ import '../artifacts.dart'; import '../base/common.dart'; import '../base/context.dart'; import '../base/file_system.dart'; -import '../base/logger.dart'; import '../base/terminal.dart'; import '../base/user_messages.dart'; import '../base/utils.dart'; @@ -236,12 +235,6 @@ class FlutterCommandRunner extends CommandRunner { Future runCommand(ArgResults topLevelResults) async { final Map contextOverrides = {}; - // Check for verbose. - if (topLevelResults['verbose'] as bool) { - // Override the logger. - contextOverrides[Logger] = VerboseLogger(globals.logger); - } - // Don't set wrapColumns unless the user said to: if it's set, then all // wrapping will occur at this width explicitly, and won't adapt if the // terminal size changes during a run. diff --git a/packages/flutter_tools/lib/src/web/web_device.dart b/packages/flutter_tools/lib/src/web/web_device.dart index 1042ee6a19c..41df82c3fc0 100644 --- a/packages/flutter_tools/lib/src/web/web_device.dart +++ b/packages/flutter_tools/lib/src/web/web_device.dart @@ -14,7 +14,6 @@ import '../base/platform.dart'; import '../build_info.dart'; import '../device.dart'; import '../features.dart'; -import '../globals.dart' as globals; import '../project.dart'; import 'chrome.dart'; @@ -36,7 +35,7 @@ abstract class ChromiumDevice extends Device { @required String name, @required this.chromeLauncher, @required FileSystem fileSystem, - Logger logger, + @required Logger logger, }) : _fileSystem = fileSystem, _logger = logger, super( @@ -49,7 +48,7 @@ abstract class ChromiumDevice extends Device { final ChromiumLauncher chromeLauncher; final FileSystem _fileSystem; - Logger _logger; + final Logger _logger; /// The active chrome instance. Chromium _chrome; @@ -115,7 +114,6 @@ abstract class ChromiumDevice extends Device { bool prebuiltApplication = false, bool ipv6 = false, }) async { - _logger ??= globals.logger; // See [ResidentWebRunner.run] in flutter_tools/lib/src/resident_web_runner.dart // for the web initialization and server logic. final String url = platformArgs['uri'] as String; @@ -241,10 +239,7 @@ class MicrosoftEdgeDevice extends ChromiumDevice { class WebDevices extends PollingDeviceDiscovery { WebDevices({ @required FileSystem fileSystem, - // TODO(jonahwilliams): the logger is overriden by the daemon command - // to support IDE integration. Accessing the global logger too early - // will grab the old stdout logger. - Logger logger, + @required Logger logger, @required Platform platform, @required ProcessManager processManager, @required FeatureFlags featureFlags, @@ -307,7 +302,7 @@ String parseVersionForWindows(String input) { /// A special device type to allow serving for arbitrary browsers. class WebServerDevice extends Device { WebServerDevice({ - Logger logger, + @required Logger logger, }) : _logger = logger, super( 'web-server', @@ -316,7 +311,7 @@ class WebServerDevice extends Device { ephemeral: false, ); - Logger _logger; + final Logger _logger; @override void clearLogs() { } @@ -375,7 +370,6 @@ class WebServerDevice extends Device { bool prebuiltApplication = false, bool ipv6 = false, }) async { - _logger ??= globals.logger; final String url = platformArgs['uri'] as String; if (debuggingOptions.startPaused) { _logger.printStatus('Waiting for connection from Dart debug extension at $url', emphasis: true); diff --git a/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart index 7b377978732..fcab172306e 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart @@ -24,7 +24,7 @@ void main() { group('daemon', () { setUp(() { - notifyingLogger = NotifyingLogger(); + notifyingLogger = NotifyingLogger(verbose: false); }); tearDown(() { @@ -298,6 +298,42 @@ void main() { }); }); + testUsingContext('notifyingLogger outputs trace messages in verbose mode', () async { + final NotifyingLogger logger = NotifyingLogger(verbose: true); + + final Future messageResult = logger.onMessage.first; + logger.printTrace('test'); + + final LogMessage message = await messageResult; + + expect(message.level, 'trace'); + expect(message.message, 'test'); + }); + + testUsingContext('notifyingLogger ignores trace messages in non-verbose mode', () async { + final NotifyingLogger logger = NotifyingLogger(verbose: false); + + final Future messageResult = logger.onMessage.first; + logger.printTrace('test'); + logger.printStatus('hello'); + + final LogMessage message = await messageResult; + + expect(message.level, 'status'); + expect(message.message, 'hello'); + }); + + testUsingContext('notifyingLogger buffers messages sent before a subscription', () async { + final NotifyingLogger logger = NotifyingLogger(verbose: false); + + logger.printStatus('hello'); + + final LogMessage message = await logger.onMessage.first; + + expect(message.level, 'status'); + expect(message.message, 'hello'); + }); + group('daemon serialization', () { test('OperationResult', () { expect( diff --git a/packages/flutter_tools/test/general.shard/runner/runner_test.dart b/packages/flutter_tools/test/general.shard/runner/runner_test.dart index cffab93fc63..6c0b536bb90 100644 --- a/packages/flutter_tools/test/general.shard/runner/runner_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/runner_test.dart @@ -44,7 +44,7 @@ void main() { () { unawaited(runner.run( ['test'], - [ + () => [ CrashingFlutterCommand(), ], // This flutterVersion disables crash reporting. @@ -86,7 +86,7 @@ void main() { () { unawaited(runner.run( ['test'], - [ + () => [ CrashingFlutterCommand(), ], // This flutterVersion disables crash reporting. diff --git a/packages/flutter_tools/test/general.shard/web/devices_test.dart b/packages/flutter_tools/test/general.shard/web/devices_test.dart index 2de45584b68..ffbe076ac79 100644 --- a/packages/flutter_tools/test/general.shard/web/devices_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devices_test.dart @@ -5,7 +5,6 @@ import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; -import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/web/chrome.dart'; import 'package:flutter_tools/src/web/web_device.dart'; @@ -16,37 +15,6 @@ import '../../src/context.dart'; import '../../src/testbed.dart'; void main() { - testUsingContext('Grabs context logger if no constructor logger is provided', () async { - final WebDevices webDevices = WebDevices( - featureFlags: TestFeatureFlags(isWebEnabled: true), - fileSystem: MemoryFileSystem.test(), - platform: FakePlatform( - operatingSystem: 'linux', - environment: {} - ), - processManager: FakeProcessManager.any(), - ); - - final List devices = await webDevices.pollingGetDevices(); - final WebServerDevice serverDevice = devices.firstWhere((Device device) => device is WebServerDevice) - as WebServerDevice; - - await serverDevice.startApp( - null, - debuggingOptions: DebuggingOptions.enabled( - BuildInfo.debug, - startPaused: true, - ), - platformArgs: { - 'uri': 'foo', - } - ); - - // Verify that the injected testLogger is used. - expect(testLogger.statusText, contains( - 'Waiting for connection from Dart debug extension at foo' - )); - }); testWithoutContext('No web devices listed if feature is disabled', () async { final WebDevices webDevices = WebDevices( featureFlags: TestFeatureFlags(isWebEnabled: false), diff --git a/packages/flutter_tools/test/integration.shard/daemon_mode_test.dart b/packages/flutter_tools/test/integration.shard/daemon_mode_test.dart index fbf41637ce2..686a4127e3d 100644 --- a/packages/flutter_tools/test/integration.shard/daemon_mode_test.dart +++ b/packages/flutter_tools/test/integration.shard/daemon_mode_test.dart @@ -47,7 +47,7 @@ void main() { 'id': 1, 'method': 'device.enable', })}]'); - response = await stream.first; + response = await stream.firstWhere((Map json) => json['id'] == 1); expect(response['id'], 1); expect(response['error'], isNull); diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index 570694c693e..65375d640df 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart @@ -4,7 +4,9 @@ import 'dart:async'; +import 'package:args/args.dart'; import 'package:args/command_runner.dart'; +import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/convert.dart'; import 'package:vm_service/vm_service.dart' as vm_service; @@ -78,7 +80,7 @@ String getFlutterRoot() { } CommandRunner createTestCommandRunner([ FlutterCommand command ]) { - final FlutterCommandRunner runner = FlutterCommandRunner(); + final FlutterCommandRunner runner = TestFlutterCommandRunner(); if (command != null) { runner.addCommand(command); } @@ -352,3 +354,20 @@ class FakeVmServiceStreamResponse implements VmServiceExpectation { @override bool get isRequest => false; } + +class TestFlutterCommandRunner extends FlutterCommandRunner { + @override + Future runCommand(ArgResults topLevelResults) async { + final Logger topLevelLogger = globals.logger; + final Map contextOverrides = { + if (topLevelResults['verbose'] as bool) + Logger: VerboseLogger(topLevelLogger), + }; + return context.run( + overrides: contextOverrides.map((Type type, dynamic value) { + return MapEntry(type, () => value); + }), + body: () => super.runCommand(topLevelResults), + ); + } +}