Unverified Commit de2a4249 authored by Nehal Patel's avatar Nehal Patel Committed by GitHub

Fix `flutter update-packages` regression by fixing parameters in "pub get" runner (#116687)

* Make pub get runner respect printProgress and retry parameters

* Fix typo

* Add regression test

* Improve test

* Fix implementation and test

* Test to fix flutter_drone tests

* Revert test

* Attempt #2 to fix flutter_drone tests

* Revert attempt

* Hack: Force printProgress to debug Windows tests

* Use ProcessUtils.run to avoid dangling stdout and stderr

* Update documentation

* Clean up retry argument
parent 4f3ed804
......@@ -444,6 +444,7 @@ class UpdatePackagesCommand extends FlutterCommand {
upgrade: doUpgrade,
offline: boolArgDeprecated('offline'),
flutterRootOverride: temporaryFlutterSdk?.path,
printProgress: false,
);
if (doUpgrade) {
......
......@@ -349,7 +349,6 @@ class _DefaultPub implements Pub {
context: context,
directory: directory,
failureMessage: 'pub $command failed',
retry: !offline,
flutterRootOverride: flutterRootOverride,
printProgress: printProgress
);
......@@ -379,7 +378,8 @@ class _DefaultPub implements Pub {
/// Uses [ProcessStartMode.normal] and [Pub._stdio] if [Pub.test] constructor
/// was used.
///
/// Prints the stdout and stderr of the whole run.
/// Prints the stdout and stderr of the whole run, unless silenced using
/// [printProgress].
///
/// Sends an analytics event.
Future<void> _runWithStdioInherited(
......@@ -387,51 +387,74 @@ class _DefaultPub implements Pub {
required String command,
required bool printProgress,
required PubContext context,
required bool retry,
required String directory,
String failureMessage = 'pub failed',
String? flutterRootOverride,
}) async {
int exitCode;
if (printProgress) {
_logger.printStatus('Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...');
}
_logger.printStatus('Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...');
final List<String> pubCommand = _pubCommand(arguments);
final Map<String, String> pubEnvironment = await _createPubEnvironment(context, flutterRootOverride);
try {
final io.Process process;
final io.Stdio? stdio = _stdio;
if (stdio != null) {
// Omit mode parameter and direct pub output to [Pub._stdio] for tests.
process = await _processUtils.start(
pubCommand,
workingDirectory: _fileSystem.path.current,
environment: pubEnvironment,
);
final StreamSubscription<List<int>> stdoutSubscription =
process.stdout.listen(stdio.stdout.add);
final StreamSubscription<List<int>> stderrSubscription =
process.stderr.listen(stdio.stderr.add);
await Future.wait<void>(<Future<void>>[
stdoutSubscription.asFuture<void>(),
stderrSubscription.asFuture<void>(),
]);
unawaited(stdoutSubscription.cancel());
unawaited(stderrSubscription.cancel());
try {
if (printProgress) {
final io.Stdio? stdio = _stdio;
if (stdio == null) {
// Let pub inherit stdio and output directly to the tool's stdout and
// stderr handles.
final io.Process process = await _processUtils.start(
pubCommand,
workingDirectory: _fileSystem.path.current,
environment: pubEnvironment,
mode: ProcessStartMode.inheritStdio,
);
exitCode = await process.exitCode;
} else {
// Omit [mode] parameter to send output to [process.stdout] and
// [process.stderr].
final io.Process process = await _processUtils.start(
pubCommand,
workingDirectory: _fileSystem.path.current,
environment: pubEnvironment,
);
// Direct pub output to [Pub._stdio] for tests.
final StreamSubscription<List<int>> stdoutSubscription =
process.stdout.listen(stdio.stdout.add);
final StreamSubscription<List<int>> stderrSubscription =
process.stderr.listen(stdio.stderr.add);
await Future.wait<void>(<Future<void>>[
stdoutSubscription.asFuture<void>(),
stderrSubscription.asFuture<void>(),
]);
unawaited(stdoutSubscription.cancel());
unawaited(stderrSubscription.cancel());
exitCode = await process.exitCode;
}
} else {
// Let pub inherit stdio for normal operation.
process = await _processUtils.start(
// Do not try to use [ProcessUtils.start] here, because it requires you
// to read all data out of the stdout and stderr streams. If you don't
// read the streams, it may appear to work fine on your platform but
// will block the tool's process on Windows.
// See https://api.dart.dev/stable/dart-io/Process/start.html
//
// [ProcessUtils.run] will send the output to [result.stdout] and
// [result.stderr], which we will ignore.
final RunResult result = await _processUtils.run(
pubCommand,
workingDirectory: _fileSystem.path.current,
environment: pubEnvironment,
mode: ProcessStartMode.inheritStdio,
);
}
exitCode = await process.exitCode;
exitCode = result.exitCode;
}
// The exception is rethrown, so don't catch only Exceptions.
} catch (exception) { // ignore: avoid_catches_without_on_clauses
if (exception is io.ProcessException) {
......
......@@ -641,6 +641,59 @@ exit code: 66
expect(processManager, hasNoRemainingExpectations);
});
// Regression test for https://github.com/flutter/flutter/issues/116627
testWithoutContext('pub get suppresses progress output', () async {
final BufferLogger logger = BufferLogger.test();
final FileSystem fileSystem = MemoryFileSystem.test();
final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
const FakeCommand(
command: <String>[
'bin/cache/dart-sdk/bin/dart',
'__deprecated_pub',
'--directory',
'.',
'get',
'--example',
],
stderr: 'err1\nerr2\nerr3\n',
stdout: 'out1\nout2\nout3\n',
environment: <String, String>{'FLUTTER_ROOT': '', 'PUB_ENVIRONMENT': 'flutter_cli:flutter_tests'},
),
]);
final FakeStdio mockStdio = FakeStdio();
final Pub pub = Pub.test(
platform: FakePlatform(),
usage: TestUsage(),
fileSystem: fileSystem,
logger: logger,
processManager: processManager,
botDetector: const BotDetectorAlwaysNo(),
stdio: mockStdio,
);
try {
await pub.get(
project: FlutterProject.fromDirectoryTest(fileSystem.currentDirectory),
context: PubContext.flutterTests,
printProgress: false
);
} on ToolExit {
// Ignore.
}
expect(
mockStdio.stdout.writes.map(utf8.decode),
isNot(
<String>[
'out1\nout2\nout3\n',
]
)
);
expect(processManager, hasNoRemainingExpectations);
});
testWithoutContext('pub cache in flutter root is ignored', () async {
final FileSystem fileSystem = MemoryFileSystem.test();
final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
......
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