From 5dc4a7cce4ce97c7b62870f54191566cfa728db6 Mon Sep 17 00:00:00 2001 From: Devon Carew <devoncarew@gmail.com> Date: Mon, 2 Nov 2015 01:50:40 -0800 Subject: [PATCH] abstract some OS operations --- packages/flutter_tools/lib/src/artifacts.dart | 19 ++-- .../flutter_tools/lib/src/commands/build.dart | 4 +- .../flutter_tools/lib/src/commands/init.dart | 4 +- .../lib/src/commands/run_mojo.dart | 2 +- packages/flutter_tools/lib/src/device.dart | 59 ++---------- packages/flutter_tools/lib/src/os_utils.dart | 91 +++++++++++++++++++ packages/flutter_tools/lib/src/process.dart | 2 - packages/flutter_tools/test/all.dart | 2 + .../flutter_tools/test/os_utils_test.dart | 66 ++++++++++++++ 9 files changed, 185 insertions(+), 64 deletions(-) create mode 100644 packages/flutter_tools/lib/src/os_utils.dart create mode 100644 packages/flutter_tools/test/os_utils_test.dart diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart index 5e39e47d27..24d97b7555 100644 --- a/packages/flutter_tools/lib/src/artifacts.dart +++ b/packages/flutter_tools/lib/src/artifacts.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -library sky_tools.artifacts; - import 'dart:async'; import 'dart:io'; @@ -11,6 +9,7 @@ import 'package:logging/logging.dart'; import 'package:path/path.dart' as path; import 'build_configuration.dart'; +import 'os_utils.dart'; final Logger _logging = new Logger('sky_tools.artifacts'); @@ -83,7 +82,11 @@ class Artifact { } String getUrl(String revision) { - return _getCloudStorageBaseUrl(category: category, platform: platform, revision: revision) + fileName; + return _getCloudStorageBaseUrl( + category: category, + platform: platform, + revision: revision + ) + fileName; } // Whether the artifact needs to be marked as executable on disk. @@ -164,7 +167,11 @@ class ArtifactStore { } static String getCloudStorageBaseUrl(String category, String platform) { - return _getCloudStorageBaseUrl(category: category, platform: platform, revision: engineRevision); + return _getCloudStorageBaseUrl( + category: category, + platform: platform, + revision: engineRevision + ); } static Future _downloadFile(String url, File file) async { @@ -208,9 +215,7 @@ class ArtifactStore { print('Downloading ${artifact.name} from the cloud, one moment please...'); await _downloadFile(artifact.getUrl(engineRevision), cachedFile); if (artifact.executable) { - // TODO(abarth): We should factor this out into a separate function that - // can have a platform-specific implementation. - ProcessResult result = Process.runSync('chmod', ['u+x', cachedFile.path]); + ProcessResult result = osUtils.makeExecutable(cachedFile); if (result.exitCode != 0) throw new Exception(result.stderr); } diff --git a/packages/flutter_tools/lib/src/commands/build.dart b/packages/flutter_tools/lib/src/commands/build.dart index 6abcf13cdf..62f1880c34 100644 --- a/packages/flutter_tools/lib/src/commands/build.dart +++ b/packages/flutter_tools/lib/src/commands/build.dart @@ -7,10 +7,10 @@ import 'dart:io'; import 'dart:typed_data'; import 'package:archive/archive.dart'; -import 'package:yaml/yaml.dart'; - import 'package:flx/bundle.dart'; import 'package:flx/signing.dart'; +import 'package:yaml/yaml.dart'; + import '../toolchain.dart'; import 'flutter_command.dart'; diff --git a/packages/flutter_tools/lib/src/commands/init.dart b/packages/flutter_tools/lib/src/commands/init.dart index 0290d1e093..756dc9562b 100644 --- a/packages/flutter_tools/lib/src/commands/init.dart +++ b/packages/flutter_tools/lib/src/commands/init.dart @@ -158,10 +158,10 @@ class FlutterDemo extends StatelessComponent { child: new Center( child: new Text("Hello world!") ) - ), + ), floatingActionButton: new FloatingActionButton( child: new Icon( - type: 'content/add' + icon: 'content/add' ) ) ); diff --git a/packages/flutter_tools/lib/src/commands/run_mojo.dart b/packages/flutter_tools/lib/src/commands/run_mojo.dart index 5853204ee7..3cbd597dfd 100644 --- a/packages/flutter_tools/lib/src/commands/run_mojo.dart +++ b/packages/flutter_tools/lib/src/commands/run_mojo.dart @@ -9,8 +9,8 @@ import 'package:args/command_runner.dart'; import 'package:logging/logging.dart'; import 'package:path/path.dart' as path; -import '../build_configuration.dart'; import '../artifacts.dart'; +import '../build_configuration.dart'; import '../process.dart'; final Logger _logging = new Logger('sky_tools.run_mojo'); diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index fa13654a22..5fadb74871 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -2,19 +2,18 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -library sky_tools.device; - import 'dart:async'; import 'dart:convert'; import 'dart:io'; import 'dart:math'; +import 'package:crypto/crypto.dart'; import 'package:logging/logging.dart'; import 'package:path/path.dart' as path; -import 'package:crypto/crypto.dart'; import 'application_package.dart'; import 'build_configuration.dart'; +import 'os_utils.dart'; import 'process.dart'; final Logger _logging = new Logger('sky_tools.device'); @@ -507,8 +506,8 @@ class IOSSimulator extends Device { class AndroidDevice extends Device { static const String _ADB_PATH = 'adb'; - static const String _observatoryPort = '8181'; - static const String _serverPort = '9888'; + static const int _observatoryPort = 8181; + static const int _serverPort = 9888; static const String className = 'AndroidDevice'; static final String defaultDeviceID = 'default_android_device'; @@ -756,9 +755,8 @@ class AndroidDevice extends Device { void _forwardObservatoryPort() { // Set up port forwarding for observatory. - String observatoryPortString = 'tcp:$_observatoryPort'; - runCheckedSync( - [adbPath, 'forward', observatoryPortString, observatoryPortString]); + String portString = 'tcp:$_observatoryPort'; + runCheckedSync([adbPath, 'forward', portString, portString]); } bool startBundle(AndroidApk apk, String bundlePath, bool poke, bool checked) { @@ -812,7 +810,7 @@ class AndroidDevice extends Device { // Actually start the server. Process server = await Process.start( - sdkBinaryName('pub'), ['run', 'sky_tools:sky_server', _serverPort], + sdkBinaryName('pub'), ['run', 'sky_tools:sky_server', _serverPort.toString()], workingDirectory: serverRoot, mode: ProcessStartMode.DETACHED_WITH_STDIO ); @@ -859,48 +857,9 @@ class AndroidDevice extends Device { runSync([adbPath, 'reverse', '--remove', 'tcp:$_serverPort']); // Stop the app runSync([adbPath, 'shell', 'am', 'force-stop', apk.id]); - // Kill the server - if (Platform.isMacOS) { - String pids = runSync(['lsof', '-i', ':$_serverPort', '-t']).trim(); - if (pids.isEmpty) { - _logging.fine('No process to kill for port $_serverPort'); - return true; - } - // Handle multiple returned pids. - for (String pidString in pids.split('\n')) { - // Killing a pid with a shell command from within dart is hard, so use a - // library command, but it's still nice to give the equivalent command - // when doing verbose logging. - _logging.info('kill $pidString'); - - int pid = int.parse(pidString, onError: (_) => null); - if (pid != null) - Process.killPid(pid); - } - } else if (Platform.isWindows) { - //Get list of network processes and split on newline - List<String> processes = runSync(['netstat.exe','-ano']).split("\r"); - - //List entries from netstat is formatted like so - // TCP 192.168.2.11:50945 192.30.252.90:443 LISTENING 1304 - //This regexp is to find process where the the port exactly matches - RegExp pattern = new RegExp(':$_serverPort[ ]+'); - - //Split the columns by 1 or more spaces - RegExp columnPattern = new RegExp('[ ]+'); - processes.forEach((String process) { - if (process.contains(pattern)) { - //The last column is the Process ID - String processId = process.split(columnPattern).last; - //Force and Tree kill the process - _logging.info('kill $processId'); - runSync(['TaskKill.exe', '/F', '/T', '/PID', processId]); - } - }); - } else { - runSync(['fuser', '-k', '$_serverPort/tcp']); - } + // Kill the server + osUtils.killTcpPortListeners(_serverPort); return true; } diff --git a/packages/flutter_tools/lib/src/os_utils.dart b/packages/flutter_tools/lib/src/os_utils.dart new file mode 100644 index 0000000000..caf124d0a9 --- /dev/null +++ b/packages/flutter_tools/lib/src/os_utils.dart @@ -0,0 +1,91 @@ +// Copyright 2015 The Chromium 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:io'; + +import 'package:logging/logging.dart'; + +import 'process.dart'; + +final OperatingSystemUtils osUtils = new OperatingSystemUtils._(); + +final Logger _logging = new Logger('sky_tools.os'); + +abstract class OperatingSystemUtils { + factory OperatingSystemUtils._() { + if (Platform.isWindows) { + return new _WindowsUtils(); + } else if (Platform.isMacOS) { + return new _MacUtils(); + } else { + return new _LinuxUtils(); + } + } + + /// Make the given file executable. This may be a no-op on some platforms. + ProcessResult makeExecutable(File file); + + /// A best-effort attempt to kill all listeners on the given TCP port. + void killTcpPortListeners(int tcpPort); +} + +abstract class _PosixUtils implements OperatingSystemUtils { + ProcessResult makeExecutable(File file) { + return Process.runSync('chmod', ['u+x', file.path]); + } +} + +class _WindowsUtils implements OperatingSystemUtils { + // This is a no-op. + ProcessResult makeExecutable(File file) { + return new ProcessResult(0, 0, null, null); + } + + void killTcpPortListeners(int tcpPort) { + // Get list of network processes and split on newline + List<String> processes = runSync(['netstat.exe','-ano']).split("\r"); + + // List entries from netstat is formatted like so: + // TCP 192.168.2.11:50945 192.30.252.90:443 LISTENING 1304 + // This regexp is to find process where the the port exactly matches + RegExp pattern = new RegExp(':$tcpPort[ ]+'); + + // Split the columns by 1 or more spaces + RegExp columnPattern = new RegExp('[ ]+'); + processes.forEach((String process) { + if (process.contains(pattern)) { + // The last column is the Process ID + String processId = process.split(columnPattern).last; + // Force and Tree kill the process + _logging.info('kill $processId'); + runSync(['TaskKill.exe', '/F', '/T', '/PID', processId]); + } + }); + } +} + +class _MacUtils extends _PosixUtils { + void killTcpPortListeners(int tcpPort) { + String pids = runSync(['lsof', '-i', ':$tcpPort', '-t']).trim(); + if (pids.isNotEmpty) { + // Handle multiple returned pids. + for (String pidString in pids.split('\n')) { + // Killing a pid with a shell command from within dart is hard, so use a + // library command, but it's still nice to give the equivalent command + // when doing verbose logging. + _logging.info('kill $pidString'); + + int pid = int.parse(pidString, onError: (_) => null); + if (pid != null) + Process.killPid(pid); + } + } + } +} + +class _LinuxUtils extends _PosixUtils { + void killTcpPortListeners(int tcpPort) { + runSync(['fuser', '-k', '$tcpPort/tcp']); + } +} diff --git a/packages/flutter_tools/lib/src/process.dart b/packages/flutter_tools/lib/src/process.dart index 2332b58aab..2fd8a7a360 100644 --- a/packages/flutter_tools/lib/src/process.dart +++ b/packages/flutter_tools/lib/src/process.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -library sky_tools.process; - import 'dart:async'; import 'dart:convert'; import 'dart:io'; diff --git a/packages/flutter_tools/test/all.dart b/packages/flutter_tools/test/all.dart index 0b2dcfe97f..91450afa86 100644 --- a/packages/flutter_tools/test/all.dart +++ b/packages/flutter_tools/test/all.dart @@ -8,6 +8,7 @@ import 'install_test.dart' as install_test; import 'listen_test.dart' as listen_test; import 'list_test.dart' as list_test; import 'logs_test.dart' as logs_test; +import 'os_utils_test.dart' as os_utils_test; import 'start_test.dart' as start_test; import 'stop_test.dart' as stop_test; import 'trace_test.dart' as trace_test; @@ -19,6 +20,7 @@ main() { listen_test.defineTests(); list_test.defineTests(); logs_test.defineTests(); + os_utils_test.defineTests(); start_test.defineTests(); stop_test.defineTests(); trace_test.defineTests(); diff --git a/packages/flutter_tools/test/os_utils_test.dart b/packages/flutter_tools/test/os_utils_test.dart new file mode 100644 index 0000000000..999fc9cc5a --- /dev/null +++ b/packages/flutter_tools/test/os_utils_test.dart @@ -0,0 +1,66 @@ +// Copyright 2015 The Chromium 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:io'; + +import 'package:sky_tools/src/os_utils.dart'; +import 'package:test/test.dart'; +import 'package:path/path.dart' as p; + +main() => defineTests(); + +defineTests() { + group('OperatingSystemUtils', () { + Directory temp; + + setUp(() { + temp = Directory.systemTemp.createTempSync('sky_tools'); + }); + + tearDown(() { + temp.deleteSync(recursive: true); + }); + + test('makeExecutable', () { + File file = new File(p.join(temp.path, 'foo.script')); + file.writeAsStringSync('hello world'); + osUtils.makeExecutable(file); + + // Skip this test on windows. + if (!Platform.isWindows) { + String mode = file.statSync().modeString(); + // rwxr--r-- + expect(mode.substring(0, 3), endsWith('x')); + } + }); + + /// Start a script listening on a port, try and kill that process. + test('killTcpPortListeners', () async { + final int port = 40170; + + File file = new File(p.join(temp.path, 'script.dart')); + file.writeAsStringSync(''' +import 'dart:io'; + +void main() async { + ServerSocket serverSocket = await ServerSocket.bind( + InternetAddress.LOOPBACK_IP_V4, ${port}); + // wait... + print('listening on port ${port}...'); +} +'''); + Process process = await Process.start('dart', [file.path]); + await process.stdout.first; + + osUtils.killTcpPortListeners(40170); + int exitCode = await process.exitCode; + expect(exitCode, isNot(equals(0))); + }); + + /// Try and kill with a port that no process is listening to. + test('killTcpPortListeners none', () { + osUtils.killTcpPortListeners(40171); + }); + }); +} -- 2.21.0