Suppress Flutter update check if --machine is present at all. (#150138)

Fixes https://github.com/flutter/flutter/issues/145158.

In an ideal world, the `--machine` flag would be strictly a global flag which sub-commands can choose to use (or perhaps just to report a `toolExit` that they don't have a `--machine` supported-mode if not. However currently, there is both a global flag, and command-specific flags.

This leads to the confusing scenario where:
```sh
flutter devices --machine
```

... still checks for a Flutter update, printing a banner and breaking the JSON output.

This PR "fixes" that by allowing `--machine` _anywhere_ in the command-line arguments to suppress the check.

/cc @johnmccutchan.
This commit is contained in:
Matan Lurey 2024-06-13 12:35:49 -07:00 committed by GitHub
parent f56e0c2845
commit 0cc27b993f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 89 additions and 9 deletions

View File

@ -227,6 +227,9 @@ class FlutterCommandRunner extends CommandRunner<void> {
}
}
// See https://github.com/flutter/flutter/issues/145158.
late bool _machineFlagPresentInAnyCliArg;
@override
Future<void> run(Iterable<String> args) {
// Have invocations of 'build', 'custom-devices', and 'pub' print out
@ -242,9 +245,62 @@ class FlutterCommandRunner extends CommandRunner<void> {
}
}
_machineFlagPresentInAnyCliArg = args.contains('--${FlutterGlobalOptions.kMachineFlag}');
return super.run(args);
}
/// Whether to perform a flutter version check, which prints a warning if old.
///
/// This method should be narrowly used in the following manner:
/// ```dart
/// final bool topLevelMachineFlag = topLevelResults[FlutterGlobalOptions.kMachineFlag] as bool? ?? false;
/// if (await _shouldCheckForUpdates(topLevelResults, topLevelMachineFlag: topLevelMachineFlag)) {
/// await globals.flutterVersion.checkFlutterVersionFreshness();
/// }
/// ```
Future<bool> _shouldCheckForUpdates(
ArgResults topLevelResults, {
required bool topLevelMachineFlag,
}) async {
// Check if the user has explicitly requested a version check.
final bool versionCheckFlag = topLevelResults[FlutterGlobalOptions.kVersionCheckFlag] as bool? ?? false;
final bool explicitVersionCheckPassed = topLevelResults.wasParsed(FlutterGlobalOptions.kVersionCheckFlag) && versionCheckFlag;
if (explicitVersionCheckPassed) {
return true;
}
// If the top level --machine flag is set, we don't want to check for updates.
if (topLevelMachineFlag) {
return false;
}
// Running the "upgrade" command is already checking, don't check twice.
if (topLevelResults.command?.name == 'upgrade') {
return false;
}
// If the same flag appears in any subcommand, we don't want to check for updates.
//
// A better solution would be the flag not being in any specific subcommand, just
// in the top level command, but that would require a more significant refactor
// and deprecation of the current behaviors.
//
// See https://github.com/flutter/flutter/issues/145158.
if (_machineFlagPresentInAnyCliArg) {
return false;
}
// e.g. `flutter bash-completion` or `flutter zsh-completion`
final bool isShellCompletionCommand = !globals.stdio.hasTerminal && (topLevelResults.command?.name ?? '').endsWith('-completion');
if (isShellCompletionCommand || await globals.botDetector.isRunningOnBot) {
return false;
}
// Otherwise, check for updates based on the flag which is typically set by default.
return versionCheckFlag;
}
@override
Future<void> runCommand(ArgResults topLevelResults) async {
final Map<Type, Object?> contextOverrides = <Type, Object?>{};
@ -323,15 +379,7 @@ class FlutterCommandRunner extends CommandRunner<void> {
globals.flutterVersion.ensureVersionFile();
final bool machineFlag = topLevelResults[FlutterGlobalOptions.kMachineFlag] as bool? ?? false;
final bool ci = await globals.botDetector.isRunningOnBot;
final bool redirectedCompletion = !globals.stdio.hasTerminal &&
(topLevelResults.command?.name ?? '').endsWith('-completion');
final bool isMachine = machineFlag || ci || redirectedCompletion;
final bool versionCheckFlag = topLevelResults[FlutterGlobalOptions.kVersionCheckFlag] as bool? ?? false;
final bool explicitVersionCheckPassed = topLevelResults.wasParsed(FlutterGlobalOptions.kVersionCheckFlag) && versionCheckFlag;
if (topLevelResults.command?.name != 'upgrade' &&
(explicitVersionCheckPassed || (versionCheckFlag && !isMachine))) {
if (await _shouldCheckForUpdates(topLevelResults, topLevelMachineFlag: machineFlag)) {
await globals.flutterVersion.checkFlutterVersionFreshness();
}

View File

@ -88,6 +88,21 @@ void main() {
OutputPreferences: () => OutputPreferences.test(),
});
testUsingContext('does not check that Flutter installation is up-to-date with --machine flag present anywhere', () async {
final FlutterCommandRunner runner = createTestCommandRunner(_FlutterCommandWithItsOwnMachineFlag()) as FlutterCommandRunner;
final FakeFlutterVersion version = globals.flutterVersion as FakeFlutterVersion;
await runner.run(<String>['dummy-with-machine', '--machine']);
expect(version.didCheckFlutterVersionFreshness, false);
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Platform: () => platform,
FlutterVersion: () => FakeFlutterVersion(),
OutputPreferences: () => OutputPreferences.test(),
});
testUsingContext('does not check that Flutter installation is up-to-date with CI=true in environment', () async {
final FlutterCommandRunner runner = createTestCommandRunner(DummyFlutterCommand()) as FlutterCommandRunner;
final FakeFlutterVersion version = globals.flutterVersion as FakeFlutterVersion;
@ -339,3 +354,20 @@ class FakeStdio extends Stdio {
@override
bool get supportsAnsiEscapes => hasFakeTerminal;
}
final class _FlutterCommandWithItsOwnMachineFlag extends FlutterCommand {
_FlutterCommandWithItsOwnMachineFlag() {
argParser.addFlag('machine', negatable: false);
}
@override
String get name => 'dummy-with-machine';
@override
Future<FlutterCommandResult> runCommand() async {
return FlutterCommandResult.success();
}
@override
String get description => 'does nothing, this time with --machine';
}