Remove Toolchain and extend ToolConfiguration to locate host tools such as sky_snapshot (#3870)

Host tools can be found in the artifact cache directory for the host platform.
If a developer wants to use a local engine build instead, then provide an
--engine-build flag that selects the specific engine build variant.

packages/flutter_tools/lib/src/android/android_device.dart

@@ -15,7 +15,6 @@ import '../device.dart';
 import '../flx.dart' as flx;
 import '../globals.dart';
 import '../service_protocol.dart';
-import '../toolchain.dart';
 import 'adb.dart';
 import 'android.dart';
 
@@ -322,8 +321,7 @@ class AndroidDevice extends Device {
 
   @override
   Future<LaunchResult> startApp(
-    ApplicationPackage package,
+    ApplicationPackage package, {
-    Toolchain toolchain, {
     String mainPath,
     String route,
     DebuggingOptions debuggingOptions,
@@ -333,7 +331,6 @@ class AndroidDevice extends Device {
       return new LaunchResult.failed();
 
     String localBundlePath = await flx.buildFlx(
-      toolchain,
       mainPath: mainPath,
       includeRobotoFonts: false
     );

packages/flutter_tools/lib/src/commands/build_apk.dart

@@ -17,7 +17,6 @@ import '../flx.dart' as flx;
 import '../globals.dart';
 import '../runner/flutter_command.dart';
 import '../services.dart';
-import '../toolchain.dart';
 import 'build_aot.dart';
 import 'run.dart';
 
@@ -242,7 +241,6 @@ class BuildApkCommand extends FlutterCommand {
     return await buildAndroid(
       TargetPlatform.android_arm,
       mode,
-      toolchain: toolchain,
       force: true,
       manifest: argResults['manifest'],
       resources: argResults['resources'],
@@ -457,7 +455,6 @@ bool _needsRebuild(String apkPath, String manifest) {
 Future<int> buildAndroid(
   TargetPlatform platform,
   BuildMode buildMode, {
-  Toolchain toolchain,
   bool force: false,
   String manifest: _kDefaultAndroidManifestPath,
   String resources,
@@ -515,7 +512,6 @@ Future<int> buildAndroid(
   } else {
     // Build the FLX.
     flxPath = await flx.buildFlx(
-      toolchain,
       mainPath: findMainDartFile(target),
       precompiledSnapshot: isAotBuildMode(buildMode),
       includeRobotoFonts: false);
@@ -556,8 +552,7 @@ Future<int> buildAndroid(
 }
 
 Future<int> buildApk(
-  TargetPlatform platform,
+  TargetPlatform platform, {
-  Toolchain toolchain, {
   String target,
   BuildMode buildMode: BuildMode.debug
 }) async {
@@ -569,7 +564,6 @@ Future<int> buildApk(
   int result = await buildAndroid(
     platform,
     buildMode,
-    toolchain: toolchain,
     force: false,
     target: target
   );

packages/flutter_tools/lib/src/commands/build_flx.dart

@@ -7,7 +7,6 @@ import 'dart:async';
 import '../flx.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
-import '../toolchain.dart';
 
 class BuildFlxCommand extends FlutterCommand {
   BuildFlxCommand() {
@@ -16,7 +15,6 @@ class BuildFlxCommand extends FlutterCommand {
     // This option is still referenced by the iOS build scripts. We should
     // remove it once we've updated those build scripts.
     argParser.addOption('asset-base', help: 'Ignored. Will be removed.', hide: true);
-    argParser.addOption('compiler');
     argParser.addOption('manifest', defaultsTo: defaultManifestPath);
     argParser.addOption('private-key', defaultsTo: defaultPrivateKeyPath);
     argParser.addOption('output-file', abbr: 'o', defaultsTo: defaultFlxOutputPath);
@@ -39,14 +37,9 @@ class BuildFlxCommand extends FlutterCommand {
 
   @override
   Future<int> runInProject() async {
-    String compilerPath = argResults['compiler'];
-    if (compilerPath != null)
-      toolchain = new Toolchain(compiler: new SnapshotCompiler(compilerPath));
-
     String outputPath = argResults['output-file'];
 
     return await build(
-      toolchain,
       mainPath: argResults['target'],
       manifestPath: argResults['manifest'],
       outputPath: outputPath,

packages/flutter_tools/lib/src/commands/daemon.dart

@@ -264,7 +264,6 @@ class AppDomain extends Domain {
     try {
       int result = await startApp(
         device,
-        command.toolchain,
         stop: true,
         target: args['target'],
         route: args['route']

packages/flutter_tools/lib/src/commands/drive.dart

@@ -248,7 +248,6 @@ Future<int> startApp(DriveCommand command, BuildMode buildMode) async {
     printTrace('Building an APK.');
     int result = await build_apk.buildApk(
       command.device.platform,
-      command.toolchain,
       target: command.target
     );
 
@@ -267,7 +266,6 @@ Future<int> startApp(DriveCommand command, BuildMode buildMode) async {
   printTrace('Starting application.');
   LaunchResult result = await command.device.startApp(
     package,
-    command.toolchain,
     mainPath: mainPath,
     route: command.route,
     debuggingOptions: new DebuggingOptions.enabled(

packages/flutter_tools/lib/src/commands/listen.dart

@@ -60,7 +60,6 @@ class ListenCommand extends RunCommandBase {
 
       result = await startApp(
         deviceForCommand,
-        toolchain,
         target: target,
         install: firstTime,
         stop: true,

packages/flutter_tools/lib/src/commands/refresh.dart

@@ -9,6 +9,7 @@ import 'package:path/path.dart' as path;
 
 import '../android/android_device.dart';
 import '../application_package.dart';
+import '../flx.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
 
@@ -39,7 +40,7 @@ class RefreshCommand extends FlutterCommand {
     try {
       String snapshotPath = path.join(tempDir.path, 'snapshot_blob.bin');
 
-      int result = await toolchain.compiler.createSnapshot(
+      int result = await createSnapshot(
         mainPath: argResults['target'],
         snapshotPath: snapshotPath
       );

packages/flutter_tools/lib/src/commands/run.dart

@@ -15,7 +15,6 @@ import '../build_configuration.dart';
 import '../device.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
-import '../toolchain.dart';
 import 'build_apk.dart';
 import 'install.dart';
 
@@ -118,7 +117,6 @@ class RunCommand extends RunCommandBase {
     if (argResults['resident']) {
       _RunAndStayResident runner = new _RunAndStayResident(
         deviceForCommand,
-        toolchain,
         target: target,
         debuggingOptions: options,
         traceStartup: traceStartup,
@@ -129,7 +127,6 @@ class RunCommand extends RunCommandBase {
     } else {
       return startApp(
         deviceForCommand,
-        toolchain,
         target: target,
         stop: argResults['full-restart'],
         install: true,
@@ -143,8 +140,7 @@ class RunCommand extends RunCommandBase {
 }
 
 Future<int> startApp(
-  Device device,
+  Device device, {
-  Toolchain toolchain, {
   String target,
   bool stop: true,
   bool install: true,
@@ -179,7 +175,6 @@ Future<int> startApp(
 
     int result = await buildApk(
       device.platform,
-      toolchain,
       target: target,
       buildMode: buildMode
     );
@@ -220,7 +215,6 @@ Future<int> startApp(
 
   LaunchResult result = await device.startApp(
     package,
-    toolchain,
     mainPath: mainPath,
     route: route,
     debuggingOptions: debuggingOptions,
@@ -349,8 +343,7 @@ String _getDisplayPath(String fullPath) {
 
 class _RunAndStayResident {
   _RunAndStayResident(
-    this.device,
+    this.device, {
-    this.toolchain, {
     this.target,
     this.debuggingOptions,
     this.traceStartup : false,
@@ -358,7 +351,6 @@ class _RunAndStayResident {
   });
 
   final Device device;
-  final Toolchain toolchain;
   final String target;
   final DebuggingOptions debuggingOptions;
   final bool traceStartup;
@@ -399,7 +391,6 @@ class _RunAndStayResident {
 
       int result = await buildApk(
         device.platform,
-        toolchain,
         target: target,
         buildMode: buildMode
       );
@@ -438,7 +429,6 @@ class _RunAndStayResident {
 
     LaunchResult result = await device.startApp(
       package,
-      toolchain,
       mainPath: mainPath,
       debuggingOptions: debuggingOptions,
       platformArgs: platformArgs

packages/flutter_tools/lib/src/commands/run_mojo.dart

@@ -167,7 +167,6 @@ class RunMojoCommand extends FlutterCommand {
       String mainPath = findMainDartFile(argResults['target']);
 
       int result = await flx.build(
-        toolchain,
         mainPath: mainPath,
         outputPath: targetApp
       );

packages/flutter_tools/lib/src/device.dart

@@ -18,7 +18,6 @@ import 'build_configuration.dart';
 import 'globals.dart';
 import 'ios/devices.dart';
 import 'ios/simulators.dart';
-import 'toolchain.dart';
 
 /// A class to get all available devices.
 class DeviceManager {
@@ -181,8 +180,7 @@ abstract class Device {
   /// [platformArgs] allows callers to pass platform-specific arguments to the
   /// start call.
   Future<LaunchResult> startApp(
-    ApplicationPackage package,
+    ApplicationPackage package, {
-    Toolchain toolchain, {
     String mainPath,
     String route,
     DebuggingOptions debuggingOptions,

packages/flutter_tools/lib/src/flx.dart

@@ -14,6 +14,7 @@ import 'package:yaml/yaml.dart';
 
 import 'artifacts.dart';
 import 'base/file_system.dart' show ensureDirectoryExists;
+import 'base/process.dart';
 import 'globals.dart';
 import 'package_map.dart';
 import 'toolchain.dart';
@@ -33,6 +34,28 @@ const String _kSnapshotKey = 'snapshot_blob.bin';
 const String _kFontSetMaterial = 'material';
 const String _kFontSetRoboto = 'roboto';
 
+Future<int> createSnapshot({
+  String mainPath,
+  String snapshotPath,
+  String depfilePath,
+  String buildOutputPath
+}) {
+  assert(mainPath != null);
+  assert(snapshotPath != null);
+
+  final List<String> args = <String>[
+    tools.getHostToolPath(HostTool.SkySnapshot),
+    mainPath,
+    '--packages=${PackageMap.instance.packagesPath}',
+    '--snapshot=$snapshotPath'
+  ];
+  if (depfilePath != null)
+    args.add('--depfile=$depfilePath');
+  if (buildOutputPath != null)
+    args.add('--build-output=$buildOutputPath');
+  return runCommandAndStreamOutput(args);
+}
+
 class _Asset {
   _Asset({ this.base, String assetEntry, this.relativePath, this.source }) {
     this._assetEntry = assetEntry;
@@ -259,8 +282,7 @@ ZipEntry _createFontManifest(Map<String, dynamic> manifestDescriptor,
 /// Build the flx in the build/ directory and return `localBundlePath` on success.
 ///
 /// Return `null` on failure.
-Future<String> buildFlx(
+Future<String> buildFlx({
-  Toolchain toolchain, {
   String mainPath: defaultMainPath,
   bool precompiledSnapshot: false,
   bool includeRobotoFonts: true
@@ -269,7 +291,6 @@ Future<String> buildFlx(
   String localBundlePath = path.join('build', 'app.flx');
   String localSnapshotPath = path.join('build', 'snapshot_blob.bin');
   result = await build(
-    toolchain,
     snapshotPath: localSnapshotPath,
     outputPath: localBundlePath,
     mainPath: mainPath,
@@ -292,8 +313,7 @@ class DirectoryResult {
   }
 }
 
-Future<int> build(
+Future<int> build({
-  Toolchain toolchain, {
   String mainPath: defaultMainPath,
   String manifestPath: defaultManifestPath,
   String outputPath: defaultFlxOutputPath,
@@ -321,7 +341,7 @@ Future<int> build(
 
     // In a precompiled snapshot, the instruction buffer contains script
     // content equivalents
-    int result = await toolchain.compiler.createSnapshot(
+    int result = await createSnapshot(
       mainPath: mainPath,
       snapshotPath: snapshotPath,
       depfilePath: depfilePath

packages/flutter_tools/lib/src/ios/devices.dart

@@ -14,7 +14,6 @@ import '../base/process.dart';
 import '../build_configuration.dart';
 import '../device.dart';
 import '../globals.dart';
-import '../toolchain.dart';
 import 'mac.dart';
 
 const String _ideviceinstallerInstructions =
@@ -154,8 +153,7 @@ class IOSDevice extends Device {
 
   @override
   Future<LaunchResult> startApp(
-    ApplicationPackage app,
+    ApplicationPackage app, {
-    Toolchain toolchain, {
     String mainPath,
     String route,
     DebuggingOptions debuggingOptions,

packages/flutter_tools/lib/src/ios/simulators.dart

@@ -16,7 +16,6 @@ import '../device.dart';
 import '../flx.dart' as flx;
 import '../globals.dart';
 import '../service_protocol.dart';
-import '../toolchain.dart';
 import 'mac.dart';
 
 const String _xcrunPath = '/usr/bin/xcrun';
@@ -437,8 +436,7 @@ class IOSSimulator extends Device {
 
   @override
   Future<LaunchResult> startApp(
-    ApplicationPackage app,
+    ApplicationPackage app, {
-    Toolchain toolchain, {
     String mainPath,
     String route,
     DebuggingOptions debuggingOptions,
@@ -446,7 +444,7 @@ class IOSSimulator extends Device {
   }) async {
     printTrace('Building ${app.name} for $id.');
 
-    if (!(await _setupUpdatedApplicationBundle(app, toolchain)))
+    if (!(await _setupUpdatedApplicationBundle(app)))
       return new LaunchResult.failed();
 
     ServiceProtocolDiscovery observatoryDiscovery;
@@ -524,8 +522,8 @@ class IOSSimulator extends Device {
     return isInstalled && isRunning;
   }
 
-  Future<bool> _setupUpdatedApplicationBundle(ApplicationPackage app, Toolchain toolchain) async {
+  Future<bool> _setupUpdatedApplicationBundle(ApplicationPackage app) async {
-    bool sideloadResult = await _sideloadUpdatedAssetsForInstalledApplicationBundle(app, toolchain);
+    bool sideloadResult = await _sideloadUpdatedAssetsForInstalledApplicationBundle(app);
 
     if (!sideloadResult)
       return false;
@@ -558,8 +556,8 @@ class IOSSimulator extends Device {
   }
 
   Future<bool> _sideloadUpdatedAssetsForInstalledApplicationBundle(
-      ApplicationPackage app, Toolchain toolchain) async {
+      ApplicationPackage app) async {
-    return (await flx.build(toolchain, precompiledSnapshot: true)) == 0;
+    return (await flx.build(precompiledSnapshot: true)) == 0;
   }
 
   @override

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -14,7 +14,6 @@ import '../device.dart';
 import '../flx.dart' as flx;
 import '../globals.dart';
 import '../package_map.dart';
-import '../toolchain.dart';
 import '../usage.dart';
 import 'flutter_command_runner.dart';
 
@@ -86,10 +85,6 @@ abstract class FlutterCommand extends Command {
     return mode;
   }
 
-  void _setupToolchain() {
-    toolchain ??= Toolchain.forConfigs(buildConfigurations);
-  }
-
   void _setupApplicationPackages() {
     applicationPackages ??= new ApplicationPackageStore();
   }
@@ -166,7 +161,6 @@ abstract class FlutterCommand extends Command {
     if (flutterUsage.isFirstRun)
       flutterUsage.printUsage();
 
-    _setupToolchain();
     _setupApplicationPackages();
 
     String commandPath = usagePath;
@@ -215,5 +209,4 @@ abstract class FlutterCommand extends Command {
   Device get deviceForCommand => _deviceForCommand;
 
   ApplicationPackageStore applicationPackages;
-  Toolchain toolchain;
 }

packages/flutter_tools/lib/src/runner/flutter_command_runner.dart

@@ -73,6 +73,7 @@ class FlutterCommandRunner extends CommandRunner {
         help:
             'Set this if you are building Flutter locally and want to use the release build products.\n'
             'The --engine-release option is not compatible with the listen command on iOS devices and simulators.');
+
     argParser.addOption('engine-src-path',
         hide: !verboseHelp,
         help:
@@ -81,6 +82,13 @@ class FlutterCommandRunner extends CommandRunner {
             'dependency_overrides for $kFlutterEnginePackageName, if any, or, failing that, tries to guess at the location\n'
             'based on the value of the --flutter-root option.');
 
+    argParser.addOption('local-engine',
+        hide: !verboseHelp,
+        help:
+            'Name of a build output within the engine out directory, if you are building Flutter locally.\n'
+            'Use this to select a specific version of the engine if you have built multiple engine targets.\n'
+            'This path is relative to --engine-src-path/out.');
+
     argParser.addOption('host-debug-build-path',
         hide: !verboseHelp,
         help:
@@ -220,6 +228,7 @@ class FlutterCommandRunner extends CommandRunner {
     // Set up the tooling configuration.
     if (enginePath != null) {
       ToolConfiguration.instance.engineSrcPath = enginePath;
+      ToolConfiguration.instance.engineBuildPath = _findEngineBuildPath(globalResults, enginePath);
 
       if (globalResults.wasParsed('engine-release'))
         ToolConfiguration.instance.engineRelease = globalResults['engine-release'];
@@ -287,6 +296,35 @@ class FlutterCommandRunner extends CommandRunner {
     return engineSourcePath;
   }
 
+  String _findEngineBuildPath(ArgResults globalResults, String enginePath) {
+    String localEngine;
+    if (globalResults['local-engine'] != null) {
+      localEngine = globalResults['local-engine'];
+    } else {
+      // This is a temporary hack to find an engine build in the same way that we found the toolchain.
+      // TODO(jsimmons): delete this and make --local-engine mandatory when BuildConfigurations are removed.
+      for (BuildConfiguration config in buildConfigurations) {
+        if (FileSystemEntity.isDirectorySync(config.buildDir)) {
+          localEngine = path.basename(config.buildDir);
+          break;
+        }
+      }
+
+      if (localEngine == null) {
+        printError('You must specify --local-engine if you are using a locally built engine.');
+        throw new ProcessExit(2);
+      }
+    }
+
+    String engineBuildPath = path.join(enginePath, 'out', localEngine);
+    if (!FileSystemEntity.isDirectorySync(engineBuildPath)) {
+      printError('No Flutter engine build found at $engineBuildPath.');
+      throw new ProcessExit(2);
+    }
+
+    return engineBuildPath;
+  }
+
   List<BuildConfiguration> _createBuildConfigurations(ArgResults globalResults) {
     bool isDebug = globalResults['engine-debug'];
     bool isRelease = globalResults['engine-release'];

packages/flutter_tools/lib/src/toolchain.dart

@@ -2,77 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
 import 'dart:io';
 
 import 'package:path/path.dart' as path;
 
-import 'artifacts.dart';
 import 'base/context.dart';
-import 'base/process.dart';
 import 'build_configuration.dart';
 import 'cache.dart';
 import 'globals.dart';
-import 'package_map.dart';
 
-class SnapshotCompiler {
+enum HostTool {
-  SnapshotCompiler(this._path);
+  SkySnapshot,
-
-  final String _path;
-
-  Future<int> createSnapshot({
-    String mainPath,
-    String snapshotPath,
-    String depfilePath,
-    String buildOutputPath
-  }) {
-    assert(mainPath != null);
-    assert(snapshotPath != null);
-
-    final List<String> args = <String>[
-      _path,
-      mainPath,
-      '--packages=${PackageMap.instance.packagesPath}',
-      '--snapshot=$snapshotPath'
-    ];
-    if (depfilePath != null)
-      args.add('--depfile=$depfilePath');
-    if (buildOutputPath != null)
-      args.add('--build-output=$buildOutputPath');
-    return runCommandAndStreamOutput(args);
-  }
-}
-
-// TODO(devoncarew): This should instead take a host platform and target platform.
-
-String _getCompilerPath(BuildConfiguration config) {
-  if (config.type != BuildType.prebuilt) {
-    String compilerPath = path.join(config.buildDir, 'clang_x64', 'sky_snapshot');
-    if (FileSystemEntity.isFileSync(compilerPath))
-      return compilerPath;
-    compilerPath = path.join(config.buildDir, 'sky_snapshot');
-    if (FileSystemEntity.isFileSync(compilerPath))
-      return compilerPath;
-    return null;
-  }
-  Artifact artifact = ArtifactStore.getArtifact(
-    type: ArtifactType.snapshot, hostPlatform: config.hostPlatform);
-  return ArtifactStore.getPath(artifact);
-}
-
-class Toolchain {
-  Toolchain({ this.compiler });
-
-  final SnapshotCompiler compiler;
-
-  static Toolchain forConfigs(List<BuildConfiguration> configs) {
-    for (BuildConfiguration config in configs) {
-      String compilerPath = _getCompilerPath(config);
-      if (compilerPath != null)
-        return new Toolchain(compiler: new SnapshotCompiler(compilerPath));
-    }
-    return null;
-  }
 }
 
 /// A ToolConfiguration can return the tools directory for the current host platform
@@ -95,6 +35,9 @@ class ToolConfiguration {
   /// Override using the artifacts from the cache directory (--engine-src-path).
   String engineSrcPath;
 
+  /// Path to a local engine build acting as a source for artifacts (--local-engine).
+  String engineBuildPath;
+
   /// The engine mode to use (only relevent when [engineSrcPath] is set).
   bool engineRelease;
 
@@ -159,4 +102,17 @@ class ToolConfiguration {
       return new Directory(path.join(engineDir.path, dirName));
     }
   }
+
+  String getHostToolPath(HostTool tool) {
+    if (tool != HostTool.SkySnapshot)
+      throw new Exception('Unexpected host tool: $tool');
+
+    if (isLocalEngine) {
+      return path.join(engineBuildPath, 'clang_x64', 'sky_snapshot');
+    } else {
+      return path.join(_cache.getArtifactDirectory('engine').path,
+                       getNameForHostPlatform(getCurrentHostPlatform()),
+                       'sky_snapshot');
+    }
+  }
 }

packages/flutter_tools/test/src/mocks.dart

@@ -11,7 +11,6 @@ import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/ios/devices.dart';
 import 'package:flutter_tools/src/ios/simulators.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
-import 'package:flutter_tools/src/toolchain.dart';
 import 'package:mockito/mockito.dart';
 
 class MockApplicationPackageStore extends ApplicationPackageStore {
@@ -28,13 +27,6 @@ class MockApplicationPackageStore extends ApplicationPackageStore {
   );
 }
 
-class MockSnapshotCompiler extends Mock implements SnapshotCompiler {
-}
-
-class MockToolchain extends Toolchain {
-  MockToolchain() : super(compiler: new MockSnapshotCompiler());
-}
-
 class MockAndroidDevice extends Mock implements AndroidDevice {
   @override
   TargetPlatform get platform => TargetPlatform.android_arm;
@@ -74,6 +66,5 @@ class MockDeviceLogReader extends DeviceLogReader {
 void applyMocksToCommand(FlutterCommand command) {
   command
     ..applicationPackages = new MockApplicationPackageStore()
-    ..toolchain = new MockToolchain()
     ..commandValidator = () => true;
 }