Skip to content

Commit

Permalink
Fix flutter update-packages regression by fixing parameters in "pub…
Browse files Browse the repository at this point in the history
… get" runner (flutter#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
  • Loading branch information
nehalvpatel authored Jan 6, 2023
1 parent 4f3ed80 commit de2a424
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ class UpdatePackagesCommand extends FlutterCommand {
upgrade: doUpgrade,
offline: boolArgDeprecated('offline'),
flutterRootOverride: temporaryFlutterSdk?.path,
printProgress: false,
);

if (doUpgrade) {
Expand Down
87 changes: 55 additions & 32 deletions packages/flutter_tools/lib/src/dart/pub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ class _DefaultPub implements Pub {
context: context,
directory: directory,
failureMessage: 'pub $command failed',
retry: !offline,
flutterRootOverride: flutterRootOverride,
printProgress: printProgress
);
Expand Down Expand Up @@ -379,59 +378,83 @@ 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(
List<String> arguments, {
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) {
Expand Down
53 changes: 53 additions & 0 deletions packages/flutter_tools/test/general.shard/dart/pub_get_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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>[
Expand Down

0 comments on commit de2a424

Please sign in to comment.