Skip to content

Commit de2a424

Browse files
authored
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 flutter#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
1 parent 4f3ed80 commit de2a424

File tree

3 files changed

+109
-32
lines changed

3 files changed

+109
-32
lines changed

packages/flutter_tools/lib/src/commands/update_packages.dart

+1
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ class UpdatePackagesCommand extends FlutterCommand {
444444
upgrade: doUpgrade,
445445
offline: boolArgDeprecated('offline'),
446446
flutterRootOverride: temporaryFlutterSdk?.path,
447+
printProgress: false,
447448
);
448449

449450
if (doUpgrade) {

packages/flutter_tools/lib/src/dart/pub.dart

+55-32
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,6 @@ class _DefaultPub implements Pub {
349349
context: context,
350350
directory: directory,
351351
failureMessage: 'pub $command failed',
352-
retry: !offline,
353352
flutterRootOverride: flutterRootOverride,
354353
printProgress: printProgress
355354
);
@@ -379,59 +378,83 @@ class _DefaultPub implements Pub {
379378
/// Uses [ProcessStartMode.normal] and [Pub._stdio] if [Pub.test] constructor
380379
/// was used.
381380
///
382-
/// Prints the stdout and stderr of the whole run.
381+
/// Prints the stdout and stderr of the whole run, unless silenced using
382+
/// [printProgress].
383383
///
384384
/// Sends an analytics event.
385385
Future<void> _runWithStdioInherited(
386386
List<String> arguments, {
387387
required String command,
388388
required bool printProgress,
389389
required PubContext context,
390-
required bool retry,
391390
required String directory,
392391
String failureMessage = 'pub failed',
393392
String? flutterRootOverride,
394393
}) async {
395394
int exitCode;
395+
if (printProgress) {
396+
_logger.printStatus('Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...');
397+
}
396398

397-
_logger.printStatus('Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...');
398399
final List<String> pubCommand = _pubCommand(arguments);
399400
final Map<String, String> pubEnvironment = await _createPubEnvironment(context, flutterRootOverride);
400-
try {
401-
final io.Process process;
402-
final io.Stdio? stdio = _stdio;
403-
404-
if (stdio != null) {
405-
// Omit mode parameter and direct pub output to [Pub._stdio] for tests.
406-
process = await _processUtils.start(
407-
pubCommand,
408-
workingDirectory: _fileSystem.path.current,
409-
environment: pubEnvironment,
410-
);
411401

412-
final StreamSubscription<List<int>> stdoutSubscription =
413-
process.stdout.listen(stdio.stdout.add);
414-
final StreamSubscription<List<int>> stderrSubscription =
415-
process.stderr.listen(stdio.stderr.add);
416-
417-
await Future.wait<void>(<Future<void>>[
418-
stdoutSubscription.asFuture<void>(),
419-
stderrSubscription.asFuture<void>(),
420-
]);
421-
422-
unawaited(stdoutSubscription.cancel());
423-
unawaited(stderrSubscription.cancel());
402+
try {
403+
if (printProgress) {
404+
final io.Stdio? stdio = _stdio;
405+
if (stdio == null) {
406+
// Let pub inherit stdio and output directly to the tool's stdout and
407+
// stderr handles.
408+
final io.Process process = await _processUtils.start(
409+
pubCommand,
410+
workingDirectory: _fileSystem.path.current,
411+
environment: pubEnvironment,
412+
mode: ProcessStartMode.inheritStdio,
413+
);
414+
415+
exitCode = await process.exitCode;
416+
} else {
417+
// Omit [mode] parameter to send output to [process.stdout] and
418+
// [process.stderr].
419+
final io.Process process = await _processUtils.start(
420+
pubCommand,
421+
workingDirectory: _fileSystem.path.current,
422+
environment: pubEnvironment,
423+
);
424+
425+
// Direct pub output to [Pub._stdio] for tests.
426+
final StreamSubscription<List<int>> stdoutSubscription =
427+
process.stdout.listen(stdio.stdout.add);
428+
final StreamSubscription<List<int>> stderrSubscription =
429+
process.stderr.listen(stdio.stderr.add);
430+
431+
await Future.wait<void>(<Future<void>>[
432+
stdoutSubscription.asFuture<void>(),
433+
stderrSubscription.asFuture<void>(),
434+
]);
435+
436+
unawaited(stdoutSubscription.cancel());
437+
unawaited(stderrSubscription.cancel());
438+
439+
exitCode = await process.exitCode;
440+
}
424441
} else {
425-
// Let pub inherit stdio for normal operation.
426-
process = await _processUtils.start(
442+
// Do not try to use [ProcessUtils.start] here, because it requires you
443+
// to read all data out of the stdout and stderr streams. If you don't
444+
// read the streams, it may appear to work fine on your platform but
445+
// will block the tool's process on Windows.
446+
// See https://api.dart.dev/stable/dart-io/Process/start.html
447+
//
448+
// [ProcessUtils.run] will send the output to [result.stdout] and
449+
// [result.stderr], which we will ignore.
450+
final RunResult result = await _processUtils.run(
427451
pubCommand,
428452
workingDirectory: _fileSystem.path.current,
429453
environment: pubEnvironment,
430-
mode: ProcessStartMode.inheritStdio,
431454
);
432-
}
433455

434-
exitCode = await process.exitCode;
456+
exitCode = result.exitCode;
457+
}
435458
// The exception is rethrown, so don't catch only Exceptions.
436459
} catch (exception) { // ignore: avoid_catches_without_on_clauses
437460
if (exception is io.ProcessException) {

packages/flutter_tools/test/general.shard/dart/pub_get_test.dart

+53
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,59 @@ exit code: 66
641641
expect(processManager, hasNoRemainingExpectations);
642642
});
643643

644+
// Regression test for https://github.com/flutter/flutter/issues/116627
645+
testWithoutContext('pub get suppresses progress output', () async {
646+
final BufferLogger logger = BufferLogger.test();
647+
final FileSystem fileSystem = MemoryFileSystem.test();
648+
649+
final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
650+
const FakeCommand(
651+
command: <String>[
652+
'bin/cache/dart-sdk/bin/dart',
653+
'__deprecated_pub',
654+
'--directory',
655+
'.',
656+
'get',
657+
'--example',
658+
],
659+
stderr: 'err1\nerr2\nerr3\n',
660+
stdout: 'out1\nout2\nout3\n',
661+
environment: <String, String>{'FLUTTER_ROOT': '', 'PUB_ENVIRONMENT': 'flutter_cli:flutter_tests'},
662+
),
663+
]);
664+
665+
final FakeStdio mockStdio = FakeStdio();
666+
final Pub pub = Pub.test(
667+
platform: FakePlatform(),
668+
usage: TestUsage(),
669+
fileSystem: fileSystem,
670+
logger: logger,
671+
processManager: processManager,
672+
botDetector: const BotDetectorAlwaysNo(),
673+
stdio: mockStdio,
674+
);
675+
676+
try {
677+
await pub.get(
678+
project: FlutterProject.fromDirectoryTest(fileSystem.currentDirectory),
679+
context: PubContext.flutterTests,
680+
printProgress: false
681+
);
682+
} on ToolExit {
683+
// Ignore.
684+
}
685+
686+
expect(
687+
mockStdio.stdout.writes.map(utf8.decode),
688+
isNot(
689+
<String>[
690+
'out1\nout2\nout3\n',
691+
]
692+
)
693+
);
694+
expect(processManager, hasNoRemainingExpectations);
695+
});
696+
644697
testWithoutContext('pub cache in flutter root is ignored', () async {
645698
final FileSystem fileSystem = MemoryFileSystem.test();
646699
final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[

0 commit comments

Comments
 (0)