[flutter_tools] refactor engine locator to its own class (#67274)

Make the logic for locating a local engine path part of its own class, add documentation, and update tests to cover engine source path locating too.

#47161

packages/flutter_tools/lib/src/context_runner.dart

@@ -47,6 +47,7 @@ import 'mdns_discovery.dart';
 import 'persistent_tool_state.dart';
 import 'reporting/reporting.dart';
 import 'run_hot.dart';
+import 'runner/local_engine.dart';
 import 'version.dart';
 import 'web/workflow.dart';
 import 'windows/visual_studio.dart';
@@ -183,6 +184,13 @@ Future<T> runInContext<T>(
         xcode: globals.xcode,
         platform: globals.platform,
       ),
+      LocalEngineLocator: () => LocalEngineLocator(
+        userMessages: userMessages,
+        logger: globals.logger,
+        platform: globals.platform,
+        fileSystem: globals.fs,
+        flutterRoot: Cache.flutterRoot,
+      ),
       Logger: () => globals.platform.isWindows
         ? WindowsStdoutLogger(
             terminal: globals.terminal,

packages/flutter_tools/lib/src/globals.dart

@@ -36,6 +36,7 @@ import 'macos/xcode.dart';
 import 'persistent_tool_state.dart';
 import 'project.dart';
 import 'reporting/reporting.dart';
+import 'runner/local_engine.dart';
 import 'version.dart';
 
 Artifacts get artifacts => context.get<Artifacts>();
@@ -59,6 +60,8 @@ FlutterProjectFactory get projectFactory {
   );
 }
 
+LocalEngineLocator get localEngineLocator => context.get<LocalEngineLocator>();
+
 /// Currently active implementation of the file system.
 ///
 /// By default it uses local disk-based implementation. Override this in tests

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

@@ -7,7 +7,6 @@ import 'package:args/command_runner.dart';
 import 'package:completion/completion.dart';
 import 'package:file/file.dart';
 import 'package:meta/meta.dart';
-import 'package:package_config/package_config.dart';
 
 import '../artifacts.dart';
 import '../base/common.dart';
@@ -269,10 +268,13 @@ class FlutterCommandRunner extends CommandRunner<void> {
     Cache.flutterRoot = globals.fs.path.normalize(globals.fs.path.absolute(flutterRoot));
 
     // Set up the tooling configuration.
-    final String enginePath = await _findEnginePath(topLevelResults);
-    if (enginePath != null) {
+    final EngineBuildPaths engineBuildPaths = await globals.localEngineLocator.findEnginePath(
+      topLevelResults['local-engine-src-path'] as String,
+      topLevelResults['local-engine'] as String
+    );
+    if (engineBuildPaths != null) {
       contextOverrides.addAll(<Type, dynamic>{
-        Artifacts: Artifacts.getLocalEngine(_findEngineBuildPath(topLevelResults, enginePath)),
+        Artifacts: Artifacts.getLocalEngine(engineBuildPaths),
       });
     }
 
@@ -335,92 +337,6 @@ class FlutterCommandRunner extends CommandRunner<void> {
     );
   }
 
-  String _tryEnginePath(String enginePath) {
-    if (globals.fs.isDirectorySync(globals.fs.path.join(enginePath, 'out'))) {
-      return enginePath;
-    }
-    return null;
-  }
-
-  Future<String> _findEnginePath(ArgResults globalResults) async {
-    String engineSourcePath = globalResults['local-engine-src-path'] as String
-      ?? globals.platform.environment[kFlutterEngineEnvironmentVariableName];
-
-    if (engineSourcePath == null && globalResults['local-engine'] != null) {
-      try {
-        final PackageConfig packageConfig = await loadPackageConfigWithLogging(
-          globals.fs.file(globalPackagesPath),
-          logger: globals.logger,
-          throwOnError: false,
-        );
-        Uri engineUri = packageConfig[kFlutterEnginePackageName]?.packageUriRoot;
-        // Skip if sky_engine is the self-contained one.
-        if (engineUri != null && globals.fs.identicalSync(globals.fs.path.join(Cache.flutterRoot, 'bin', 'cache', 'pkg', kFlutterEnginePackageName, 'lib'), engineUri.path)) {
-          engineUri = null;
-        }
-        // If sky_engine is specified and the engineSourcePath not set, try to determine the engineSourcePath by sky_engine setting.
-        // A typical engineUri looks like: file://flutter-engine-local-path/src/out/host_debug_unopt/gen/dart-pkg/sky_engine/lib/
-        if (engineUri?.path != null) {
-          engineSourcePath = globals.fs.directory(engineUri.path)?.parent?.parent?.parent?.parent?.parent?.parent?.path;
-          if (engineSourcePath != null && (engineSourcePath == globals.fs.path.dirname(engineSourcePath) || engineSourcePath.isEmpty)) {
-            engineSourcePath = null;
-            throwToolExit(userMessages.runnerNoEngineSrcDir(kFlutterEnginePackageName, kFlutterEngineEnvironmentVariableName),
-              exitCode: 2);
-          }
-        }
-      } on FileSystemException {
-        engineSourcePath = null;
-      } on FormatException {
-        engineSourcePath = null;
-      }
-      // If engineSourcePath is still not set, try to determine it by flutter root.
-      engineSourcePath ??= _tryEnginePath(globals.fs.path.join(globals.fs.directory(Cache.flutterRoot).parent.path, 'engine', 'src'));
-    }
-
-    if (engineSourcePath != null && _tryEnginePath(engineSourcePath) == null) {
-      throwToolExit(userMessages.runnerNoEngineBuildDirInPath(engineSourcePath),
-        exitCode: 2);
-    }
-
-    return engineSourcePath;
-  }
-
-  String _getHostEngineBasename(String localEngineBasename) {
-    // Determine the host engine directory associated with the local engine:
-    // Strip '_sim_' since there are no host simulator builds.
-    String tmpBasename = localEngineBasename.replaceFirst('_sim_', '_');
-    tmpBasename = tmpBasename.substring(tmpBasename.indexOf('_') + 1);
-    // Strip suffix for various archs.
-    final List<String> suffixes = <String>['_arm', '_arm64', '_x86', '_x64'];
-    for (final String suffix in suffixes) {
-      tmpBasename = tmpBasename.replaceFirst(RegExp('$suffix\$'), '');
-    }
-    return 'host_' + tmpBasename;
-  }
-
-  EngineBuildPaths _findEngineBuildPath(ArgResults globalResults, String enginePath) {
-    String localEngine;
-    if (globalResults['local-engine'] != null) {
-      localEngine = globalResults['local-engine'] as String;
-    } else {
-      throwToolExit(userMessages.runnerLocalEngineRequired, exitCode: 2);
-    }
-
-    final String engineBuildPath = globals.fs.path.normalize(globals.fs.path.join(enginePath, 'out', localEngine));
-    if (!globals.fs.isDirectorySync(engineBuildPath)) {
-      throwToolExit(userMessages.runnerNoEngineBuild(engineBuildPath), exitCode: 2);
-    }
-
-    final String basename = globals.fs.path.basename(engineBuildPath);
-    final String hostBasename = _getHostEngineBasename(basename);
-    final String engineHostBuildPath = globals.fs.path.normalize(globals.fs.path.join(globals.fs.path.dirname(engineBuildPath), hostBasename));
-    if (!globals.fs.isDirectorySync(engineHostBuildPath)) {
-      throwToolExit(userMessages.runnerNoEngineBuild(engineHostBuildPath), exitCode: 2);
-    }
-
-    return EngineBuildPaths(targetEngine: engineBuildPath, hostEngine: engineHostBuildPath);
-  }
-
   @visibleForTesting
   static void initFlutterRoot() {
     Cache.flutterRoot ??= defaultFlutterRoot;

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

+// Copyright 2014 The Flutter 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 'package:meta/meta.dart';
+import 'package:package_config/package_config.dart';
+
+import '../artifacts.dart';
+import '../base/common.dart';
+import '../base/file_system.dart';
+import '../base/logger.dart';
+import '../base/platform.dart';
+import '../base/user_messages.dart' hide userMessages;
+import '../dart/package_map.dart';
+import 'flutter_command_runner.dart';
+
+/// A strategy for locating the out/ directory of a local engine build.
+///
+/// The flutter tool can be run with the output files of one or more engine builds
+/// replacing the cached artifacts. Typically this is done by setting the
+/// `--local-engine` command line flag to the name of the desired engine variant
+/// (e.g. "host_debug_unopt").  Provided that the `flutter/` and `engine/` directories
+/// are located adjacent to one another, the output folder will be located
+/// automatically.
+///
+/// For scenarios where the engine is not adjacent to flutter, the
+/// `--local-engine-src-path` can be provided to give an exact path.
+///
+/// For more information on local engines, see CONTRIBUTING.md.
+class LocalEngineLocator {
+  LocalEngineLocator({
+    @required Platform platform,
+    @required Logger logger,
+    @required FileSystem fileSystem,
+    @required String flutterRoot,
+    @required UserMessages userMessages,
+  }) : _platform = platform,
+       _logger = logger,
+       _fileSystem = fileSystem,
+       _flutterRoot = flutterRoot,
+        _userMessages = userMessages;
+
+  final Platform _platform;
+  final Logger _logger;
+  final FileSystem _fileSystem;
+  final String _flutterRoot;
+  final UserMessages _userMessages;
+
+  /// Returns the engine build path of a local engine if one is located, otherwise `null`.
+  Future<EngineBuildPaths> findEnginePath(String engineSourcePath, String localEngine) async {
+    engineSourcePath ??= _platform.environment[kFlutterEngineEnvironmentVariableName];
+
+    if (engineSourcePath == null && localEngine != null) {
+      try {
+        final PackageConfig packageConfig = await loadPackageConfigWithLogging(
+          _fileSystem.file(globalPackagesPath),
+          logger: _logger,
+          throwOnError: false,
+        );
+        // Skip if sky_engine is the version in bin/cache.
+        Uri engineUri = packageConfig[kFlutterEnginePackageName]?.packageUriRoot;
+        final String cachedPath = _fileSystem.path.join(_flutterRoot, 'bin', 'cache', 'pkg', kFlutterEnginePackageName, 'lib');
+        if (engineUri != null && _fileSystem.identicalSync(cachedPath, engineUri.path)) {
+          engineUri = null;
+        }
+        // If sky_engine is specified and the engineSourcePath not set, try to
+        // determine the engineSourcePath by sky_engine setting. A typical engine Uri
+        // looks like:
+        // file://flutter-engine-local-path/src/out/host_debug_unopt/gen/dart-pkg/sky_engine/lib/
+        if (engineUri?.path != null) {
+          engineSourcePath = _fileSystem.directory(engineUri.path)
+            ?.parent
+            ?.parent
+            ?.parent
+            ?.parent
+            ?.parent
+            ?.parent
+            ?.path;
+          if (engineSourcePath != null && (engineSourcePath == _fileSystem.path.dirname(engineSourcePath) || engineSourcePath.isEmpty)) {
+            engineSourcePath = null;
+            throwToolExit(
+              _userMessages.runnerNoEngineSrcDir(
+                kFlutterEnginePackageName,
+                kFlutterEngineEnvironmentVariableName,
+              ),
+              exitCode: 2,
+            );
+          }
+        }
+      } on FileSystemException {
+        engineSourcePath = null;
+      }
+      // If engineSourcePath is still not set, try to determine it by flutter root.
+      engineSourcePath ??= _tryEnginePath(
+        _fileSystem.path.join(_fileSystem.directory(_flutterRoot).parent.path, 'engine', 'src'),
+      );
+    }
+
+    if (engineSourcePath != null && _tryEnginePath(engineSourcePath) == null) {
+      throwToolExit(
+        _userMessages.runnerNoEngineBuildDirInPath(engineSourcePath),
+        exitCode: 2,
+      );
+    }
+
+    if (engineSourcePath != null) {
+      return _findEngineBuildPath(localEngine, engineSourcePath);
+    }
+    return null;
+  }
+
+  // Determine the host engine directory associated with the local engine:
+  // Strip '_sim_' since there are no host simulator builds.
+  String _getHostEngineBasename(String localEngineBasename) {
+    String tmpBasename = localEngineBasename.replaceFirst('_sim_', '_');
+    tmpBasename = tmpBasename.substring(tmpBasename.indexOf('_') + 1);
+    // Strip suffix for various archs.
+    const List<String> suffixes = <String>['_arm', '_arm64', '_x86', '_x64'];
+    for (final String suffix in suffixes) {
+      tmpBasename = tmpBasename.replaceFirst(RegExp('$suffix\$'), '');
+    }
+    return 'host_' + tmpBasename;
+  }
+
+  EngineBuildPaths _findEngineBuildPath(String localEngine, String enginePath) {
+    if (localEngine == null) {
+      throwToolExit(_userMessages.runnerLocalEngineRequired, exitCode: 2);
+    }
+
+    final String engineBuildPath = _fileSystem.path.normalize(_fileSystem.path.join(enginePath, 'out', localEngine));
+    if (!_fileSystem.isDirectorySync(engineBuildPath)) {
+      throwToolExit(_userMessages.runnerNoEngineBuild(engineBuildPath), exitCode: 2);
+    }
+
+    final String basename = _fileSystem.path.basename(engineBuildPath);
+    final String hostBasename = _getHostEngineBasename(basename);
+    final String engineHostBuildPath = _fileSystem.path.normalize(
+      _fileSystem.path.join(_fileSystem.path.dirname(engineBuildPath), hostBasename),
+    );
+    if (!_fileSystem.isDirectorySync(engineHostBuildPath)) {
+      throwToolExit(_userMessages.runnerNoEngineBuild(engineHostBuildPath), exitCode: 2);
+    }
+
+    return EngineBuildPaths(targetEngine: engineBuildPath, hostEngine: engineHostBuildPath);
+  }
+
+  String _tryEnginePath(String enginePath) {
+    print('looking for $enginePath');
+    if (_fileSystem.isDirectorySync(_fileSystem.path.join(enginePath, 'out'))) {
+      return enginePath;
+    }
+    return null;
+  }
+}

packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart

@@ -21,10 +21,7 @@ import '../../src/context.dart';
 import 'utils.dart';
 
 const String _kFlutterRoot = '/flutter/flutter';
-const String _kEngineRoot = '/flutter/engine';
-const String _kArbitraryEngineRoot = '/arbitrary/engine';
 const String _kProjectRoot = '/project';
-const String _kDotPackages = '.packages';
 
 void main() {
   group('FlutterCommandRunner', () {
@@ -112,49 +109,6 @@ void main() {
         Platform: () => platform,
       }, initializeFlutterRoot: false);
 
-      testUsingContext('works if --local-engine is specified and --local-engine-src-path is determined by sky_engine', () async {
-        fs.directory('$_kArbitraryEngineRoot/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/').createSync(recursive: true);
-        fs.directory('$_kArbitraryEngineRoot/src/out/host_debug').createSync(recursive: true);
-        fs.file(_kDotPackages).writeAsStringSync('sky_engine:file://$_kArbitraryEngineRoot/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/');
-
-        await runner.run(<String>['dummy', '--local-engine=ios_debug']);
-
-        // Verify that this also works if the sky_engine path is a symlink to the engine root.
-        fs.link('/symlink').createSync(_kArbitraryEngineRoot);
-        fs.file(_kDotPackages).writeAsStringSync('sky_engine:file:///symlink/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/');
-
-        await runner.run(<String>['dummy', '--local-engine=ios_debug']);
-      }, overrides: <Type, Generator>{
-        FileSystem: () => fs,
-        ProcessManager: () => FakeProcessManager.any(),
-        Platform: () => platform,
-      }, initializeFlutterRoot: false);
-
-      testUsingContext('works if --local-engine is specified and --local-engine-src-path is specified', () async {
-        // Intentionally do not create a package_config to verify that it is not required.
-        fs.directory('$_kArbitraryEngineRoot/src/out/ios_debug').createSync(recursive: true);
-        fs.directory('$_kArbitraryEngineRoot/src/out/host_debug').createSync(recursive: true);
-
-        await runner.run(<String>['dummy', '--local-engine-src-path=$_kArbitraryEngineRoot/src', '--local-engine=ios_debug']);
-      }, overrides: <Type, Generator>{
-        FileSystem: () => fs,
-        ProcessManager: () => FakeProcessManager.any(),
-        Platform: () => platform,
-      }, initializeFlutterRoot: false);
-
-      testUsingContext('works if --local-engine is specified and --local-engine-src-path is determined by flutter root', () async {
-        fs.file(_kDotPackages).writeAsStringSync('\n');
-        fs.directory('$_kEngineRoot/src/out/ios_debug').createSync(recursive: true);
-        fs.directory('$_kEngineRoot/src/out/host_debug').createSync(recursive: true);
-
-        await runner.run(<String>['dummy', '--local-engine=ios_debug']);
-      }, overrides: <Type, Generator>{
-        FileSystem: () => fs,
-        ProcessManager: () => FakeProcessManager.any(),
-        Platform: () => platform,
-      }, initializeFlutterRoot: false);
-    });
-
     testUsingContext('Doesnt crash on invalid .packages file', () async {
       fs.file('pubspec.yaml').createSync();
       fs.file('.packages')
@@ -286,6 +240,7 @@ void main() {
       }, initializeFlutterRoot: false);
     });
   });
+  });
 }
 class MockProcessManager extends Mock implements ProcessManager {}
 

packages/flutter_tools/test/general.shard/runner/local_engine_test.dart

+// Copyright 2014 The Flutter 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 'package:file/memory.dart';
+import 'package:flutter_tools/src/artifacts.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/base/user_messages.dart';
+import 'package:flutter_tools/src/runner/local_engine.dart';
+import 'package:matcher/matcher.dart';
+
+import '../../src/common.dart';
+
+const String kEngineRoot = '/flutter/engine';
+const String kArbitraryEngineRoot = '/arbitrary/engine';
+const String kDotPackages = '.packages';
+
+void main() {
+  testWithoutContext('works if --local-engine is specified and --local-engine-src-path '
+    'is determined by sky_engine', () async {
+    final FileSystem fileSystem = MemoryFileSystem.test();
+    fileSystem
+      .directory('$kArbitraryEngineRoot/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/')
+      .createSync(recursive: true);
+    fileSystem
+      .directory('$kArbitraryEngineRoot/src/out/host_debug')
+      .createSync(recursive: true);
+    fileSystem
+      .file(kDotPackages)
+      .writeAsStringSync('sky_engine:file://$kArbitraryEngineRoot/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/');
+    fileSystem
+      .file('bin/cache/pkg/sky_engine/lib')
+      .createSync(recursive: true);
+
+    final LocalEngineLocator localEngineLocator = LocalEngineLocator(
+      fileSystem: fileSystem,
+      flutterRoot: '',
+      logger: BufferLogger.test(),
+      userMessages: UserMessages(),
+      platform: FakePlatform(environment: <String, String>{}),
+    );
+
+    expect(
+      await localEngineLocator.findEnginePath(null, 'ios_debug'),
+      matchesEngineBuildPaths(
+        hostEngine: '/arbitrary/engine/src/out/host_debug',
+        targetEngine: '/arbitrary/engine/src/out/ios_debug',
+      ),
+    );
+
+    // Verify that this also works if the sky_engine path is a symlink to the engine root.
+    fileSystem.link('/symlink').createSync(kArbitraryEngineRoot);
+    fileSystem
+      .file(kDotPackages)
+      .writeAsStringSync('sky_engine:file:///symlink/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/');
+
+    expect(
+      await localEngineLocator.findEnginePath(null, 'ios_debug'),
+      matchesEngineBuildPaths(
+        hostEngine: '/symlink/src/out/host_debug',
+        targetEngine: '/symlink/src/out/ios_debug',
+      ),
+    );
+  });
+
+  testWithoutContext('works if --local-engine is specified and --local-engine-src-path '
+    'is specified', () async {
+    final FileSystem fileSystem = MemoryFileSystem.test();
+    // Intentionally do not create a package_config to verify that it is not required.
+    fileSystem.directory('$kArbitraryEngineRoot/src/out/ios_debug').createSync(recursive: true);
+    fileSystem.directory('$kArbitraryEngineRoot/src/out/host_debug').createSync(recursive: true);
+
+    final LocalEngineLocator localEngineLocator = LocalEngineLocator(
+      fileSystem: fileSystem,
+      flutterRoot: '',
+      logger: BufferLogger.test(),
+      userMessages: UserMessages(),
+      platform: FakePlatform(environment: <String, String>{}),
+    );
+
+    expect(
+      await localEngineLocator.findEnginePath('$kArbitraryEngineRoot/src', 'ios_debug'),
+      matchesEngineBuildPaths(
+        hostEngine: '/arbitrary/engine/src/out/host_debug',
+        targetEngine: '/arbitrary/engine/src/out/ios_debug',
+      ),
+    );
+  });
+
+  testWithoutContext('works if --local-engine is specified and --local-engine-src-path '
+    'is determined by flutter root', () async {
+    final FileSystem fileSystem = MemoryFileSystem.test();
+    fileSystem.file(kDotPackages).writeAsStringSync('\n');
+    fileSystem
+      .directory('$kEngineRoot/src/out/ios_debug')
+      .createSync(recursive: true);
+    fileSystem
+      .directory('$kEngineRoot/src/out/host_debug')
+      .createSync(recursive: true);
+    fileSystem
+      .file('bin/cache/pkg/sky_engine/lib')
+      .createSync(recursive: true);
+
+    final LocalEngineLocator localEngineLocator = LocalEngineLocator(
+      fileSystem: fileSystem,
+      flutterRoot: 'flutter/flutter',
+      logger: BufferLogger.test(),
+      userMessages: UserMessages(),
+      platform: FakePlatform(environment: <String, String>{}),
+    );
+
+    expect(
+      await localEngineLocator.findEnginePath(null, 'ios_debug'),
+      matchesEngineBuildPaths(
+        hostEngine: 'flutter/engine/src/out/host_debug',
+        targetEngine: 'flutter/engine/src/out/ios_debug',
+      ),
+    );
+  });
+}
+
+Matcher matchesEngineBuildPaths({
+  String hostEngine,
+  String targetEngine,
+}) {
+  return const TypeMatcher<EngineBuildPaths>()
+    .having((EngineBuildPaths paths) => paths.hostEngine, 'hostEngine', hostEngine)
+    .having((EngineBuildPaths paths) => paths.targetEngine, 'targetEngine', targetEngine);
+}