Unverified Commit dcd061c7 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] experiment with skipping process resolution (#74447)

parent fad18d6c
......@@ -575,6 +575,70 @@ T _runSync<T>(T Function() op, {
}
}
class _ProcessDelegate {
const _ProcessDelegate();
Future<io.Process> start(
List<String> command, {
String workingDirectory,
Map<String, String> environment,
bool includeParentEnvironment = true,
bool runInShell = false,
io.ProcessStartMode mode = io.ProcessStartMode.normal,
}) {
return io.Process.start(
command[0],
command.skip(1).toList(),
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
);
}
Future<io.ProcessResult> run(
List<String> command, {
String workingDirectory,
Map<String, String> environment,
bool includeParentEnvironment = true,
bool runInShell = false,
Encoding stdoutEncoding = io.systemEncoding,
Encoding stderrEncoding = io.systemEncoding,
}) {
return io.Process.run(
command[0],
command.skip(1).toList(),
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
);
}
io.ProcessResult runSync(
List<String> command, {
String workingDirectory,
Map<String, String> environment,
bool includeParentEnvironment = true,
bool runInShell = false,
Encoding stdoutEncoding = io.systemEncoding,
Encoding stderrEncoding = io.systemEncoding,
}) {
return io.Process.runSync(
command[0],
command.skip(1).toList(),
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
);
}
}
/// A [ProcessManager] that throws a [ToolExit] on certain errors.
///
/// If a [ProcessException] is not caused by the Flutter tool, and can only be
......@@ -592,6 +656,21 @@ class ErrorHandlingProcessManager extends ProcessManager {
final ProcessManager _delegate;
final Platform _platform;
static const _ProcessDelegate _processDelegate = _ProcessDelegate();
static bool _skipCommandLookup = false;
/// Bypass package:process command lookup for all functions in this block.
///
/// This required that the fully resolved executable path is provided.
static Future<T> skipCommandLookup<T>(Future<T> Function() operation) async {
final bool previousValue = ErrorHandlingProcessManager._skipCommandLookup;
try {
ErrorHandlingProcessManager._skipCommandLookup = true;
return await operation();
} finally {
ErrorHandlingProcessManager._skipCommandLookup = previousValue;
}
}
@override
bool canRun(dynamic executable, {String workingDirectory}) {
......@@ -619,7 +698,19 @@ class ErrorHandlingProcessManager extends ProcessManager {
Encoding stdoutEncoding = io.systemEncoding,
Encoding stderrEncoding = io.systemEncoding,
}) {
return _run(() => _delegate.run(
return _run(() {
if (_skipCommandLookup && _delegate is LocalProcessManager) {
return _processDelegate.run(
command.cast<String>(),
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
);
}
return _delegate.run(
command,
workingDirectory: workingDirectory,
environment: environment,
......@@ -627,7 +718,8 @@ class ErrorHandlingProcessManager extends ProcessManager {
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
), platform: _platform);
);
}, platform: _platform);
}
@override
......@@ -639,13 +731,24 @@ class ErrorHandlingProcessManager extends ProcessManager {
bool runInShell = false,
io.ProcessStartMode mode = io.ProcessStartMode.normal,
}) {
return _run(() => _delegate.start(
return _run(() {
if (_skipCommandLookup && _delegate is LocalProcessManager) {
return _processDelegate.start(
command.cast<String>(),
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
);
}
return _delegate.start(
command,
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
), platform: _platform);
);
}, platform: _platform);
}
@override
......@@ -658,7 +761,19 @@ class ErrorHandlingProcessManager extends ProcessManager {
Encoding stdoutEncoding = io.systemEncoding,
Encoding stderrEncoding = io.systemEncoding,
}) {
return _runSync(() => _delegate.runSync(
return _runSync(() {
if (_skipCommandLookup && _delegate is LocalProcessManager) {
return _processDelegate.runSync(
command.cast<String>(),
workingDirectory: workingDirectory,
environment: environment,
includeParentEnvironment: includeParentEnvironment,
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
);
}
return _delegate.runSync(
command,
workingDirectory: workingDirectory,
environment: environment,
......@@ -666,7 +781,8 @@ class ErrorHandlingProcessManager extends ProcessManager {
runInShell: runInShell,
stdoutEncoding: stdoutEncoding,
stderrEncoding: stderrEncoding,
), platform: _platform);
);
}, platform: _platform);
}
}
......
......@@ -4,7 +4,6 @@
// @dart = 2.8
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:process/process.dart';
......@@ -12,6 +11,7 @@ import 'package:process/process.dart';
import '../base/bot_detector.dart';
import '../base/common.dart';
import '../base/context.dart';
import '../base/error_handling_io.dart';
import '../base/file_system.dart';
import '../base/io.dart' as io;
import '../base/logger.dart';
......@@ -333,11 +333,14 @@ class _DefaultPub implements Pub {
bool touchesPackageConfig = false,
bool generateSyntheticPackage = false,
}) async {
final io.Process process = await _processUtils.start(
// Fully resolved pub or pub.bat is calculated based on current platform.
final io.Process process = await ErrorHandlingProcessManager.skipCommandLookup(() async {
return _processUtils.start(
_pubCommand(arguments),
workingDirectory: directory,
environment: await _createPubEnvironment(PubContext.interactive),
);
});
// Pipe the Flutter tool stdin to the pub stdin.
unawaited(process.stdin.addStream(stdio.stdin)
......
......@@ -15,6 +15,7 @@ import 'package:flutter_tools/src/globals.dart' as globals show flutterUsage;
import 'package:flutter_tools/src/reporting/reporting.dart';
import 'package:mockito/mockito.dart';
import 'package:path/path.dart' as path; // ignore: package_path_import
import 'package:process/process.dart';
import '../../src/common.dart';
import '../../src/context.dart';
......@@ -669,6 +670,25 @@ void main() {
});
});
test('skipCommandLookup invokes Process calls directly', () async {
final ErrorHandlingProcessManager processManager = ErrorHandlingProcessManager(
delegate: const LocalProcessManager(),
platform: windowsPlatform,
);
// Throws an argument error because package:process fails to locate the executable.
expect(() => processManager.runSync(<String>['foo']), throwsArgumentError);
expect(() => processManager.run(<String>['foo']), throwsArgumentError);
expect(() => processManager.start(<String>['foo']), throwsArgumentError);
// Throws process exception because the executable does not exist.
await ErrorHandlingProcessManager.skipCommandLookup<void>(() async {
expect(() => processManager.runSync(<String>['foo']), throwsA(isA<ProcessException>()));
expect(() => processManager.run(<String>['foo']), throwsA(isA<ProcessException>()));
expect(() => processManager.start(<String>['foo']), throwsA(isA<ProcessException>()));
});
});
group('ProcessManager on windows throws tool exit', () {
const int kDeviceFull = 112;
const int kUserMappedSectionOpened = 1224;
......
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