Unverified Commit 8e6a9e3a authored by Victoria Ashworth's avatar Victoria Ashworth Committed by GitHub

Revert "Log all lines from ios-deploy (#127502)" (#127684)

This reverts commit a19b3436.

We added this logging to try and determine if the reason for Dart VM errors (https://github.com/flutter/flutter/issues/121231) was caused by some issue with the streams.
A recent test proves that is not the case:
https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20platform_view_ios__start_up/11046/overview
The test shows the Dart VM url in the device log. However, the test log does NOT show a log for the Dart VM url but does show the stack trace, which all come from the main stream, which means it's not an issue with the secondary streams not receiving the log.

So reverting the debugging we added.
parent bd10c79b
...@@ -348,12 +348,6 @@ class IOSDeployDebugger { ...@@ -348,12 +348,6 @@ class IOSDeployDebugger {
.transform<String>(utf8.decoder) .transform<String>(utf8.decoder)
.transform<String>(const LineSplitter()) .transform<String>(const LineSplitter())
.listen((String line) { .listen((String line) {
// TODO(vashworth): Revert after https://github.com/flutter/flutter/issues/121231 is resolved.
if (line.isNotEmpty) {
_logger.printTrace(line);
}
_monitorIOSDeployFailure(line, _logger); _monitorIOSDeployFailure(line, _logger);
// (lldb) platform select remote-'ios' --sysroot // (lldb) platform select remote-'ios' --sysroot
...@@ -371,6 +365,7 @@ class IOSDeployDebugger { ...@@ -371,6 +365,7 @@ class IOSDeployDebugger {
} }
final String prompt = line.substring(0, promptEndIndex); final String prompt = line.substring(0, promptEndIndex);
lldbRun = RegExp(RegExp.escape(prompt) + r'\s*run'); lldbRun = RegExp(RegExp.escape(prompt) + r'\s*run');
_logger.printTrace(line);
return; return;
} }
...@@ -389,6 +384,7 @@ class IOSDeployDebugger { ...@@ -389,6 +384,7 @@ class IOSDeployDebugger {
// success // success
// 2020-09-15 13:42:25.185474-0700 Runner[477:181141] flutter: The Dart VM service is listening on http://127.0.0.1:57782/ // 2020-09-15 13:42:25.185474-0700 Runner[477:181141] flutter: The Dart VM service is listening on http://127.0.0.1:57782/
if (lldbRun.hasMatch(line)) { if (lldbRun.hasMatch(line)) {
_logger.printTrace(line);
_debuggerState = _IOSDeployDebuggerState.launching; _debuggerState = _IOSDeployDebuggerState.launching;
// TODO(vashworth): Remove all debugger state comments when https://github.com/flutter/flutter/issues/126412 is resolved. // TODO(vashworth): Remove all debugger state comments when https://github.com/flutter/flutter/issues/126412 is resolved.
_logger.printTrace('Debugger state set to launching.'); _logger.printTrace('Debugger state set to launching.');
...@@ -397,6 +393,7 @@ class IOSDeployDebugger { ...@@ -397,6 +393,7 @@ class IOSDeployDebugger {
// Next line after "run" must be "success", or the attach failed. // Next line after "run" must be "success", or the attach failed.
// Example: "error: process launch failed" // Example: "error: process launch failed"
if (_debuggerState == _IOSDeployDebuggerState.launching) { if (_debuggerState == _IOSDeployDebuggerState.launching) {
_logger.printTrace(line);
final bool attachSuccess = line == 'success'; final bool attachSuccess = line == 'success';
_debuggerState = attachSuccess ? _IOSDeployDebuggerState.attached : _IOSDeployDebuggerState.detached; _debuggerState = attachSuccess ? _IOSDeployDebuggerState.attached : _IOSDeployDebuggerState.detached;
_logger.printTrace('Debugger state set to ${attachSuccess ? 'attached' : 'detached'}.'); _logger.printTrace('Debugger state set to ${attachSuccess ? 'attached' : 'detached'}.');
...@@ -411,6 +408,7 @@ class IOSDeployDebugger { ...@@ -411,6 +408,7 @@ class IOSDeployDebugger {
// process signal SIGSTOP // process signal SIGSTOP
if (line.contains(_signalStop)) { if (line.contains(_signalStop)) {
// The app is about to be stopped. Only show in verbose mode. // The app is about to be stopped. Only show in verbose mode.
_logger.printTrace(line);
return; return;
} }
...@@ -423,6 +421,7 @@ class IOSDeployDebugger { ...@@ -423,6 +421,7 @@ class IOSDeployDebugger {
if (line == _backTraceAll) { if (line == _backTraceAll) {
// The app is stopped and the backtrace for all threads will be printed. // The app is stopped and the backtrace for all threads will be printed.
_logger.printTrace(line);
// Even though we're not "detached", just stopped, mark as detached so the backtrace // Even though we're not "detached", just stopped, mark as detached so the backtrace
// is only show in verbose. // is only show in verbose.
_debuggerState = _IOSDeployDebuggerState.detached; _debuggerState = _IOSDeployDebuggerState.detached;
...@@ -439,6 +438,7 @@ class IOSDeployDebugger { ...@@ -439,6 +438,7 @@ class IOSDeployDebugger {
if (line.contains('PROCESS_STOPPED') || _lldbProcessStopped.hasMatch(line)) { if (line.contains('PROCESS_STOPPED') || _lldbProcessStopped.hasMatch(line)) {
// The app has been stopped. Dump the backtrace, and detach. // The app has been stopped. Dump the backtrace, and detach.
_logger.printTrace(line);
_iosDeployProcess?.stdin.writeln(_backTraceAll); _iosDeployProcess?.stdin.writeln(_backTraceAll);
if (_processResumeCompleter == null) { if (_processResumeCompleter == null) {
detach(); detach();
...@@ -449,17 +449,20 @@ class IOSDeployDebugger { ...@@ -449,17 +449,20 @@ class IOSDeployDebugger {
if (line.contains('PROCESS_EXITED') || _lldbProcessExit.hasMatch(line)) { if (line.contains('PROCESS_EXITED') || _lldbProcessExit.hasMatch(line)) {
// The app exited or crashed, so exit. Continue passing debugging // The app exited or crashed, so exit. Continue passing debugging
// messages to the log reader until it exits to capture crash dumps. // messages to the log reader until it exits to capture crash dumps.
_logger.printTrace(line);
exit(); exit();
return; return;
} }
if (_lldbProcessDetached.hasMatch(line)) { if (_lldbProcessDetached.hasMatch(line)) {
// The debugger has detached from the app, and there will be no more debugging messages. // The debugger has detached from the app, and there will be no more debugging messages.
// Kill the ios-deploy process. // Kill the ios-deploy process.
_logger.printTrace(line);
exit(); exit();
return; return;
} }
if (_lldbProcessResuming.hasMatch(line)) { if (_lldbProcessResuming.hasMatch(line)) {
_logger.printTrace(line);
// we marked this detached when we received [_backTraceAll] // we marked this detached when we received [_backTraceAll]
_debuggerState = _IOSDeployDebuggerState.attached; _debuggerState = _IOSDeployDebuggerState.attached;
_logger.printTrace('Debugger state set to attached.'); _logger.printTrace('Debugger state set to attached.');
...@@ -467,6 +470,7 @@ class IOSDeployDebugger { ...@@ -467,6 +470,7 @@ class IOSDeployDebugger {
} }
if (_debuggerState != _IOSDeployDebuggerState.attached) { if (_debuggerState != _IOSDeployDebuggerState.attached) {
_logger.printTrace(line);
return; return;
} }
if (lastLineFromDebugger != null && lastLineFromDebugger!.isNotEmpty && line.isEmpty) { if (lastLineFromDebugger != null && lastLineFromDebugger!.isNotEmpty && line.isEmpty) {
...@@ -484,7 +488,7 @@ class IOSDeployDebugger { ...@@ -484,7 +488,7 @@ class IOSDeployDebugger {
.transform<String>(const LineSplitter()) .transform<String>(const LineSplitter())
.listen((String line) { .listen((String line) {
_monitorIOSDeployFailure(line, _logger); _monitorIOSDeployFailure(line, _logger);
_logger.printTrace('error: $line'); _logger.printTrace(line);
}); });
unawaited(_iosDeployProcess!.exitCode.then((int status) async { unawaited(_iosDeployProcess!.exitCode.then((int status) async {
_logger.printTrace('ios-deploy exited with code $exitCode'); _logger.printTrace('ios-deploy exited with code $exitCode');
......
...@@ -36,7 +36,6 @@ import 'devfs.dart'; ...@@ -36,7 +36,6 @@ import 'devfs.dart';
import 'device.dart'; import 'device.dart';
import 'features.dart'; import 'features.dart';
import 'globals.dart' as globals; import 'globals.dart' as globals;
import 'ios/devices.dart';
import 'project.dart'; import 'project.dart';
import 'resident_devtools_handler.dart'; import 'resident_devtools_handler.dart';
import 'run_cold.dart'; import 'run_cold.dart';
...@@ -229,9 +228,8 @@ class FlutterDevice { ...@@ -229,9 +228,8 @@ class FlutterDevice {
FlutterVmService? vmService; FlutterVmService? vmService;
DevFS? devFS; DevFS? devFS;
ApplicationPackage? package; ApplicationPackage? package;
@visibleForTesting
// ignore: cancel_subscriptions // ignore: cancel_subscriptions
StreamSubscription<String>? loggingSubscription; StreamSubscription<String>? _loggingSubscription;
bool? _isListeningForVmServiceUri; bool? _isListeningForVmServiceUri;
/// Whether the stream [vmServiceUris] is still open. /// Whether the stream [vmServiceUris] is still open.
...@@ -394,26 +392,23 @@ class FlutterDevice { ...@@ -394,26 +392,23 @@ class FlutterDevice {
} }
Future<void> startEchoingDeviceLog() async { Future<void> startEchoingDeviceLog() async {
if (loggingSubscription != null) { if (_loggingSubscription != null) {
return; return;
} }
final DeviceLogReader logReader = await device!.getLogReader(app: package); final Stream<String> logStream = (await device!.getLogReader(app: package)).logLines;
final Stream<String> logStream = logReader.logLines; _loggingSubscription = logStream.listen((String line) {
// TODO(vashworth): Remove check for IOSDeviceLogReader after if (!line.contains(globals.kVMServiceMessageRegExp)) {
// https://github.com/flutter/flutter/issues/121231 is resolved.
loggingSubscription = logStream.listen((String line) {
if (logReader is! IOSDeviceLogReader && !line.contains(globals.kVMServiceMessageRegExp)) {
globals.printStatus(line, wrap: false); globals.printStatus(line, wrap: false);
} }
}); });
} }
Future<void> stopEchoingDeviceLog() async { Future<void> stopEchoingDeviceLog() async {
if (loggingSubscription == null) { if (_loggingSubscription == null) {
return; return;
} }
await loggingSubscription!.cancel(); await _loggingSubscription!.cancel();
loggingSubscription = null; _loggingSubscription = null;
} }
Future<void> initLogReader() async { Future<void> initLogReader() async {
......
...@@ -94,26 +94,6 @@ void main () { ...@@ -94,26 +94,6 @@ void main () {
logger = BufferLogger.test(); logger = BufferLogger.test();
}); });
testWithoutContext('print all lines', () async {
final StreamController<List<int>> stdin = StreamController<List<int>>();
final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
FakeCommand(
command: const <String>['ios-deploy'],
stdout: "(mylldb) platform select remote-'ios' --sysroot\r\n(mylldb) run\r\nsuccess\r\nrandom string\r\n",
stdin: IOSink(stdin.sink),
),
]);
final IOSDeployDebugger iosDeployDebugger = IOSDeployDebugger.test(
processManager: processManager,
logger: logger,
);
expect(await iosDeployDebugger.launchAndAttach(), isTrue);
expect(logger.traceText, contains("(mylldb) platform select remote-'ios' --sysroot"));
expect(logger.traceText, contains('(mylldb) run'));
expect(logger.traceText, contains('success'));
expect(logger.traceText, contains('random string'));
});
testWithoutContext('custom lldb prompt', () async { testWithoutContext('custom lldb prompt', () async {
final StreamController<List<int>> stdin = StreamController<List<int>>(); final StreamController<List<int>> stdin = StreamController<List<int>>();
final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[ final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
......
...@@ -20,7 +20,6 @@ import 'package:flutter_tools/src/base/platform.dart'; ...@@ -20,7 +20,6 @@ import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/scene_importer.dart'; import 'package:flutter_tools/src/build_system/targets/scene_importer.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart'; import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/compile.dart'; import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/convert.dart'; import 'package:flutter_tools/src/convert.dart';
import 'package:flutter_tools/src/devfs.dart'; import 'package:flutter_tools/src/devfs.dart';
...@@ -28,8 +27,6 @@ import 'package:flutter_tools/src/device.dart'; ...@@ -28,8 +27,6 @@ import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/device_port_forwarder.dart'; import 'package:flutter_tools/src/device_port_forwarder.dart';
import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/features.dart';
import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:flutter_tools/src/ios/devices.dart';
import 'package:flutter_tools/src/ios/mac.dart';
import 'package:flutter_tools/src/project.dart'; import 'package:flutter_tools/src/project.dart';
import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/reporting/reporting.dart';
import 'package:flutter_tools/src/resident_devtools_handler.dart'; import 'package:flutter_tools/src/resident_devtools_handler.dart';
...@@ -44,7 +41,6 @@ import 'package:vm_service/vm_service.dart' as vm_service; ...@@ -44,7 +41,6 @@ import 'package:vm_service/vm_service.dart' as vm_service;
import '../src/common.dart'; import '../src/common.dart';
import '../src/context.dart'; import '../src/context.dart';
import '../src/fake_devices.dart';
import '../src/fake_vm_services.dart'; import '../src/fake_vm_services.dart';
import '../src/fakes.dart'; import '../src/fakes.dart';
import '../src/testbed.dart'; import '../src/testbed.dart';
...@@ -2442,70 +2438,6 @@ flutter: ...@@ -2442,70 +2438,6 @@ flutter:
expect(flutterDevice.devFS!.hasSetAssetDirectory, true); expect(flutterDevice.devFS!.hasSetAssetDirectory, true);
expect(fakeVmServiceHost!.hasRemainingExpectations, false); expect(fakeVmServiceHost!.hasRemainingExpectations, false);
})); }));
group('startEchoingDeviceLog', () {
late FakeProcessManager processManager;
late Artifacts artifacts;
late Cache fakeCache;
late BufferLogger logger;
setUp(() {
processManager = FakeProcessManager.empty();
fakeCache = Cache.test(processManager: FakeProcessManager.any());
artifacts = Artifacts.test();
logger = BufferLogger.test();
});
testUsingContext('IOSDeviceLogReader does not print logs', () async {
final IOSDeviceLogReader logReader = IOSDeviceLogReader.test(
iMobileDevice: IMobileDevice(
artifacts: artifacts,
processManager: processManager,
cache: fakeCache,
logger: logger,
),
useSyslog: false,
);
device = FakeDevice(deviceLogReader: logReader);
final TestFlutterDevice flutterDevice = TestFlutterDevice(
device,
);
await flutterDevice.startEchoingDeviceLog();
final Future<List<String>> linesFromStream = logReader.logLines.toList();
logReader.linesController.add('event');
await logReader.linesController.close();
final List<String> lines = await linesFromStream;
expect(lines, contains('event'));
expect(logger.statusText, isEmpty);
await flutterDevice.stopEchoingDeviceLog();
}, overrides: <Type, Generator>{
Logger: () => logger,
});
testUsingContext('Non-IOSDeviceLogReader does print logs', () async {
final FakeDeviceLogReader logReader = FakeDeviceLogReader();
device = FakeDevice(deviceLogReader: logReader);
final TestFlutterDevice flutterDevice = TestFlutterDevice(
device,
);
await flutterDevice.startEchoingDeviceLog();
final Future<List<String>> linesFromStream = logReader.logLines.toList();
logReader.addLine('event');
await logReader.dispose();
final List<String> lines = await linesFromStream;
expect(lines, contains('event'));
expect(logger.statusText, contains('event'));
await flutterDevice.stopEchoingDeviceLog();
}, overrides: <Type, Generator>{
Logger: () => logger,
});
});
} }
// This implements [dds.DartDevelopmentService], not the [DartDevelopmentService] // This implements [dds.DartDevelopmentService], not the [DartDevelopmentService]
...@@ -2744,16 +2676,13 @@ class FakeDevice extends Fake implements Device { ...@@ -2744,16 +2676,13 @@ class FakeDevice extends Fake implements Device {
this.supportsHotRestart = true, this.supportsHotRestart = true,
this.supportsScreenshot = true, this.supportsScreenshot = true,
this.supportsFlutterExit = true, this.supportsFlutterExit = true,
DeviceLogReader? deviceLogReader,
}) : _isLocalEmulator = isLocalEmulator, }) : _isLocalEmulator = isLocalEmulator,
_targetPlatform = targetPlatform, _targetPlatform = targetPlatform,
_sdkNameAndVersion = sdkNameAndVersion, _sdkNameAndVersion = sdkNameAndVersion;
_deviceLogReader = deviceLogReader;
final bool _isLocalEmulator; final bool _isLocalEmulator;
final TargetPlatform _targetPlatform; final TargetPlatform _targetPlatform;
final String _sdkNameAndVersion; final String _sdkNameAndVersion;
final DeviceLogReader? _deviceLogReader;
bool disposed = false; bool disposed = false;
bool appStopped = false; bool appStopped = false;
...@@ -2811,12 +2740,7 @@ class FakeDevice extends Fake implements Device { ...@@ -2811,12 +2740,7 @@ class FakeDevice extends Fake implements Device {
FutureOr<DeviceLogReader> getLogReader({ FutureOr<DeviceLogReader> getLogReader({
ApplicationPackage? app, ApplicationPackage? app,
bool includePastLogs = false, bool includePastLogs = false,
}) { }) => NoOpDeviceLogReader(name);
if (_deviceLogReader != null) {
return _deviceLogReader!;
}
return NoOpDeviceLogReader(name);
}
@override @override
DevicePortForwarder portForwarder = const NoOpDevicePortForwarder(); DevicePortForwarder portForwarder = const NoOpDevicePortForwarder();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment