Unverified Commit 18bb4d72 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Revert "[flutter_tools] reland: fold process resolution logic into the flutter...

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

This reverts commit bd813879.
parent 52f8b89c
......@@ -11,10 +11,7 @@ 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
......@@ -56,11 +53,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 T noExitOnFailure<T>(T Function() operation) {
static void noExitOnFailure(void Function() operation) {
final bool previousValue = ErrorHandlingFileSystem._noExitOnFailure;
try {
ErrorHandlingFileSystem._noExitOnFailure = true;
return operation();
operation();
} finally {
ErrorHandlingFileSystem._noExitOnFailure = previousValue;
}
......@@ -504,44 +501,6 @@ 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
......@@ -550,47 +509,28 @@ typedef ProcessStart = Future<io.Process> Function(
///
/// See also:
/// * [ErrorHandlingFileSystem], for a similar file system strategy.
class ErrorHandlingProcessManager implements ProcessManager {
class ErrorHandlingProcessManager extends ProcessManager {
ErrorHandlingProcessManager({
@required ProcessManager delegate,
@required Platform platform,
@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;
}) : _delegate = delegate,
_platform = platform;
// 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>>{};
final ProcessManager _delegate;
final Platform _platform;
@override
bool canRun(dynamic executable, {String workingDirectory}) {
return _runSync<bool>(
() => _getCommandPath(<dynamic>[executable], workingDirectory, strict: true) != null,
return _runSync(
() => _delegate.canRun(executable, workingDirectory: workingDirectory),
platform: _platform,
);
}
@override
bool killPid(int pid, [io.ProcessSignal signal = io.ProcessSignal.sigterm]) {
return _runSync<bool>(
() => _killPid(pid, signal),
return _runSync(
() => _delegate.killPid(pid, signal),
platform: _platform,
);
}
......@@ -605,18 +545,15 @@ class ErrorHandlingProcessManager implements ProcessManager {
Encoding stdoutEncoding = io.systemEncoding,
Encoding stderrEncoding = io.systemEncoding,
}) {
return _run(() {
return _processRun(
_getCommandPath(command, workingDirectory),
_getArguments(command),
return _run(() => _delegate.run(
command,
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
);
}, platform: _platform);
), platform: _platform);
}
@override
......@@ -628,16 +565,13 @@ class ErrorHandlingProcessManager implements ProcessManager {
bool runInShell = false,
io.ProcessStartMode mode = io.ProcessStartMode.normal,
}) {
return _run(() {
return _processStart(
_getCommandPath(command, workingDirectory),
_getArguments(command),
return _run(() => _delegate.start(
command,
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
);
}, platform: _platform);
), platform: _platform);
}
@override
......@@ -650,91 +584,15 @@ class ErrorHandlingProcessManager implements ProcessManager {
Encoding stdoutEncoding = io.systemEncoding,
Encoding stderrEncoding = io.systemEncoding,
}) {
return _runSync(() {
return _processRunSync(
_getCommandPath(command, workingDirectory),
_getArguments(command),
return _runSync(() => _delegate.runSync(
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()];
), platform: _platform);
}
}
......
......@@ -213,10 +213,7 @@ class _PosixUtils extends OperatingSystemUtils {
throwOnError: true,
verboseExceptions: true,
);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
// 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.';
......@@ -300,10 +297,7 @@ class _WindowsUtils extends OperatingSystemUtils {
ProcessResult result;
try {
result = _processManager.runSync(<String>['where', execName]);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
// `where` could be missing if system32 is not on the PATH.
throwToolExit(
'Cannot find the executable for `where`. This can happen if the System32 '
......
......@@ -610,12 +610,3 @@ 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;
}
......@@ -223,9 +223,8 @@ 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,
......
......@@ -361,6 +361,10 @@ 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;
......
......@@ -6,7 +6,6 @@ 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';
......@@ -105,10 +104,7 @@ Future<void> _runCmake(String buildModeName, Directory sourceDir, Directory buil
},
trace: true,
);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
throwToolExit("cmake not found. Run 'flutter doctor' for more information.");
}
if (result != 0) {
......@@ -135,10 +131,7 @@ Future<void> _runBuild(Directory buildDir) async {
},
trace: true,
);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
throwToolExit("ninja not found. Run 'flutter doctor' for more information.");
}
if (result != 0) {
......
......@@ -6,7 +6,6 @@ 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';
......@@ -169,10 +168,7 @@ class LinuxDoctorValidator extends DoctorValidator {
binary,
'--version',
]);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
// ignore error.
}
if (result == null || result.exitCode != 0) {
......@@ -191,10 +187,7 @@ class LinuxDoctorValidator extends DoctorValidator {
'--exists',
library,
]);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
// ignore error.
}
return (result?.exitCode ?? 1) == 0;
......
......@@ -84,6 +84,8 @@ class Xcode {
).stdout.trim();
} on ProcessException {
// Ignored, return null below.
} on ArgumentError {
// Ignored, return null below.
}
}
return _xcodeSelectPath;
......@@ -274,6 +276,8 @@ 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;
......@@ -310,6 +314,8 @@ 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;
......@@ -402,6 +408,8 @@ class XCDevice {
}));
} on ProcessException catch (exception, stackTrace) {
_deviceIdentifierByEvent.addError(exception, stackTrace);
} on ArgumentError catch (exception, stackTrace) {
_deviceIdentifierByEvent.addError(exception, stackTrace);
}
}
......
......@@ -150,7 +150,11 @@ 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;
}
}
/// The executable this launcher will use.
......
......@@ -6,7 +6,6 @@ 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';
......@@ -108,10 +107,7 @@ Future<void> _runCmakeGeneration(String cmakePath, Directory buildDir, Directory
],
trace: true,
);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
throwToolExit("cmake not found. Run 'flutter doctor' for more information.");
}
if (result != 0) {
......@@ -148,10 +144,7 @@ Future<void> _runBuild(String cmakePath, Directory buildDir, String buildModeNam
trace: true,
stdoutErrorMatcher: errorMatcher,
);
} on ProcessException catch (err) {
if (!isMissingExecutableException(err)) {
rethrow;
}
} on ArgumentError {
throwToolExit("cmake not found. Run 'flutter doctor' for more information.");
}
if (result != 0) {
......
......@@ -303,6 +303,8 @@ 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 {
......@@ -418,6 +420,8 @@ 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.
}
......
......@@ -6,7 +6,6 @@ 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';
......@@ -153,7 +152,7 @@ void main() {
setUpMockProjectFilesForBuild();
processManager = FakeProcessManager.list(<FakeCommand>[
cmakeCommand('release', onRun: () {
throw const ProcessException('cmake', <String>[], '', 2);
throw ArgumentError();
}),
]);
......@@ -173,7 +172,7 @@ void main() {
processManager = FakeProcessManager.list(<FakeCommand>[
cmakeCommand('release'),
ninjaCommand('release', onRun: () {
throw const ProcessException('ninja', <String>[], '', 2);
throw ArgumentError();
}),
]);
......
......@@ -63,9 +63,9 @@ void main() {
});
group('which on Windows', () {
testWithoutContext('throws tool exit if where throws a process exception with code 2', () async {
testWithoutContext('throws tool exit if where throws an argument error', () async {
when(mockProcessManager.runSync(<String>['where', kExecutable]))
.thenThrow(const ProcessException('where', <String>[], 'Cannot find executable for where', 2));
.thenThrow(ArgumentError('Cannot find executable for where'));
final OperatingSystemUtils utils = createOSUtils(FakePlatform(operatingSystem: 'windows'));
expect(() => utils.which(kExecutable), throwsA(isA<ToolExit>()));
......@@ -143,11 +143,11 @@ void main() {
);
});
testWithoutContext('If unzip throws a ProcessException with code 2, display an install message', () {
testWithoutContext('If unzip throws an ArgumentError, display an install message', () {
final FileSystem fileSystem = MemoryFileSystem.test();
when(mockProcessManager.runSync(
<String>['unzip', '-o', '-q', 'foo.zip', '-d', fileSystem.currentDirectory.path],
)).thenThrow(const ProcessException('unzip', <String>['-o', '-q', 'foo.zip', '-d'], '', 2));
)).thenThrow(ArgumentError());
final OperatingSystemUtils linuxOsUtils = OperatingSystemUtils(
fileSystem: fileSystem,
......
......@@ -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(const ProcessException('/usr/bin/xcode-select', <String>['--print-path'], '', 2));
.thenThrow(ArgumentError('Invalid argument(s): Cannot find executable for /usr/bin/xcode-select'));
expect(xcode.xcodeSelectPath, isNull);
});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment