From 8803cecea1b8897bb5a3a6bfe0d75fdc3236cc6f Mon Sep 17 00:00:00 2001 From: John McCutchan Date: Mon, 7 Mar 2016 13:52:27 -0800 Subject: [PATCH] Refactor DeviceLogReader --- .../lib/src/android/android_device.dart | 94 ++++++-- .../flutter_tools/lib/src/base/process.dart | 24 +- .../flutter_tools/lib/src/commands/logs.dart | 21 +- packages/flutter_tools/lib/src/device.dart | 22 +- .../flutter_tools/lib/src/ios/devices.dart | 77 ++++++- .../flutter_tools/lib/src/ios/simulators.dart | 207 +++++++++++++----- 6 files changed, 351 insertions(+), 94 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index a364ee4d085..01890141649 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'dart:io'; import 'package:path/path.dart' as path; @@ -49,6 +50,8 @@ class AndroidDevice extends Device { bool get isLocalEmulator => false; + _AdbLogReader _logReader; + List adbCommandForDevice(List args) { return [androidSdk.adbPath, '-s', id]..addAll(args); } @@ -283,7 +286,12 @@ class AndroidDevice extends Device { runSync(adbCommandForDevice(['-s', id, 'logcat', '-c'])); } - DeviceLogReader createLogReader() => new _AdbLogReader(this); + DeviceLogReader get logReader { + if (_logReader == null) + _logReader = new _AdbLogReader(this); + + return _logReader; + } void startTracing(AndroidApk apk) { runCheckedSync(adbCommandForDevice([ @@ -460,26 +468,76 @@ class _AdbLogReader extends DeviceLogReader { final AndroidDevice device; + final StreamController _linesStreamController = + new StreamController.broadcast(); + + Process _process; + StreamSubscription _stdoutSubscription; + StreamSubscription _stderrSubscription; + + Stream get lines => _linesStreamController.stream; + String get name => device.name; - Future logs({ bool clear: false, bool showPrefix: false }) async { - if (clear) - device.clearLogs(); + bool get isReading => _process != null; - return await runCommandAndStreamOutput(device.adbCommandForDevice([ - '-s', - device.id, - 'logcat', - '-v', - 'tag', // Only log the tag and the message - '-T', - device.lastLogcatTimestamp, - '-s', - 'flutter:V', - 'ActivityManager:W', - 'System.err:W', - '*:F', - ]), prefix: showPrefix ? '[$name] ' : ''); + Future get finished => + _process != null ? _process.exitCode : new Future.value(0); + + Future start() async { + if (_process != null) { + throw new StateError( + '_AdbLogReader must be stopped before it can be started.'); + } + + // Start the adb logcat process. + _process = await runCommand(device.adbCommandForDevice( + [ + '-s', + device.id, + 'logcat', + '-v', + 'tag', // Only log the tag and the message + '-T', + device.lastLogcatTimestamp, + '-s', + 'flutter:V', + 'ActivityManager:W', + 'System.err:W', + '*:F', + ])); + _stdoutSubscription = + _process.stdout.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onLine); + _stderrSubscription = + _process.stderr.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onLine); + _process.exitCode.then(_onExit); + } + + Future stop() async { + if (_process == null) { + throw new StateError( + '_AdbLogReader must be started before it can be stopped.'); + } + _stdoutSubscription?.cancel(); + _stdoutSubscription = null; + _stderrSubscription?.cancel(); + _stderrSubscription = null; + await _process.kill(); + _process = null; + } + + void _onExit(int exitCode) { + _stdoutSubscription?.cancel(); + _stdoutSubscription = null; + _stderrSubscription?.cancel(); + _stderrSubscription = null; + _process = null; + } + + void _onLine(String line) { + _linesStreamController.add(line); } int get hashCode => name.hashCode; diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index f30c9b2629d..89ae5a37535 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -10,20 +10,30 @@ import '../globals.dart'; typedef String StringConverter(String string); +/// This runs the command in the background from the specified working +/// directory. Completes when the process has been started. +Future runCommand(List cmd, {String workingDirectory}) async { + printTrace(cmd.join(' ')); + String executable = cmd[0]; + List arguments = cmd.length > 1 ? cmd.sublist(1) : []; + Process process = await Process.start( + executable, + arguments, + workingDirectory: workingDirectory + ); + return process; +} + /// This runs the command and streams stdout/stderr from the child process to -/// this process' stdout/stderr. +/// this process' stdout/stderr. Completes with the process's exit code. Future runCommandAndStreamOutput(List cmd, { String workingDirectory, String prefix: '', RegExp filter, StringConverter mapFunction }) async { - printTrace(cmd.join(' ')); - Process process = await Process.start( - cmd[0], - cmd.sublist(1), - workingDirectory: workingDirectory - ); + Process process = await runCommand(cmd, + workingDirectory: workingDirectory); process.stdout .transform(UTF8.decoder) .transform(const LineSplitter()) diff --git a/packages/flutter_tools/lib/src/commands/logs.dart b/packages/flutter_tools/lib/src/commands/logs.dart index 9f7d1149a02..75261ca4ece 100644 --- a/packages/flutter_tools/lib/src/commands/logs.dart +++ b/packages/flutter_tools/lib/src/commands/logs.dart @@ -39,13 +39,30 @@ class LogsCommand extends FlutterCommand { List readers = new List(); for (Device device in devices) { - readers.add(device.createLogReader()); + if (clear) + device.clearLogs(); + + readers.add(device.logReader); } printStatus('Showing ${readers.join(', ')} logs:'); List results = await Future.wait(readers.map((DeviceLogReader reader) async { - int result = await reader.logs(clear: clear, showPrefix: devices.length > 1); + if (!reader.isReading) { + // Start reading. + await reader.start(); + } + StreamSubscription subscription = reader.lines.listen((String line) { + if (devices.length > 1) { + // Prefix with the name of the device. + print('[${reader.name}] $line'); + } else { + print(line); + } + }); + // Wait for the log reader to be finished. + int result = await reader.finished; + subscription.cancel(); if (result != 0) printError('Error listening to $reader logs.'); return result; diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 985bc6b1600..822b8bbf1eb 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -148,7 +148,11 @@ abstract class Device { TargetPlatform get platform; - DeviceLogReader createLogReader(); + /// Get the log reader for this device. + DeviceLogReader get logReader; + + /// Clear the device's logs. + void clearLogs(); /// Start an app package on the current device. /// @@ -189,7 +193,21 @@ abstract class Device { abstract class DeviceLogReader { String get name; - Future logs({ bool clear: false, bool showPrefix: false }); + /// A broadcast stream where each element in the string is a line of log + /// output. + Stream get lines; + + /// Start reading logs from the device. + Future start(); + + /// Actively reading lines from the log? + bool get isReading; + + /// Actively stop reading logs from the device. + Future stop(); + + /// Completes when the log is finished. + Future get finished; int get hashCode; bool operator ==(dynamic other); diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index 3f57bd25b28..8af09f4dd4d 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'dart:io'; import 'package:path/path.dart' as path; @@ -62,6 +63,8 @@ class IOSDevice extends Device { final String name; + _IOSDeviceLogReader _logReader; + bool get isLocalEmulator => false; bool get supportsStartPaused => false; @@ -220,7 +223,15 @@ class IOSDevice extends Device { @override TargetPlatform get platform => TargetPlatform.iOS; - DeviceLogReader createLogReader() => new _IOSDeviceLogReader(this); + DeviceLogReader get logReader { + if (_logReader == null) + _logReader = new _IOSDeviceLogReader(this); + + return _logReader; + } + + void clearLogs() { + } } class _IOSDeviceLogReader extends DeviceLogReader { @@ -228,15 +239,65 @@ class _IOSDeviceLogReader extends DeviceLogReader { final IOSDevice device; + final StreamController _linesStreamController = + new StreamController.broadcast(); + + Process _process; + StreamSubscription _stdoutSubscription; + StreamSubscription _stderrSubscription; + + Stream get lines => _linesStreamController.stream; + String get name => device.name; - // TODO(devoncarew): Support [clear]. - Future logs({ bool clear: false, bool showPrefix: false }) async { - return await runCommandAndStreamOutput( - [device.loggerPath], - prefix: showPrefix ? '[$name] ' : '', - filter: new RegExp(r'Runner') - ); + bool get isReading => _process != null; + + Future get finished => + _process != null ? _process.exitCode : new Future.value(0); + + Future start() async { + if (_process != null) { + throw new StateError( + '_IOSDeviceLogReader must be stopped before it can be started.'); + } + _process = await runCommand([device.loggerPath]); + _stdoutSubscription = + _process.stdout.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onLine); + _stderrSubscription = + _process.stderr.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onLine); + _process.exitCode.then(_onExit); + } + + Future stop() async { + if (_process == null) { + throw new StateError( + '_IOSDeviceLogReader must be started before it can be stopped.'); + } + _stdoutSubscription?.cancel(); + _stdoutSubscription = null; + _stderrSubscription?.cancel(); + _stderrSubscription = null; + await _process.kill(); + _process = null; + } + + void _onExit(int exitCode) { + _stdoutSubscription?.cancel(); + _stdoutSubscription = null; + _stderrSubscription?.cancel(); + _stderrSubscription = null; + _process = null; + } + + RegExp _runnerRegex = new RegExp(r'Runner'); + + void _onLine(String line) { + if (!_runnerRegex.hasMatch(line)) + return; + + _linesStreamController.add(line); } int get hashCode => name.hashCode; diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 0cb5e540a40..cb9ab5d5853 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -3,7 +3,7 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:convert' show JSON; +import 'dart:convert'; import 'dart:io'; import 'package:path/path.dart' as path; @@ -213,6 +213,8 @@ class IOSSimulator extends Device { bool get isLocalEmulator => true; + _IOSSimulatorLogReader _logReader; + String get xcrunPath => path.join('/usr', 'bin', 'xcrun'); String _getSimulatorPath() { @@ -428,7 +430,12 @@ class IOSSimulator extends Device { @override TargetPlatform get platform => TargetPlatform.iOSSimulator; - DeviceLogReader createLogReader() => new _IOSSimulatorLogReader(this); + DeviceLogReader get logReader { + if (_logReader == null) + _logReader = new _IOSSimulatorLogReader(this); + + return _logReader; + } void clearLogs() { File logFile = new File(logFilePath); @@ -451,71 +458,157 @@ class _IOSSimulatorLogReader extends DeviceLogReader { final IOSSimulator device; + final StreamController _linesStreamController = + new StreamController.broadcast(); + bool _lastWasFiltered = false; + // We log from two logs: the device and the system log. + Process _deviceProcess; + StreamSubscription _deviceStdoutSubscription; + StreamSubscription _deviceStderrSubscription; + + Process _systemProcess; + StreamSubscription _systemStdoutSubscription; + StreamSubscription _systemStderrSubscription; + + Stream get lines => _linesStreamController.stream; + String get name => device.name; - Future logs({ bool clear: false, bool showPrefix: false }) async { - if (clear) - device.clearLogs(); + bool get isReading => (_deviceProcess != null) && (_systemProcess != null); + Future get finished => + (_deviceProcess != null) ? _deviceProcess.exitCode : new Future.value(0); + + Future start() async { + if (isReading) { + throw new StateError( + '_IOSSimulatorLogReader must be stopped before it can be started.'); + } + + // TODO(johnmccutchan): Add a ProcessSet abstraction that handles running + // N processes and merging their output. + + // Device log. device.ensureLogsExists(); - - // Match the log prefix (in order to shorten it): - // 'Jan 29 01:31:44 devoncarew-macbookpro3 SpringBoard[96648]: ...' - RegExp mapRegex = new RegExp(r'\S+ +\S+ +\S+ \S+ (.+)\[\d+\]\)?: (.*)$'); - // Jan 31 19:23:28 --- last message repeated 1 time --- - RegExp lastMessageRegex = new RegExp(r'\S+ +\S+ +\S+ --- (.*) ---$'); - - // This filter matches many Flutter lines in the log: - // new RegExp(r'(FlutterRunner|flutter.runner.Runner|$id)'), but it misses - // a fair number, including ones that would be useful in diagnosing crashes. - // For now, we're not filtering the log file (but do clear it with each run). - - Future result = runCommandAndStreamOutput( - ['tail', '-n', '+0', '-F', device.logFilePath], - prefix: showPrefix ? '[$name] ' : '', - mapFunction: (String string) { - Match match = mapRegex.matchAsPrefix(string); - if (match != null) { - _lastWasFiltered = true; - - // Filter out some messages that clearly aren't related to Flutter. - if (string.contains(': could not find icon for representation -> com.apple.')) - return null; - String category = match.group(1); - String content = match.group(2); - if (category == 'Game Center' || category == 'itunesstored' || category == 'nanoregistrylaunchd' || - category == 'mstreamd' || category == 'syncdefaultsd' || category == 'companionappd' || - category == 'searchd') - return null; - - _lastWasFiltered = false; - - if (category == 'Runner') - return content; - return '$category: $content'; - } - match = lastMessageRegex.matchAsPrefix(string); - if (match != null && !_lastWasFiltered) - return '(${match.group(1)})'; - return string; - } - ); + _deviceProcess = await runCommand( + ['tail', '-n', '+0', '-F', device.logFilePath]); + _deviceStdoutSubscription = + _deviceProcess.stdout.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onDeviceLine); + _deviceStderrSubscription = + _deviceProcess.stderr.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onDeviceLine); + _deviceProcess.exitCode.then(_onDeviceExit); // Track system.log crashes. // ReportCrash[37965]: Saved crash report for FlutterRunner[37941]... - runCommandAndStreamOutput( - ['tail', '-F', '/private/var/log/system.log'], - prefix: showPrefix ? '[$name] ' : '', - filter: new RegExp(r' FlutterRunner\[\d+\] '), - mapFunction: (String string) { - Match match = mapRegex.matchAsPrefix(string); - return match == null ? string : '${match.group(1)}: ${match.group(2)}'; - } - ); + _systemProcess = await runCommand( + ['tail', '-F', '/private/var/log/system.log']); + _systemStdoutSubscription = + _systemProcess.stdout.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onSystemLine); + _systemStderrSubscription = + _systemProcess.stderr.transform(UTF8.decoder) + .transform(const LineSplitter()).listen(_onSystemLine); + _systemProcess.exitCode.then(_onSystemExit); + } - return await result; + Future stop() async { + if (!isReading) { + throw new StateError( + '_IOSSimulatorLogReader must be started before it can be stopped.'); + } + if (_deviceProcess != null) { + await _deviceProcess.kill(); + _deviceProcess = null; + } + _onDeviceExit(0); + if (_systemProcess != null) { + await _systemProcess.kill(); + _systemProcess = null; + } + _onSystemExit(0); + } + + void _onDeviceExit(int exitCode) { + _deviceStdoutSubscription?.cancel(); + _deviceStdoutSubscription = null; + _deviceStderrSubscription?.cancel(); + _deviceStderrSubscription = null; + _deviceProcess = null; + } + + void _onSystemExit(int exitCode) { + _systemStdoutSubscription?.cancel(); + _systemStdoutSubscription = null; + _systemStderrSubscription?.cancel(); + _systemStderrSubscription = null; + _systemProcess = null; + } + + // Match the log prefix (in order to shorten it): + // 'Jan 29 01:31:44 devoncarew-macbookpro3 SpringBoard[96648]: ...' + final RegExp _mapRegex = + new RegExp(r'\S+ +\S+ +\S+ \S+ (.+)\[\d+\]\)?: (.*)$'); + + // Jan 31 19:23:28 --- last message repeated 1 time --- + final RegExp _lastMessageRegex = new RegExp(r'\S+ +\S+ +\S+ --- (.*) ---$'); + + final RegExp _flutterRunnerRegex = new RegExp(r' FlutterRunner\[\d+\] '); + + String _filterDeviceLine(String string) { + Match match = _mapRegex.matchAsPrefix(string); + if (match != null) { + _lastWasFiltered = true; + + // Filter out some messages that clearly aren't related to Flutter. + if (string.contains(': could not find icon for representation -> com.apple.')) + return null; + + String category = match.group(1); + String content = match.group(2); + if (category == 'Game Center' || category == 'itunesstored' || + category == 'nanoregistrylaunchd' || category == 'mstreamd' || + category == 'syncdefaultsd' || category == 'companionappd' || + category == 'searchd') + return null; + + _lastWasFiltered = false; + + if (category == 'Runner') + return content; + return '$category: $content'; + } + match = _lastMessageRegex.matchAsPrefix(string); + if (match != null && !_lastWasFiltered) + return '(${match.group(1)})'; + return string; + } + + void _onDeviceLine(String line) { + String filteredLine = _filterDeviceLine(line); + if (filteredLine == null) + return; + + _linesStreamController.add(filteredLine); + } + + String _filterSystemLog(String string) { + Match match = _mapRegex.matchAsPrefix(string); + return match == null ? string : '${match.group(1)}: ${match.group(2)}'; + } + + void _onSystemLine(String line) { + if (!_flutterRunnerRegex.hasMatch(line)) + return; + + String filteredLine = _filterSystemLog(line); + if (filteredLine == null) + return; + + _linesStreamController.add(filteredLine); } int get hashCode => device.logFilePath.hashCode;