Add ipv6 and observatory port support to the attach command (#25303) * Add ipv6 and observatory port support to the attach command. * Remove whitespace * Explain why a name change is useful here * Refactor common flags out into the parent * Add an additional test case for when observatory port is provided but debug port is not * Remove whitespace * Fix flag validation
diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index 844abb0b..63316af 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart
@@ -48,12 +48,14 @@ addBuildModeFlags(defaultToRelease: false); usesIsolateFilterOption(hide: !verboseHelp); usesTargetOption(); + usesPortOptions(); + usesIpv6Flag(); usesFilesystemOptions(hide: !verboseHelp); usesFuchsiaOptions(hide: !verboseHelp); argParser ..addOption( 'debug-port', - help: 'Local port where the observatory is listening.', + help: 'Device port where the observatory is listening.', )..addOption('pid-file', help: 'Specify a file to write the process id to. ' 'You can send SIGUSR1 to trigger a hot reload ' @@ -79,7 +81,7 @@ @override final String description = 'Attach to a running application.'; - int get observatoryPort { + int get debugPort { if (argResults['debug-port'] == null) return null; try { @@ -95,7 +97,19 @@ await super.validateCommand(); if (await findTargetDevice() == null) throwToolExit(null); - observatoryPort; + debugPort; + if (debugPort == null && argResults.wasParsed(FlutterCommand.ipv6Flag)) { + throwToolExit( + 'When the --debug-port is unknown, this command determines ' + 'the value of --ipv6 on its own.', + ); + } + if (debugPort == null && argResults.wasParsed(FlutterCommand.observatoryPortOption)) { + throwToolExit( + 'When the --debug-port is unknown, this command does not use ' + 'the value of --observatory-port.', + ); + } } @override @@ -107,7 +121,7 @@ writePidFile(argResults['pid-file']); final Device device = await findTargetDevice(); - final int devicePort = observatoryPort; + final int devicePort = debugPort; final Daemon daemon = argResults['machine'] ? Daemon(stdinCommandStream, stdoutCommandResponse, @@ -115,7 +129,7 @@ : null; Uri observatoryUri; - bool ipv6 = false; + bool usesIpv6 = false; bool attachLogger = false; if (devicePort == null) { if (device is FuchsiaDevice) { @@ -124,7 +138,7 @@ if (module == null) { throwToolExit('\'--module\' is requried for attaching to a Fuchsia device'); } - ipv6 = _isIpv6(device.id); + usesIpv6 = _isIpv6(device.id); final List<int> ports = await device.servicePorts(); if (ports.isEmpty) { throwToolExit('No active service ports on ${device.name}'); @@ -142,7 +156,7 @@ if (localPort == null) { throwToolExit('No active Observatory running module \'$module\' on ${device.name}'); } - observatoryUri = ipv6 + observatoryUri = usesIpv6 ? Uri.parse('http://[$ipv6Loopback]:$localPort/') : Uri.parse('http://$ipv4Loopback:$localPort/'); status.stop(); @@ -163,14 +177,20 @@ ); printStatus('Waiting for a connection from Flutter on ${device.name}...'); observatoryUri = await observatoryDiscovery.uri; + // Determine ipv6 status from the scanned logs. + usesIpv6 = observatoryDiscovery.ipv6; printStatus('Done.'); } finally { await observatoryDiscovery?.cancel(); } } } else { - final int localPort = await device.portForwarder.forward(devicePort); - observatoryUri = Uri.parse('http://$ipv4Loopback:$localPort/'); + usesIpv6 = ipv6; + final int localPort = observatoryPort + ?? await device.portForwarder.forward(devicePort); + observatoryUri = usesIpv6 + ? Uri.parse('http://[$ipv6Loopback]:$localPort/') + : Uri.parse('http://$ipv4Loopback:$localPort/'); } try { final FlutterDevice flutterDevice = FlutterDevice( @@ -191,7 +211,7 @@ usesTerminalUI: daemon == null, projectRootPath: argResults['project-root'], dillOutputPath: argResults['output-dill'], - ipv6: ipv6, + ipv6: usesIpv6, ); if (attachLogger) { flutterDevice.startEchoingDeviceLog();
diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index cb2efb9..f7d83d7 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart
@@ -30,12 +30,6 @@ negatable: false, help: 'Start tracing during startup.', ) - ..addFlag('ipv6', - hide: true, - negatable: false, - help: 'Binds to IPv6 localhost instead of IPv4 when the flutter tool ' - 'forwards the host port to a device port.', - ) ..addOption('route', help: 'Which route to load when running the app.', ) @@ -46,31 +40,13 @@ 'Android device.\nIgnored on iOS.'); usesTargetOption(); usesPortOptions(); + usesIpv6Flag(); usesPubOption(); usesIsolateFilterOption(hide: !verboseHelp); } bool get traceStartup => argResults['trace-startup']; - bool get ipv6 => argResults['ipv6']; String get route => argResults['route']; - - void usesPortOptions() { - argParser.addOption('observatory-port', - help: 'Listen to the given port for an observatory debugger connection.\n' - 'Specifying port 0 (the default) will find a random free port.' - ); - } - - int get observatoryPort { - if (argResults['observatory-port'] != null) { - try { - return int.parse(argResults['observatory-port']); - } catch (error) { - throwToolExit('Invalid port for `--observatory-port`: $error'); - } - } - return null; - } } class RunCommand extends RunCommandBase {
diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 83ea0b4..8bc1e3d 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart
@@ -72,6 +72,12 @@ /// Will be `null` until the top-most command has begun execution. static FlutterCommand get current => context[FlutterCommand]; + /// The option name for a custom observatory port. + static const String observatoryPortOption = 'observatory-port'; + + /// The flag name for whether or not to use ipv6. + static const String ipv6Flag = 'ipv6'; + @override ArgParser get argParser => _argParser; final ArgParser _argParser = ArgParser(allowTrailingOptions: false); @@ -86,6 +92,10 @@ bool _usesPubOption = false; + bool _usesPortOption = false; + + bool _usesIpv6Flag = false; + bool get shouldRunPub => _usesPubOption && argResults['pub']; bool get shouldUpdateCache => true; @@ -148,6 +158,43 @@ ); } + /// Adds options for connecting to the Dart VM observatory port. + void usesPortOptions() { + argParser.addOption(observatoryPortOption, + help: 'Listen to the given port for an observatory debugger connection.\n' + 'Specifying port 0 (the default) will find a random free port.' + ); + _usesPortOption = true; + } + + /// Gets the observatory port provided to in the 'observatory-port' option. + /// + /// If no port is set, returns null. + int get observatoryPort { + if (!_usesPortOption || argResults['observatory-port'] == null) { + return null; + } + try { + return int.parse(argResults['observatory-port']); + } catch (error) { + throwToolExit('Invalid port for `--observatory-port`: $error'); + } + return null; + } + + void usesIpv6Flag() { + argParser.addFlag(ipv6Flag, + hide: true, + negatable: false, + help: 'Binds to IPv6 localhost instead of IPv4 when the flutter tool ' + 'forwards the host port to a device port. Not used when the ' + '--debug-port flag is not set.', + ); + _usesIpv6Flag = true; + } + + bool get ipv6 => _usesIpv6Flag ? argResults['ipv6'] : null; + void usesBuildNumberOption() { argParser.addOption('build-number', help: 'An integer used as an internal version number.\n'
diff --git a/packages/flutter_tools/test/commands/attach_test.dart b/packages/flutter_tools/test/commands/attach_test.dart index 9f4e67c..064a7e6 100644 --- a/packages/flutter_tools/test/commands/attach_test.dart +++ b/packages/flutter_tools/test/commands/attach_test.dart
@@ -104,6 +104,7 @@ debuggingOptions: anyNamed('debuggingOptions'), packagesFilePath: anyNamed('packagesFilePath'), usesTerminalUI: anyNamed('usesTerminalUI'), + ipv6: false, ), )..thenReturn(MockHotRunner()); @@ -134,6 +135,7 @@ debuggingOptions: anyNamed('debuggingOptions'), packagesFilePath: anyNamed('packagesFilePath'), usesTerminalUI: anyNamed('usesTerminalUI'), + ipv6: false, ), )..called(1); @@ -150,6 +152,36 @@ }, overrides: <Type, Generator>{ FileSystem: () => testFileSystem, }); + + testUsingContext('exits when ipv6 is specified and debug-port is not', () async { + testDeviceManager.addDevice(device); + + final AttachCommand command = AttachCommand(); + await expectLater( + createTestCommandRunner(command).run(<String>['attach', '--ipv6']), + throwsToolExit( + message: 'When the --debug-port is unknown, this command determines ' + 'the value of --ipv6 on its own.', + ), + ); + }, overrides: <Type, Generator>{ + FileSystem: () => testFileSystem, + },); + + testUsingContext('exits when observatory-port is specified and debug-port is not', () async { + testDeviceManager.addDevice(device); + + final AttachCommand command = AttachCommand(); + await expectLater( + createTestCommandRunner(command).run(<String>['attach', '--observatory-port', '100']), + throwsToolExit( + message: 'When the --debug-port is unknown, this command does not use ' + 'the value of --observatory-port.', + ), + ); + }, overrides: <Type, Generator>{ + FileSystem: () => testFileSystem, + },); }); @@ -170,7 +202,8 @@ target: anyNamed('target'), debuggingOptions: anyNamed('debuggingOptions'), packagesFilePath: anyNamed('packagesFilePath'), - usesTerminalUI: anyNamed('usesTerminalUI'))).thenReturn( + usesTerminalUI: anyNamed('usesTerminalUI'), + ipv6: false)).thenReturn( MockHotRunner()); testDeviceManager.addDevice(device); @@ -199,33 +232,92 @@ target: foo.path, debuggingOptions: anyNamed('debuggingOptions'), packagesFilePath: anyNamed('packagesFilePath'), - usesTerminalUI: anyNamed('usesTerminalUI'))).called(1); + usesTerminalUI: anyNamed('usesTerminalUI'), + ipv6: false)).called(1); }, overrides: <Type, Generator>{ FileSystem: () => testFileSystem, },); - testUsingContext('forwards to given port', () async { + group('forwarding to given port', () { const int devicePort = 499; const int hostPort = 42; - final MockPortForwarder portForwarder = MockPortForwarder(); - final MockAndroidDevice device = MockAndroidDevice(); + MockPortForwarder portForwarder; + MockAndroidDevice device; - when(device.portForwarder).thenReturn(portForwarder); - when(portForwarder.forward(devicePort)).thenAnswer((_) async => hostPort); - when(portForwarder.forwardedPorts).thenReturn( - <ForwardedPort>[ForwardedPort(hostPort, devicePort)]); - when(portForwarder.unforward(any)).thenAnswer((_) async => null); - testDeviceManager.addDevice(device); + setUp(() { + portForwarder = MockPortForwarder(); + device = MockAndroidDevice(); - final AttachCommand command = AttachCommand(); + when(device.portForwarder).thenReturn(portForwarder); + when(portForwarder.forward(devicePort)).thenAnswer((_) async => hostPort); + when(portForwarder.forwardedPorts).thenReturn( + <ForwardedPort>[ForwardedPort(hostPort, devicePort)]); + when(portForwarder.unforward(any)).thenAnswer((_) async => null); + }); - await createTestCommandRunner(command).run( - <String>['attach', '--debug-port', '$devicePort']); + testUsingContext('succeeds in ipv4 mode', () async { + testDeviceManager.addDevice(device); + final AttachCommand command = AttachCommand(); - verify(portForwarder.forward(devicePort)).called(1); - }, overrides: <Type, Generator>{ - FileSystem: () => testFileSystem, - },); + await createTestCommandRunner(command).run( + <String>['attach', '--debug-port', '$devicePort']); + + verify(portForwarder.forward(devicePort)).called(1); + }, overrides: <Type, Generator>{ + FileSystem: () => testFileSystem, + }); + + testUsingContext('succeeds in ipv6 mode', () async { + testDeviceManager.addDevice(device); + final AttachCommand command = AttachCommand(); + + await createTestCommandRunner(command).run( + <String>['attach', '--debug-port', '$devicePort', '--ipv6']); + + verify(portForwarder.forward(devicePort)).called(1); + }, overrides: <Type, Generator>{ + FileSystem: () => testFileSystem, + }); + + testUsingContext('skips in ipv4 mode with a provided observatory port', () async { + testDeviceManager.addDevice(device); + final AttachCommand command = AttachCommand(); + + await createTestCommandRunner(command).run( + <String>[ + 'attach', + '--debug-port', + '$devicePort', + '--observatory-port', + '$hostPort', + ], + ); + + verifyNever(portForwarder.forward(devicePort)); + }, overrides: <Type, Generator>{ + FileSystem: () => testFileSystem, + }); + + testUsingContext('skips in ipv6 mode with a provided observatory port', () async { + testDeviceManager.addDevice(device); + final AttachCommand command = AttachCommand(); + + await createTestCommandRunner(command).run( + <String>[ + 'attach', + '--debug-port', + '$devicePort', + '--observatory-port', + '$hostPort', + '--ipv6', + ], + ); + + verifyNever(portForwarder.forward(devicePort)); + }, overrides: <Type, Generator>{ + FileSystem: () => testFileSystem, + }); + }); testUsingContext('exits when no device connected', () async { final AttachCommand command = AttachCommand();