[flutter_tools] reland: fold process resolution logic into the flutter tool (#67957)

Reland of #67669

The flutter tool has a number of crashes on stable where an ArgumentError is thrown due to the process manager not being able to resolve an executable.

So that we can adjust/modify this logic, fold it into flutter and add some additional logging.
caches the resolved executable per target directory, to avoid repeated look ups.
Instead of throwing an argument error, attempts to run the executable as given if an exact path can't be found
Accept files or symlinks for the executable path.
user where/which to resolve path instead of package:process logic.

packages/flutter_tools/lib/src/base/error_handling_io.dart

@@ -11,7 +11,10 @@ import 'package:path/path.dart' as p; // ignore: package_path_import
 import 'package:process/process.dart';
 
 import 'common.dart' show throwToolExit;
+import 'io.dart';
+import 'logger.dart';
 import 'platform.dart';
+import 'process.dart';
 
 // The Flutter tool hits file system and process errors that only the end-user can address.
 // We would like these errors to not hit crash logging. In these cases, we
@@ -53,11 +56,11 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem {
   /// This can be used to bypass the [ErrorHandlingFileSystem] permission exit
   /// checks for situations where failure is acceptable, such as the flutter
   /// persistent settings cache.
-  static void noExitOnFailure(void Function() operation) {
+  static T noExitOnFailure<T>(T Function() operation) {
     final bool previousValue = ErrorHandlingFileSystem._noExitOnFailure;
     try {
       ErrorHandlingFileSystem._noExitOnFailure = true;
-      operation();
+      return operation();
     } finally {
       ErrorHandlingFileSystem._noExitOnFailure = previousValue;
     }
@@ -501,6 +504,44 @@ T _runSync<T>(T Function() op, {
   }
 }
 
+/// Type signature for [io.Process.killPid].
+typedef ProcessKillPid = bool Function(int pid, [io.ProcessSignal signal]);
+
+/// Type signature for [io.Process.run].
+typedef ProcessRun = Future<io.ProcessResult> Function(
+  String executable,
+  List<String> arguments, {
+  String workingDirectory,
+  Map<String, String> environment,
+  bool includeParentEnvironment,
+  bool runInShell,
+  Encoding stdoutEncoding,
+  Encoding stderrEncoding,
+});
+
+/// Type signature for [io.Process.runSync].
+typedef ProcessRunSync = io.ProcessResult Function(
+  String executable,
+  List<String> arguments, {
+  String workingDirectory,
+  Map<String, String> environment,
+  bool includeParentEnvironment,
+  bool runInShell,
+  Encoding stdoutEncoding,
+  Encoding stderrEncoding,
+});
+
+/// Type signature for [io.Process.start].
+typedef ProcessStart = Future<io.Process> Function(
+  String executable,
+  List<String> arguments, {
+  String workingDirectory,
+  Map<String, String> environment,
+  bool includeParentEnvironment,
+  bool runInShell,
+  io.ProcessStartMode mode,
+});
+
 /// A [ProcessManager] that throws a [ToolExit] on certain errors.
 ///
 /// If a [ProcessException] is not caused by the Flutter tool, and can only be
@@ -509,28 +550,47 @@ T _runSync<T>(T Function() op, {
 ///
 /// See also:
 ///   * [ErrorHandlingFileSystem], for a similar file system strategy.
-class ErrorHandlingProcessManager extends ProcessManager {
+class ErrorHandlingProcessManager implements ProcessManager {
   ErrorHandlingProcessManager({
-    @required ProcessManager delegate,
     @required Platform platform,
-  }) : _delegate = delegate,
-       _platform = platform;
-
-  final ProcessManager _delegate;
+    @required FileSystem fileSystem,
+    @required Logger logger,
+    @visibleForTesting ProcessKillPid killPid = io.Process.killPid,
+    @visibleForTesting ProcessRun run = io.Process.run,
+    @visibleForTesting ProcessRunSync runSync = io.Process.runSync,
+    @visibleForTesting ProcessStart start = io.Process.start,
+  }) : _platform = platform,
+       _fileSytem = fileSystem,
+       _logger = logger,
+       _killPid = killPid,
+       _processRun = run,
+       _processRunSync = runSync,
+       _processStart = start;
+
+  final Logger _logger;
+  final FileSystem _fileSytem;
   final Platform _platform;
+  final ProcessKillPid _killPid;
+  final ProcessRun _processRun;
+  final ProcessRunSync _processRunSync;
+  final ProcessStart _processStart;
+
+  // Cached executable paths, the first is the working directory, the next is a map from
+  // executable name to actual path.
+  final Map<String, Map<String, String>> _resolvedExecutables = <String, Map<String, String>>{};
 
   @override
   bool canRun(dynamic executable, {String workingDirectory}) {
-    return _runSync(
-      () => _delegate.canRun(executable, workingDirectory: workingDirectory),
+    return _runSync<bool>(
+      () => _getCommandPath(<dynamic>[executable], workingDirectory, strict: true) != null,
       platform: _platform,
     );
   }
 
   @override
   bool killPid(int pid, [io.ProcessSignal signal = io.ProcessSignal.sigterm]) {
-    return _runSync(
-      () => _delegate.killPid(pid, signal),
+    return _runSync<bool>(
+      () => _killPid(pid, signal),
       platform: _platform,
     );
   }
@@ -545,15 +605,18 @@ class ErrorHandlingProcessManager extends ProcessManager {
     Encoding stdoutEncoding = io.systemEncoding,
     Encoding stderrEncoding = io.systemEncoding,
   }) {
-    return _run(() => _delegate.run(
-      command,
-      workingDirectory: workingDirectory,
-      environment: environment,
-      includeParentEnvironment: includeParentEnvironment,
-      runInShell: runInShell,
-      stdoutEncoding: stdoutEncoding,
-      stderrEncoding: stderrEncoding,
-    ), platform: _platform);
+    return _run(() {
+      return _processRun(
+        _getCommandPath(command, workingDirectory),
+        _getArguments(command),
+        workingDirectory: workingDirectory,
+        environment: environment,
+        includeParentEnvironment: includeParentEnvironment,
+        runInShell: runInShell,
+        stdoutEncoding: stdoutEncoding,
+        stderrEncoding: stderrEncoding,
+      );
+    }, platform: _platform);
   }
 
   @override
@@ -565,13 +628,16 @@ class ErrorHandlingProcessManager extends ProcessManager {
     bool runInShell = false,
     io.ProcessStartMode mode = io.ProcessStartMode.normal,
   }) {
-    return _run(() => _delegate.start(
-      command,
-      workingDirectory: workingDirectory,
-      environment: environment,
-      includeParentEnvironment: includeParentEnvironment,
-      runInShell: runInShell,
-    ), platform: _platform);
+    return _run(() {
+      return _processStart(
+        _getCommandPath(command, workingDirectory),
+        _getArguments(command),
+        workingDirectory: workingDirectory,
+        environment: environment,
+        includeParentEnvironment: includeParentEnvironment,
+        runInShell: runInShell,
+      );
+    }, platform: _platform);
   }
 
   @override
@@ -584,15 +650,91 @@ class ErrorHandlingProcessManager extends ProcessManager {
     Encoding stdoutEncoding = io.systemEncoding,
     Encoding stderrEncoding = io.systemEncoding,
   }) {
-    return _runSync(() => _delegate.runSync(
-      command,
-      workingDirectory: workingDirectory,
-      environment: environment,
-      includeParentEnvironment: includeParentEnvironment,
-      runInShell: runInShell,
-      stdoutEncoding: stdoutEncoding,
-      stderrEncoding: stderrEncoding,
-    ), platform: _platform);
+    return _runSync(() {
+      return _processRunSync(
+        _getCommandPath(command, workingDirectory),
+        _getArguments(command),
+        workingDirectory: workingDirectory,
+        environment: environment,
+        includeParentEnvironment: includeParentEnvironment,
+        runInShell: runInShell,
+        stdoutEncoding: stdoutEncoding,
+        stderrEncoding: stderrEncoding,
+      );
+    }, platform: _platform);
+  }
+
+  /// Resolve the first item of [command] to a file or link.
+  ///
+  /// This function will cache each raw command string per working directory,
+  /// so that repeated lookups for the same command will resolve to the same
+  /// executable. If a new executable is installed in a higher priority
+  /// location after a command has already been cached, the older executable
+  /// will be used until the tool restarts.
+  ///
+  /// If the entity cannot be found, return the raw command as is and do not
+  /// cache it.
+  String _getCommandPath(List<dynamic> command, String workingDirectory, { bool strict = false }) {
+    final String executable = command.first.toString();
+    final Map<String, String> workingDirectoryCache = _resolvedExecutables[workingDirectory] ??= <String, String>{};
+    final String resolvedExecutable = workingDirectoryCache[executable]
+      ?? _which(executable, workingDirectory, strict);
+    if (resolvedExecutable != executable) {
+      workingDirectoryCache[executable] = resolvedExecutable;
+    }
+    return resolvedExecutable;
+  }
+
+  String _which(String execName, String workingDirectory, bool strict) {
+    // `where` always returns all matches, not just the first one.
+    ProcessResult result;
+    try {
+      result = _processRunSync(
+        _platform.isWindows ? 'where' : 'which',
+        <String>[execName],
+        workingDirectory: workingDirectory,
+      );
+    } on ProcessException catch (err) {
+      if (!isMissingExecutableException(err) || !_platform.isWindows) {
+        rethrow;
+      }
+      // `where` could be missing if system32 is not on the PATH.
+      throwToolExit(
+        'Cannot find the executable for `where`. This can happen if the System32 '
+        'folder (e.g. C:\\Windows\\System32 ) is removed from the PATH environment '
+        'variable. Ensure that this is present and then try again after restarting '
+        'the terminal and/or IDE.'
+      );
+    }
+    if (result.exitCode != 0) {
+      return execName;
+    }
+    final List<String> lines = (result.stdout as String).trim().split('\n');
+    for (final String line in lines) {
+      final String resolvedPath = line.trim();
+      try {
+        final String candidate = ErrorHandlingFileSystem.noExitOnFailure(() {
+          if (_fileSytem.isFileSync(resolvedPath)) {
+            return resolvedPath;
+          }
+          return null;
+        });
+        if (candidate != null) {
+          return candidate;
+        }
+      } on Exception {
+        // Do nothing and prevent permission issues from causing a tool exit.
+      }
+    }
+    _logger.printTrace('Could not resolve executable for $execName in $workingDirectory.');
+    if (strict) {
+      return null;
+    }
+    return execName;
+  }
+
+  List<String> _getArguments(List<dynamic> command) {
+    return <String>[for (dynamic arg in command.skip(1)) arg.toString()];
   }
 }
 

packages/flutter_tools/lib/src/base/os.dart

@@ -213,7 +213,10 @@ class _PosixUtils extends OperatingSystemUtils {
         throwOnError: true,
         verboseExceptions: true,
       );
-    } on ArgumentError {
+    } on ProcessException catch (err) {
+      if (!isMissingExecutableException(err)) {
+        rethrow;
+      }
       // unzip is not available. this error message is modeled after the download
       // error in bin/internal/update_dart_sdk.sh
       String message = 'Please install unzip.';
@@ -297,7 +300,10 @@ class _WindowsUtils extends OperatingSystemUtils {
     ProcessResult result;
     try {
       result = _processManager.runSync(<String>['where', execName]);
-    } on ArgumentError {
+    } on ProcessException catch (err) {
+      if (!isMissingExecutableException(err)) {
+        rethrow;
+      }
       // `where` could be missing if system32 is not on the PATH.
       throwToolExit(
         'Cannot find the executable for `where`. This can happen if the System32 '

packages/flutter_tools/lib/src/base/process.dart

@@ -610,3 +610,12 @@ class _DefaultProcessUtils implements ProcessUtils {
     }
   }
 }
+
+/// Whether the process exception was thrown due to a mising executable.
+///
+/// On macOS, Linux, and Windows error code 2 indicates a missing file, which
+/// means that the process resolution failed to locate the correct
+/// executable.
+bool isMissingExecutableException(ProcessException processException) {
+  return processException.errorCode == 2;
+}

packages/flutter_tools/lib/src/context_runner.dart

@@ -223,8 +223,9 @@ Future<T> runInContext<T>(
       ),
       ProcessInfo: () => ProcessInfo(),
       ProcessManager: () => ErrorHandlingProcessManager(
-        delegate: const LocalProcessManager(),
         platform: globals.platform,
+        fileSystem: globals.fs,
+        logger: globals.logger,
       ),
       ProcessUtils: () => ProcessUtils(
         processManager: globals.processManager,

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

@@ -361,10 +361,6 @@ class IOSDeployDebugger {
       _logger.printTrace('ios-deploy failed: $exception');
       _debuggerState = _IOSDeployDebuggerState.detached;
       _debuggerOutput.addError(exception, stackTrace);
-    } on ArgumentError catch (exception, stackTrace) {
-      _logger.printTrace('ios-deploy failed: $exception');
-      _debuggerState = _IOSDeployDebuggerState.detached;
-      _debuggerOutput.addError(exception, stackTrace);
     }
     // Wait until the debugger attaches, or the attempt fails.
     return debuggerCompleter.future;

packages/flutter_tools/lib/src/linux/build_linux.dart

@@ -6,6 +6,7 @@ import '../artifacts.dart';
 import '../base/analyze_size.dart';
 import '../base/common.dart';
 import '../base/file_system.dart';
+import '../base/io.dart';
 import '../base/logger.dart';
 import '../base/process.dart';
 import '../base/utils.dart';
@@ -104,7 +105,10 @@ Future<void> _runCmake(String buildModeName, Directory sourceDir, Directory buil
       },
       trace: true,
     );
-  } on ArgumentError {
+  } on ProcessException catch (err) {
+    if (!isMissingExecutableException(err)) {
+      rethrow;
+    }
     throwToolExit("cmake not found. Run 'flutter doctor' for more information.");
   }
   if (result != 0) {
@@ -131,7 +135,10 @@ Future<void> _runBuild(Directory buildDir) async {
       },
       trace: true,
     );
-  } on ArgumentError {
+  } on ProcessException catch (err) {
+    if (!isMissingExecutableException(err)) {
+      rethrow;
+    }
     throwToolExit("ninja not found. Run 'flutter doctor' for more information.");
   }
   if (result != 0) {

packages/flutter_tools/lib/src/linux/linux_doctor.dart

@@ -6,6 +6,7 @@ import 'package:meta/meta.dart';
 import 'package:process/process.dart';
 
 import '../base/io.dart';
+import '../base/process.dart';
 import '../base/user_messages.dart';
 import '../base/version.dart';
 import '../doctor.dart';
@@ -168,7 +169,10 @@ class LinuxDoctorValidator extends DoctorValidator {
         binary,
         '--version',
       ]);
-    } on ArgumentError {
+    } on ProcessException catch (err) {
+      if (!isMissingExecutableException(err)) {
+        rethrow;
+      }
       // ignore error.
     }
     if (result == null || result.exitCode != 0) {
@@ -187,7 +191,10 @@ class LinuxDoctorValidator extends DoctorValidator {
         '--exists',
         library,
       ]);
-    } on ArgumentError {
+    } on ProcessException catch (err) {
+      if (!isMissingExecutableException(err)) {
+        rethrow;
+      }
       // ignore error.
     }
     return (result?.exitCode ?? 1) == 0;

packages/flutter_tools/lib/src/macos/xcode.dart

@@ -84,8 +84,6 @@ class Xcode {
         ).stdout.trim();
       } on ProcessException {
         // Ignored, return null below.
-      } on ArgumentError {
-        // Ignored, return null below.
       }
     }
     return _xcodeSelectPath;
@@ -276,8 +274,6 @@ class XCDevice {
         ).stdout.trim();
       } on ProcessException catch (exception) {
         _logger.printTrace('Process exception finding xcdevice:\n$exception');
-      } on ArgumentError catch (exception) {
-        _logger.printTrace('Argument exception finding xcdevice:\n$exception');
       }
     }
     return _xcdevicePath;
@@ -314,8 +310,6 @@ class XCDevice {
       _logger.printTrace('xcdevice returned an error:\n${result.stderr}');
     } on ProcessException catch (exception) {
       _logger.printTrace('Process exception running xcdevice list:\n$exception');
-    } on ArgumentError catch (exception) {
-      _logger.printTrace('Argument exception running xcdevice list:\n$exception');
     }
 
     return null;
@@ -408,8 +402,6 @@ class XCDevice {
       }));
     } on ProcessException catch (exception, stackTrace) {
       _deviceIdentifierByEvent.addError(exception, stackTrace);
-    } on ArgumentError catch (exception, stackTrace) {
-      _deviceIdentifierByEvent.addError(exception, stackTrace);
     }
   }
 

packages/flutter_tools/lib/src/web/chrome.dart

@@ -150,11 +150,7 @@ class ChromiumLauncher {
   /// Whether we can locate the chrome executable.
   bool canFindExecutable() {
     final String chrome = _browserFinder(_platform, _fileSystem);
-    try {
-      return _processManager.canRun(chrome);
-    } on ArgumentError {
-      return false;
-    }
+    return _processManager.canRun(chrome);
   }
 
   /// The executable this launcher will use.

packages/flutter_tools/lib/src/windows/build_windows.dart

@@ -6,6 +6,7 @@ import '../artifacts.dart';
 import '../base/analyze_size.dart';
 import '../base/common.dart';
 import '../base/file_system.dart';
+import '../base/io.dart';
 import '../base/logger.dart';
 import '../base/process.dart';
 import '../base/utils.dart';
@@ -107,7 +108,10 @@ Future<void> _runCmakeGeneration(String cmakePath, Directory buildDir, Directory
       ],
       trace: true,
     );
-  } on ArgumentError {
+  } on ProcessException catch (err) {
+    if (!isMissingExecutableException(err)) {
+      rethrow;
+    }
     throwToolExit("cmake not found. Run 'flutter doctor' for more information.");
   }
   if (result != 0) {
@@ -144,7 +148,10 @@ Future<void> _runBuild(String cmakePath, Directory buildDir, String buildModeNam
       trace: true,
       stdoutErrorMatcher: errorMatcher,
     );
-  } on ArgumentError {
+  } on ProcessException catch (err) {
+    if (!isMissingExecutableException(err)) {
+      rethrow;
+    }
     throwToolExit("cmake not found. Run 'flutter doctor' for more information.");
   }
   if (result != 0) {

packages/flutter_tools/lib/src/windows/visual_studio.dart

@@ -303,8 +303,6 @@ class VisualStudio {
           return installations[0];
         }
       }
-    } on ArgumentError {
-      // Thrown if vswhere doesn't exist; ignore and return null below.
     } on ProcessException {
       // Ignored, return null below.
     } on FormatException {
@@ -420,8 +418,6 @@ class VisualStudio {
           return match.group(1).trim();
         }
       }
-    } on ArgumentError {
-      // Thrown if reg somehow doesn't exist; ignore and return null below.
     } on ProcessException {
       // Ignored, return null below.
     }

packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart

@@ -6,6 +6,7 @@ import 'package:args/command_runner.dart';
 import 'package:file/memory.dart';
 import 'package:file_testing/file_testing.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/base/utils.dart';
 import 'package:flutter_tools/src/cache.dart';
@@ -152,7 +153,7 @@ void main() {
     setUpMockProjectFilesForBuild();
     processManager = FakeProcessManager.list(<FakeCommand>[
       cmakeCommand('release', onRun: () {
-        throw ArgumentError();
+        throw const ProcessException('cmake', <String>[], '', 2);
       }),
     ]);
 
@@ -172,7 +173,7 @@ void main() {
     processManager = FakeProcessManager.list(<FakeCommand>[
       cmakeCommand('release'),
       ninjaCommand('release', onRun: () {
-        throw ArgumentError();
+        throw const ProcessException('ninja', <String>[], '', 2);
       }),
     ]);
 

packages/flutter_tools/test/general.shard/base/os_test.dart

@@ -63,9 +63,9 @@ void main() {
   });
 
   group('which on Windows', () {
-    testWithoutContext('throws tool exit if where throws an argument error', () async {
+    testWithoutContext('throws tool exit if where throws a process exception with code 2', () async {
       when(mockProcessManager.runSync(<String>['where', kExecutable]))
-          .thenThrow(ArgumentError('Cannot find executable for where'));
+          .thenThrow(const ProcessException('where', <String>[], 'Cannot find executable for where', 2));
       final OperatingSystemUtils utils = createOSUtils(FakePlatform(operatingSystem: 'windows'));
 
       expect(() => utils.which(kExecutable), throwsA(isA<ToolExit>()));
@@ -143,11 +143,11 @@ void main() {
     );
   });
 
-  testWithoutContext('If unzip throws an ArgumentError, display an install message', () {
+  testWithoutContext('If unzip throws a ProcessException with code 2, display an install message', () {
     final FileSystem fileSystem = MemoryFileSystem.test();
     when(mockProcessManager.runSync(
       <String>['unzip', '-o', '-q', 'foo.zip', '-d', fileSystem.currentDirectory.path],
-    )).thenThrow(ArgumentError());
+    )).thenThrow(const ProcessException('unzip', <String>['-o', '-q', 'foo.zip', '-d'], '', 2));
 
     final OperatingSystemUtils linuxOsUtils = OperatingSystemUtils(
       fileSystem: fileSystem,

packages/flutter_tools/test/general.shard/macos/xcode_test.dart

@@ -55,7 +55,7 @@ void main() {
           .thenThrow(const ProcessException('/usr/bin/xcode-select', <String>['--print-path']));
         expect(xcode.xcodeSelectPath, isNull);
         when(processManager.runSync(<String>['/usr/bin/xcode-select', '--print-path']))
-          .thenThrow(ArgumentError('Invalid argument(s): Cannot find executable for /usr/bin/xcode-select'));
+          .thenThrow(const ProcessException('/usr/bin/xcode-select', <String>['--print-path'], '', 2));
 
         expect(xcode.xcodeSelectPath, isNull);
       });