[et] Adds a logger (flutter/engine#50693)

engine/src/flutter/tools/engine_tool/lib/main.dart

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:ffi' as ffi show Abi;
-import 'dart:io' as io show Directory, exitCode, stderr, stdout;
+import 'dart:io' as io show Directory, exitCode, stderr;
 
 import 'package:engine_build_configs/engine_build_configs.dart';
 import 'package:engine_repo_tools/engine_repo_tools.dart';
@@ -13,6 +13,7 @@ import 'package:process_runner/process_runner.dart';
 
 import 'src/commands/command_runner.dart';
 import 'src/environment.dart';
+import 'src/logger.dart';
 
 void main(List<String> args) async {
   // Find the engine repo.
@@ -55,8 +56,7 @@ void main(List<String> args) async {
       engine: engine,
       platform: const LocalPlatform(),
       processRunner: ProcessRunner(),
-      stderr: io.stderr,
-      stdout: io.stdout,
+      logger: Logger(),
     ),
     configs: configs,
   );

engine/src/flutter/tools/engine_tool/lib/src/commands/command_runner.dart

@@ -43,10 +43,10 @@ final class ToolCommandRunner extends CommandRunner<int> {
     try{
       return await runCommand(parse(args)) ?? 1;
     } on FormatException catch (e) {
-      environment.stderr.writeln(e);
+      environment.logger.error(e);
       return 1;
     } on UsageException catch (e) {
-      environment.stderr.writeln(e);
+      environment.logger.error(e);
       return 1;
     }
   }

engine/src/flutter/tools/engine_tool/lib/src/commands/query_command.dart

@@ -63,12 +63,6 @@ final class QueryCommand extends CommandBase {
   @override
   String get description => 'Provides information about build configurations '
                             'and tests.';
-
-  @override
-  Future<int> run() async {
-    environment.stdout.write(usage);
-    return 0;
-  }
 }
 
 /// The 'query builds' command.
@@ -97,10 +91,10 @@ final class QueryBuildsCommand extends CommandBase {
     final String? builderName = parent!.argResults![_builderFlag] as String?;
     final bool verbose = parent!.argResults![_verboseFlag] as bool;
     if (!verbose) {
-      environment.stdout.writeln(
+      environment.logger.status(
         'Add --verbose to see detailed information about each builder',
       );
-      environment.stdout.writeln();
+      environment.logger.status('');
     }
     for (final String key in configs.keys) {
       if (builderName != null && key != builderName) {
@@ -112,23 +106,23 @@ final class QueryBuildsCommand extends CommandBase {
         continue;
       }
 
-      environment.stdout.writeln('"$key" builder:');
+      environment.logger.status('"$key" builder:');
       for (final GlobalBuild build in config.builds) {
         if (!build.canRunOn(environment.platform) && !all) {
           continue;
         }
-        environment.stdout.writeln('  "${build.name}" config');
+        environment.logger.status('"${build.name}" config', indent: 3);
         if (!verbose) {
           continue;
         }
-        environment.stdout.writeln('    gn flags:');
+        environment.logger.status('gn flags:', indent: 6);
         for (final String flag in build.gn) {
-          environment.stdout.writeln('      $flag');
+          environment.logger.status(flag, indent: 9);
         }
         if (build.ninja.targets.isNotEmpty) {
-          environment.stdout.writeln('    ninja targets:');
+          environment.logger.status('ninja targets:', indent: 6);
           for (final String target in build.ninja.targets) {
-            environment.stdout.writeln('      $target');
+            environment.logger.status(target, indent: 9);
           }
         }
       }

engine/src/flutter/tools/engine_tool/lib/src/environment.dart

@@ -8,6 +8,8 @@ import 'package:engine_repo_tools/engine_repo_tools.dart';
 import 'package:platform/platform.dart';
 import 'package:process_runner/process_runner.dart';
 
+import 'logger.dart';
+
 /// This class encapsulates information about the host system.
 ///
 /// Rather than being written directly against `dart:io`, implementations in the
@@ -19,10 +21,9 @@ final class Environment {
   Environment({
     required this.abi,
     required this.engine,
+    required this.logger,
     required this.platform,
     required this.processRunner,
-    required this.stderr,
-    required this.stdout,
   });
 
   /// The host OS and architecture that the tool is running on.
@@ -31,17 +32,12 @@ final class Environment {
   /// Information about paths in the engine repo.
   final Engine engine;
 
+  /// Where log messages are written.
+  final Logger logger;
+
   /// More detailed information about the host platform.
   final Platform platform;
 
   /// Facility for commands to run subprocesses.
   final ProcessRunner processRunner;
-
-  // TODO(zanderso): Replace stderr and stdout with a real logger.
-
-  /// A sink for error messages from commands.
-  final StringSink stderr;
-
-  /// A sink for non-error messages from commands.
-  final StringSink stdout;
 }

engine/src/flutter/tools/engine_tool/lib/src/logger.dart

@@ -0,0 +1,135 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async' show runZoned;
+import 'dart:io' as io show
+  IOSink,
+  stderr,
+  stdout;
+
+import 'package:logging/logging.dart' as log;
+import 'package:meta/meta.dart';
+
+// This is where a flutter_tool style progress spinner, color output,
+// ascii art, terminal control for clearing lines or the whole screen, etc.
+// can go. We can just add more methods to Logger using the flutter_tool's
+// Logger as a guide:
+//
+// https://github.com/flutter/flutter/blob/c530276f7806c77da2541c518a0e103c9bb44f10/packages/flutter_tools/lib/src/base/logger.dart#L422
+
+/// A simplified wrapper around the [Logger] from package:logging.
+///
+/// The default log level is [Logger.status]. A --quiet flag might change it to
+/// [Logger.warning] or [Logger.error]. A --verbose flag might change it to
+/// [Logger.info].
+///
+/// Log messages at [Logger.warning] and higher will be written to stderr, and
+/// to stdout otherwise. [Logger.test] records all log messages to a buffer,
+/// which can be inspected by unit tetss.
+class Logger {
+  /// Constructs a logger for use in the tool.
+  Logger() : _logger = log.Logger.detached('et') {
+    _logger.level = statusLevel;
+    _logger.onRecord.listen(_handler);
+    _setupIoSink(io.stderr);
+    _setupIoSink(io.stdout);
+  }
+
+  /// A logger for tests.
+  @visibleForTesting
+  Logger.test() : _logger = log.Logger.detached('et') {
+    _logger.level = statusLevel;
+    _logger.onRecord.listen((log.LogRecord r) => _testLogs.add(r));
+  }
+
+  /// The logging level for error messages. These go to stderr.
+  static const log.Level errorLevel = log.Level('ERROR', 100);
+
+  /// The logging level for warning messages. These go to stderr.
+  static const log.Level warningLevel = log.Level('WARNING', 75);
+
+  /// The logging level for normal status messages. These go to stdout.
+  static const log.Level statusLevel = log.Level('STATUS', 25);
+
+  /// The logging level for verbose informational messages. These go to stdout.
+  static const log.Level infoLevel = log.Level('INFO', 10);
+
+  static void _handler(log.LogRecord r) {
+    final io.IOSink sink = r.level >= warningLevel ? io.stderr : io.stdout;
+    final String prefix = r.level >= warningLevel
+      ? '[${r.time}] ${r.level}: '
+      : '';
+    _ioSinkWrite(sink, '$prefix${r.message}');
+  }
+
+  // Status of the global io.stderr and io.stdout is shared across all
+  // Logger instances.
+  static bool _stdioDone = false;
+
+  // stdout and stderr might already be closed, and when not already closed,
+  // writing can still fail by throwing either a sync or async exception.
+  // This function handles all three cases.
+  static void _ioSinkWrite(io.IOSink sink, String message) {
+    if (_stdioDone) {
+      return;
+    }
+    runZoned<void>(() {
+      try {
+        sink.writeln(message);
+      } catch (_) { // ignore: avoid_catches_without_on_clauses
+        _stdioDone = true;
+      }
+    }, onError: (Object e, StackTrace s) {
+      _stdioDone = true;
+    });
+  }
+
+  static void _setupIoSink(io.IOSink sink) {
+    sink.done.then(
+      (void _) { _stdioDone = true; },
+      onError: (Object err, StackTrace st) { _stdioDone = true; },
+    );
+  }
+
+  final log.Logger _logger;
+  final List<log.LogRecord> _testLogs = <log.LogRecord>[];
+
+  /// Get the current logging level.
+  log.Level get level => _logger.level;
+
+  /// Set the current logging level.
+  set level(log.Level l) {
+    _logger.level = l;
+  }
+
+  /// Record a log message at level [Logger.error].
+  void error(Object? message, {int indent = 0}) {
+    _emitLog(errorLevel, message, indent);
+  }
+
+  /// Record a log message at level [Logger.warning].
+  void warning(Object? message, {int indent = 0}) {
+    _emitLog(warningLevel, message, indent);
+  }
+
+  /// Record a log message at level [Logger.warning].
+  void status(Object? message, {int indent = 0}) {
+    _emitLog(statusLevel, message, indent);
+  }
+
+  /// Record a log message at level [Logger.info].
+  void info(Object? message, {int indent = 0}) {
+    _emitLog(infoLevel, message, indent);
+  }
+
+  void _emitLog(log.Level level, Object? message, int indent) {
+    final String m = '${' ' * indent}$message';
+    _logger.log(level, m);
+  }
+
+  /// In a [Logger] constructed by [Logger.test], this list will contain all of
+  /// the [LogRecord]s emitted by the test.
+  @visibleForTesting
+  List<log.LogRecord> get testLogs => _testLogs;
+}

engine/src/flutter/tools/engine_tool/pubspec.yaml

@@ -23,6 +23,7 @@ dependencies:
   engine_repo_tools:
     path: ../pkg/engine_repo_tools
   file: any
+  logging: any
   meta: any
   path: any
   platform: any
@@ -51,6 +52,8 @@ dependency_overrides:
     path: ../../../third_party/dart/third_party/pkg/file/packages/file
   litetest:
     path: ../../testing/litetest
+  logging:
+    path: ../../../third_party/dart/third_party/pkg/logging
   meta:
     path: ../../../third_party/dart/pkg/meta
   path:

engine/src/flutter/tools/engine_tool/test/logger_test.dart

@@ -0,0 +1,56 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:engine_tool/src/logger.dart';
+import 'package:litetest/litetest.dart';
+import 'package:logging/logging.dart' as log;
+
+void main() {
+  List<String> stringsFromLogs(List<log.LogRecord> logs) {
+    return logs.map((log.LogRecord r) => r.message).toList();
+  }
+
+  test('Setting the level works', () {
+    final Logger logger = Logger.test();
+    logger.level = Logger.infoLevel;
+    expect(logger.level, equals(Logger.infoLevel));
+  });
+
+  test('error messages are recorded at the default log level', () {
+    final Logger logger = Logger.test();
+    logger.error('Error');
+    expect(stringsFromLogs(logger.testLogs), equals(<String>['Error']));
+  });
+
+  test('warning messages are recorded at the default log level', () {
+    final Logger logger = Logger.test();
+    logger.warning('Warning');
+    expect(stringsFromLogs(logger.testLogs), equals(<String>['Warning']));
+  });
+
+  test('status messages are recorded at the default log level', () {
+    final Logger logger = Logger.test();
+    logger.status('Status');
+    expect(stringsFromLogs(logger.testLogs), equals(<String>['Status']));
+  });
+
+  test('info messages are not recorded at the default log level', () {
+    final Logger logger = Logger.test();
+    logger.info('info');
+    expect(stringsFromLogs(logger.testLogs), equals(<String>[]));
+  });
+
+  test('info messages are recorded at the infoLevel log level', () {
+    final Logger logger = Logger.test();
+    logger.level = Logger.infoLevel;
+    logger.info('info');
+    expect(stringsFromLogs(logger.testLogs), equals(<String>['info']));
+  });
+
+  test('indent indents the message', () {
+    final Logger logger = Logger.test();
+    logger.status('Status', indent: 1);
+    expect(stringsFromLogs(logger.testLogs), equals(<String>[' Status']));
+  });
+}

engine/src/flutter/tools/engine_tool/test/query_command_test.dart

@@ -10,7 +10,9 @@ import 'package:engine_build_configs/engine_build_configs.dart';
 import 'package:engine_repo_tools/engine_repo_tools.dart';
 import 'package:engine_tool/src/commands/command_runner.dart';
 import 'package:engine_tool/src/environment.dart';
+import 'package:engine_tool/src/logger.dart';
 import 'package:litetest/litetest.dart';
+import 'package:logging/logging.dart' as log;
 import 'package:platform/platform.dart';
 import 'package:process_fakes/process_fakes.dart';
 import 'package:process_runner/process_runner.dart';
@@ -49,7 +51,7 @@ void main() {
     'win_test_config': winTestConfig,
   };
 
-  Environment linuxEnv(StringBuffer stderr, StringBuffer stdout) {
+  Environment linuxEnv(Logger logger) {
     return Environment(
       abi: ffi.Abi.linuxX64,
       engine: engine,
@@ -57,15 +59,17 @@ void main() {
       processRunner: ProcessRunner(
         processManager: FakeProcessManager(),
       ),
-      stderr: stderr,
-      stdout: stdout,
+      logger: logger,
     );
   }
 
+  List<String> stringsFromLogs(List<log.LogRecord> logs) {
+    return logs.map((log.LogRecord r) => r.message).toList();
+  }
+
   test('query command returns builds for the host platform.', () async {
-    final StringBuffer stderr = StringBuffer();
-    final StringBuffer stdout = StringBuffer();
-    final Environment env = linuxEnv(stderr, stdout);
+    final Logger logger = Logger.test();
+    final Environment env = linuxEnv(logger);
     final ToolCommandRunner runner = ToolCommandRunner(
       environment: env,
       configs: configs,
@@ -75,22 +79,21 @@ void main() {
     ]);
     expect(result, equals(0));
     expect(
-      stdout.toString().trim().split('\n'),
+      stringsFromLogs(logger.testLogs),
       equals(<String>[
         'Add --verbose to see detailed information about each builder',
         '',
         '"linux_test_config" builder:',
-        '  "build_name" config',
+        '   "build_name" config',
         '"linux_test_config2" builder:',
-        '  "build_name" config',
+        '   "build_name" config',
       ]),
     );
   });
 
   test('query command with --builder returns only from the named builder.', () async {
-    final StringBuffer stderr = StringBuffer();
-    final StringBuffer stdout = StringBuffer();
-    final Environment env = linuxEnv(stderr, stdout);
+    final Logger logger = Logger.test();
+    final Environment env = linuxEnv(logger);
     final ToolCommandRunner runner = ToolCommandRunner(
       environment: env,
       configs: configs,
@@ -100,20 +103,19 @@ void main() {
     ]);
     expect(result, equals(0));
     expect(
-      stdout.toString().trim().split('\n'),
+      stringsFromLogs(logger.testLogs),
       equals(<String>[
         'Add --verbose to see detailed information about each builder',
         '',
         '"linux_test_config" builder:',
-        '  "build_name" config',
+        '   "build_name" config',
       ]),
     );
   });
 
   test('query command with --all returns all builds.', () async {
-    final StringBuffer stderr = StringBuffer();
-    final StringBuffer stdout = StringBuffer();
-    final Environment env = linuxEnv(stderr, stdout);
+    final Logger logger = Logger.test();
+    final Environment env = linuxEnv(logger);
     final ToolCommandRunner runner = ToolCommandRunner(
       environment: env,
       configs: configs,
@@ -123,7 +125,7 @@ void main() {
     ]);
     expect(result, equals(0));
     expect(
-      stdout.toString().trim().split('\n').length,
+      logger.testLogs.length,
       equals(10),
     );
   });