[flutter_tool] Crash less when git fails during 'version' (#45628)

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

@@ -18,6 +18,7 @@ import '../version.dart';
 
 class VersionCommand extends FlutterCommand {
   VersionCommand() : super() {
+    usesPubOption(hide: true);
     argParser.addFlag('force',
       abbr: 'f',
       help: 'Force switch to older Flutter versions that do not include a version command',
@@ -117,7 +118,7 @@ class VersionCommand extends FlutterCommand {
     printStatus(flutterVersion.toString());
 
     final String projectRoot = findProjectRoot();
-    if (projectRoot != null) {
+    if (projectRoot != null && shouldRunPub) {
       printStatus('');
       await pub.get(
         context: PubContext.pubUpgrade,

packages/flutter_tools/lib/src/doctor.dart

@@ -596,8 +596,15 @@ class FlutterValidator extends DoctorValidator {
       final FlutterVersion version = FlutterVersion.instance;
       versionChannel = version.channel;
       frameworkVersion = version.frameworkVersion;
-      messages.add(ValidationMessage(userMessages.flutterVersion(frameworkVersion, Cache.flutterRoot)));
-      messages.add(ValidationMessage(userMessages.flutterRevision(version.frameworkRevisionShort, version.frameworkAge, version.frameworkDate)));
+      messages.add(ValidationMessage(userMessages.flutterVersion(
+        frameworkVersion,
+        Cache.flutterRoot,
+      )));
+      messages.add(ValidationMessage(userMessages.flutterRevision(
+        version.frameworkRevisionShort,
+        version.frameworkAge,
+        version.frameworkDate,
+      )));
       messages.add(ValidationMessage(userMessages.engineRevision(version.engineRevisionShort)));
       messages.add(ValidationMessage(userMessages.dartRevision(version.dartSdkVersion)));
     } on VersionCheckError catch (e) {

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -173,9 +173,10 @@ abstract class FlutterCommand extends Command<void> {
     return bundle.defaultMainPath;
   }
 
-  void usesPubOption() {
+  void usesPubOption({bool hide = false}) {
     argParser.addFlag('pub',
       defaultsTo: true,
+      hide: hide,
       help: 'Whether to run "flutter pub get" before executing this command.');
     _usesPubOption = true;
   }

packages/flutter_tools/lib/src/version.dart

@@ -130,9 +130,19 @@ class FlutterVersion {
   };
 
   /// A date String describing the last framework commit.
-  String get frameworkCommitDate => _latestGitCommitDate();
-
-  static String _latestGitCommitDate([ String branch ]) {
+  ///
+  /// If a git command fails, this will return a placeholder date.
+  String get frameworkCommitDate => _latestGitCommitDate(lenient: true);
+
+  // The date of the latest commit on the given branch. If no branch is
+  // specified, then it is the current local branch.
+  //
+  // If lenient is true, and the git command fails, a placeholder date is
+  // returned. Otherwise, the VersionCheckError exception is propagated.
+  static String _latestGitCommitDate({
+    String branch,
+    bool lenient = false,
+  }) {
     final List<String> args = gitLog(<String>[
       if (branch != null) branch,
       '-n',
@@ -140,7 +150,21 @@ class FlutterVersion {
       '--pretty=format:%ad',
       '--date=iso',
     ]);
+    try {
+      // Don't plumb 'lenient' through directly so that we can print an error
+      // if something goes wrong.
       return _runSync(args, lenient: false);
+    } on VersionCheckError catch (e) {
+      if (lenient) {
+        final DateTime dummyDate = DateTime.fromMillisecondsSinceEpoch(0);
+        printError('Failed to find the latest git commit date: $e\n'
+          'Returning $dummyDate instead.');
+        // Return something that DateTime.parse() can parse.
+        return dummyDate.toString();
+      } else {
+        rethrow;
+      }
+    }
   }
 
   /// The name of the temporary git remote used to check for the latest
@@ -153,8 +177,8 @@ class FlutterVersion {
 
   /// The date of the latest framework commit in the remote repository.
   ///
-  /// Throws [ToolExit] if a git command fails, for example, when the remote git
-  /// repository is not reachable due to a network issue.
+  /// Throws [VersionCheckError] if a git command fails, for example, when the
+  /// remote git repository is not reachable due to a network issue.
   static Future<String> fetchRemoteFrameworkCommitDate(String branch) async {
     await _removeVersionCheckRemoteIfExists();
     try {
@@ -166,7 +190,10 @@ class FlutterVersion {
         'https://github.com/flutter/flutter.git',
       ]);
       await _run(<String>['git', 'fetch', _versionCheckRemote, branch]);
-      return _latestGitCommitDate('$_versionCheckRemote/$branch');
+      return _latestGitCommitDate(
+        branch: '$_versionCheckRemote/$branch',
+        lenient: false,
+      );
     } finally {
       await _removeVersionCheckRemoteIfExists();
     }
@@ -219,7 +246,13 @@ class FlutterVersion {
     String tentativeAncestorRevision,
   }) {
     final ProcessResult result = processManager.runSync(
-      <String>['git', 'merge-base', '--is-ancestor', tentativeAncestorRevision, tentativeDescendantRevision],
+      <String>[
+        'git',
+        'merge-base',
+        '--is-ancestor',
+        tentativeAncestorRevision,
+        tentativeDescendantRevision
+      ],
       workingDirectory: Cache.flutterRoot,
     );
     return result.exitCode == 0;
@@ -291,7 +324,16 @@ class FlutterVersion {
       return;
     }
 
-    final DateTime localFrameworkCommitDate = DateTime.parse(frameworkCommitDate);
+    DateTime localFrameworkCommitDate;
+    try {
+      localFrameworkCommitDate = DateTime.parse(_latestGitCommitDate(
+        lenient: false
+      ));
+    } on VersionCheckError {
+      // Don't perform the update check if the verison check failed.
+      return;
+    }
+
     final Duration frameworkAge = _clock.now().difference(localFrameworkCommitDate);
     final bool installationSeemsOutdated = frameworkAge > versionAgeConsideredUpToDate(channel);
 
@@ -299,8 +341,7 @@ class FlutterVersion {
     // to the server if we haven't checked recently so won't happen on every
     // command.
     final DateTime latestFlutterCommitDate = await _getLatestAvailableFlutterDate();
-    final VersionCheckResult remoteVersionStatus =
-        latestFlutterCommitDate == null
+    final VersionCheckResult remoteVersionStatus = latestFlutterCommitDate == null
         ? VersionCheckResult.unknown
         : latestFlutterCommitDate.isAfter(localFrameworkCommitDate)
           ? VersionCheckResult.newVersionAvailable
@@ -379,7 +420,9 @@ class FlutterVersion {
     final VersionCheckStamp versionCheckStamp = await VersionCheckStamp.load();
 
     if (versionCheckStamp.lastTimeVersionWasChecked != null) {
-      final Duration timeSinceLastCheck = _clock.now().difference(versionCheckStamp.lastTimeVersionWasChecked);
+      final Duration timeSinceLastCheck = _clock.now().difference(
+        versionCheckStamp.lastTimeVersionWasChecked,
+      );
 
       // Don't ping the server too often. Return cached value if it's fresh.
       if (timeSinceLastCheck < checkAgeConsideredUpToDate) {
@@ -389,7 +432,9 @@ class FlutterVersion {
 
     // Cache is empty or it's been a while since the last server ping. Ping the server.
     try {
-      final DateTime remoteFrameworkCommitDate = DateTime.parse(await FlutterVersion.fetchRemoteFrameworkCommitDate(channel));
+      final DateTime remoteFrameworkCommitDate = DateTime.parse(
+        await FlutterVersion.fetchRemoteFrameworkCommitDate(channel),
+      );
       await versionCheckStamp.store(
         newTimeVersionWasChecked: _clock.now(),
         newKnownRemoteVersion: remoteFrameworkCommitDate,
@@ -533,7 +578,10 @@ class VersionCheckError implements Exception {
 /// If [lenient] is true and the command fails, returns an empty string.
 /// Otherwise, throws a [ToolExit] exception.
 String _runSync(List<String> command, { bool lenient = true }) {
-  final ProcessResult results = processManager.runSync(command, workingDirectory: Cache.flutterRoot);
+  final ProcessResult results = processManager.runSync(
+    command,
+    workingDirectory: Cache.flutterRoot,
+  );
 
   if (results.exitCode == 0) {
     return (results.stdout as String).trim();
@@ -542,6 +590,7 @@ String _runSync(List<String> command, { bool lenient = true }) {
   if (!lenient) {
     throw VersionCheckError(
       'Command exited with code ${results.exitCode}: ${command.join(' ')}\n'
+      'Standard out: ${results.stdout}\n'
       'Standard error: ${results.stderr}'
     );
   }

packages/flutter_tools/test/commands.shard/hermetic/version_test.dart

@@ -10,7 +10,6 @@ import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/version.dart';
-import 'package:flutter_tools/src/dart/pub.dart';
 import 'package:flutter_tools/src/version.dart';
 import 'package:mockito/mockito.dart';
 import 'package:process/process.dart';
@@ -27,60 +26,95 @@ void main() {
 
     testUsingContext('version ls', () async {
       final VersionCommand command = VersionCommand();
-      await createTestCommandRunner(command).run(<String>['version']);
+      await createTestCommandRunner(command).run(<String>[
+        'version',
+        '--no-pub',
+      ]);
       expect(testLogger.statusText, equals('v10.0.0\r\nv20.0.0\n' ''));
     }, overrides: <Type, Generator>{
       ProcessManager: () => MockProcessManager(),
-      Pub: () => const Pub(),
     });
 
     testUsingContext('version switch', () async {
       const String version = '10.0.0';
       final VersionCommand command = VersionCommand();
-      final Future<void> runCommand = createTestCommandRunner(command).run(<String>['version', version]);
-      await Future.wait<void>(<Future<void>>[runCommand]);
+      await createTestCommandRunner(command).run(<String>[
+        'version',
+        '--no-pub',
+        version,
+      ]);
       expect(testLogger.statusText, contains('Switching Flutter to version $version'));
     }, overrides: <Type, Generator>{
       ProcessManager: () => MockProcessManager(),
-      Pub: () => const Pub(),
+    });
+
+    testUsingContext('version switch, latest commit query fails', () async {
+      const String version = '10.0.0';
+      final VersionCommand command = VersionCommand();
+      await createTestCommandRunner(command).run(<String>[
+        'version',
+        '--no-pub',
+        version,
+      ]);
+      expect(testLogger.errorText, contains('git failed'));
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => MockProcessManager(latestCommitFails: true),
+    });
+
+    testUsingContext('latest commit is parsable when query fails', () {
+      final FlutterVersion flutterVersion = FlutterVersion();
+      expect(
+        () => DateTime.parse(flutterVersion.frameworkCommitDate),
+        returnsNormally,
+      );
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => MockProcessManager(latestCommitFails: true),
     });
 
     testUsingContext('switch to not supported version without force', () async {
       const String version = '1.1.5';
       final VersionCommand command = VersionCommand();
-      final Future<void> runCommand = createTestCommandRunner(command).run(<String>['version', version]);
-      await Future.wait<void>(<Future<void>>[runCommand]);
+      await createTestCommandRunner(command).run(<String>[
+        'version',
+        '--no-pub',
+        version,
+      ]);
       expect(testLogger.errorText, contains('Version command is not supported in'));
     }, overrides: <Type, Generator>{
       ProcessManager: () => MockProcessManager(),
-      Pub: () => const Pub(),
     });
 
     testUsingContext('switch to not supported version with force', () async {
       const String version = '1.1.5';
       final VersionCommand command = VersionCommand();
-      final Future<void> runCommand = createTestCommandRunner(command).run(<String>['version', '--force', version]);
-      await Future.wait<void>(<Future<void>>[runCommand]);
+      await createTestCommandRunner(command).run(<String>[
+        'version',
+        '--no-pub',
+        '--force',
+        version,
+      ]);
       expect(testLogger.statusText, contains('Switching Flutter to version $version with force'));
     }, overrides: <Type, Generator>{
       ProcessManager: () => MockProcessManager(),
-      Pub: () => const Pub(),
     });
 
     testUsingContext('tool exit on confusing version', () async {
       const String version = 'master';
       final VersionCommand command = VersionCommand();
-      final Future<void> runCommand = createTestCommandRunner(command).run(<String>['version', version]);
-      expect(() async => await Future.wait<void>(<Future<void>>[runCommand]),
-             throwsA(isInstanceOf<ToolExit>()));
+      expect(() async =>
+        await createTestCommandRunner(command).run(<String>[
+          'version',
+          '--no-pub',
+          version,
+        ]),
+        throwsA(isInstanceOf<ToolExit>()),
+      );
     }, overrides: <Type, Generator>{
       ProcessManager: () => MockProcessManager(),
-      Pub: () => const Pub(),
     });
 
     testUsingContext('exit tool if can\'t get the tags', () async {
       final VersionCommand command = VersionCommand();
-
       try {
         await command.getTags();
         fail('ToolExit expected');
@@ -94,11 +128,15 @@ void main() {
 }
 
 class MockProcessManager extends Mock implements ProcessManager {
-  MockProcessManager({ this.failGitTag = false });
+  MockProcessManager({
+    this.failGitTag = false,
+    this.latestCommitFails = false,
+  });
 
   String version = '';
 
-  bool failGitTag;
+  final bool failGitTag;
+  final bool latestCommitFails;
 
   @override
   Future<ProcessResult> run(
@@ -142,6 +180,14 @@ class MockProcessManager extends Mock implements ProcessManager {
         return ProcessResult(0, 0, '$version-0-g00000000', '');
       }
     }
+    final List<String> commitDateCommand = <String>[
+      '-n', '1',
+      '--pretty=format:%ad',
+      '--date=iso',
+    ];
+    if (latestCommitFails && commandStr == FlutterVersion.gitLog(commitDateCommand).join(' ')) {
+      return ProcessResult(0, -9, '', 'git failed');
+    }
     return ProcessResult(0, 0, '', '');
   }