abstract some OS operations

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);
       }

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';
 

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'
         )
       )
     );

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');

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']);
-    }
+    osUtils.killTcpPortListeners(_serverPort);
 
     return true;
   }

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']);
+  }
+}

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';

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();

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);
+    });
+  });
+}