[flutter_tools] Have FlutterValidator fail on non-ideal git config (#103259)

Author: royarg02

packages/flutter_tools/lib/src/base/user_messages.dart

@@ -16,11 +16,13 @@ class UserMessages {
   // Messages used in FlutterValidator
   String flutterStatusInfo(String? channel, String? version, String os, String locale) =>
       'Channel ${channel ?? 'unknown'}, ${version ?? 'Unknown'}, on $os, locale $locale';
-  String flutterVersion(String version, String flutterRoot) =>
-      'Flutter version $version at $flutterRoot';
+  String flutterVersion(String version, String channel, String flutterRoot) =>
+      'Flutter version $version on channel $channel at $flutterRoot';
   String flutterRevision(String revision, String age, String date) =>
       'Framework revision $revision ($age), $date';
   String flutterUpstreamRepositoryUrl(String url) => 'Upstream repository $url';
+  String flutterUpstreamRepositoryUrlEnvMismatch(String url) => 'Upstream repository $url is not the same as FLUTTER_GIT_URL';
+  String flutterUpstreamRepositoryUrlNonStandard(String url) => 'Upstream repository $url is not a standard remote';
   String flutterGitUrl(String url) => 'FLUTTER_GIT_URL = $url';
   String engineRevision(String revision) => 'Engine revision $revision';
   String dartRevision(String revision) => 'Dart version $revision';

packages/flutter_tools/lib/src/doctor.dart

@@ -381,7 +381,7 @@ class Doctor {
       }
 
       for (final ValidationMessage message in result.messages) {
-        if (message.type != ValidationMessageType.information || verbose == true) {
+        if (!message.isInformation || verbose == true) {
           int hangingIndent = 2;
           int indent = 4;
           final String indicator = showColor ? message.coloredIndicator : message.indicator;
@@ -467,20 +467,17 @@ class FlutterValidator extends DoctorValidator {
   @override
   Future<ValidationResult> validate() async {
     final List<ValidationMessage> messages = <ValidationMessage>[];
-    ValidationType valid = ValidationType.installed;
     String? versionChannel;
     String? frameworkVersion;
 
     try {
       final FlutterVersion version = _flutterVersion();
+      final String? gitUrl = _platform.environment['FLUTTER_GIT_URL'];
       versionChannel = version.channel;
       frameworkVersion = version.frameworkVersion;
-      messages.add(ValidationMessage(_userMessages.flutterVersion(
-        frameworkVersion,
-        _flutterRoot(),
-      )));
-      messages.add(ValidationMessage(_userMessages.flutterUpstreamRepositoryUrl(version.repositoryUrl ?? 'unknown')));
-      final String? gitUrl = _platform.environment['FLUTTER_GIT_URL'];
+
+      messages.add(_getFlutterVersionMessage(frameworkVersion, versionChannel));
+      messages.add(_getFlutterUpstreamMessage(version));
       if (gitUrl != null) {
         messages.add(ValidationMessage(_userMessages.flutterGitUrl(gitUrl)));
       }
@@ -502,7 +499,6 @@ class FlutterValidator extends DoctorValidator {
       }
     } on VersionCheckError catch (e) {
       messages.add(ValidationMessage.error(e.message));
-      valid = ValidationType.partial;
     }
 
     // Check that the binaries we downloaded for this platform actually run on it.
@@ -516,9 +512,12 @@ class FlutterValidator extends DoctorValidator {
         buffer.writeln(_userMessages.flutterBinariesLinuxRepairCommands);
       }
       messages.add(ValidationMessage.error(buffer.toString()));
-      valid = ValidationType.partial;
     }
 
+    final ValidationType valid = messages.every((ValidationMessage message) => message.isInformation)
+      ? ValidationType.installed
+      : ValidationType.partial;
+
     return ValidationResult(
       valid,
       messages,
@@ -531,6 +530,40 @@ class FlutterValidator extends DoctorValidator {
     );
   }
 
+  ValidationMessage _getFlutterVersionMessage(String frameworkVersion, String versionChannel) {
+    final String flutterVersionMessage = _userMessages.flutterVersion(frameworkVersion, versionChannel, _flutterRoot());
+
+    // The tool sets the channel as "unknown", if the current branch is on a
+    // "detached HEAD" state or doesn't have an upstream, and sets the
+    // frameworkVersion as "0.0.0-unknown" if  "git describe" on HEAD doesn't
+    // produce an expected format to be parsed for the frameworkVersion.
+    if (versionChannel == 'unknown' || frameworkVersion == '0.0.0-unknown') {
+      return ValidationMessage.hint(flutterVersionMessage);
+    }
+    return ValidationMessage(flutterVersionMessage);
+  }
+
+  ValidationMessage _getFlutterUpstreamMessage(FlutterVersion version) {
+    final String? repositoryUrl = version.repositoryUrl;
+    final VersionCheckError? upstreamValidationError = VersionUpstreamValidator(version: version, platform: _platform).run();
+
+    // VersionUpstreamValidator can produce an error if repositoryUrl is null
+    if (upstreamValidationError != null) {
+      final String errorMessage = upstreamValidationError.message;
+      if (errorMessage.contains('could not determine the remote upstream which is being tracked')) {
+        return ValidationMessage.hint(_userMessages.flutterUpstreamRepositoryUrl('unknown'));
+      }
+      // At this point, repositoryUrl must not be null
+      if (errorMessage.contains('Flutter SDK is tracking a non-standard remote')) {
+        return ValidationMessage.hint(_userMessages.flutterUpstreamRepositoryUrlNonStandard(repositoryUrl!));
+      }
+      if (errorMessage.contains('Either remove "FLUTTER_GIT_URL" from the environment or set it to')){
+        return ValidationMessage.hint(_userMessages.flutterUpstreamRepositoryUrlEnvMismatch(repositoryUrl!));
+      }
+    }
+    return ValidationMessage(_userMessages.flutterUpstreamRepositoryUrl(repositoryUrl!));
+  }
+
   bool _genSnapshotRuns(String genSnapshotPath) {
     const int kExpectedExitCode = 255;
     try {

packages/flutter_tools/lib/src/doctor_validator.dart

@@ -252,6 +252,8 @@ class ValidationMessage {
 
   bool get isHint => type == ValidationMessageType.hint;
 
+  bool get isInformation => type == ValidationMessageType.information;
+
   String get indicator {
     switch (type) {
       case ValidationMessageType.error:

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

@@ -33,6 +33,7 @@ void main() {
     'downloaded and exits with code 1', () async {
     final FakeFlutterVersion flutterVersion = FakeFlutterVersion(
       frameworkVersion: '1.0.0',
+      channel: 'beta',
     );
     final MemoryFileSystem fileSystem = MemoryFileSystem.test();
     final Artifacts artifacts = Artifacts.test();
@@ -60,7 +61,7 @@ void main() {
 
     expect(await flutterValidator.validate(), _matchDoctorValidation(
       validationType: ValidationType.partial,
-      statusInfo: 'Channel unknown, 1.0.0, on Linux, locale en_US.UTF-8',
+      statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
       messages: containsAll(const <ValidationMessage>[
         ValidationMessage.error(
           'Downloaded executables cannot execute on host.\n'
@@ -76,6 +77,7 @@ void main() {
   testWithoutContext('FlutterValidator does not run gen_snapshot binary check if it is not already downloaded', () async {
     final FakeFlutterVersion flutterVersion = FakeFlutterVersion(
       frameworkVersion: '1.0.0',
+      channel: 'beta',
     );
     final FlutterValidator flutterValidator = FlutterValidator(
       platform: FakePlatform(
@@ -97,7 +99,7 @@ void main() {
     // fail if the gen_snapshot binary is not present.
     expect(await flutterValidator.validate(), _matchDoctorValidation(
       validationType: ValidationType.installed,
-      statusInfo: 'Channel unknown, 1.0.0, on Windows, locale en_US.UTF-8',
+      statusInfo: 'Channel beta, 1.0.0, on Windows, locale en_US.UTF-8',
       messages: anything,
     ));
   });
@@ -117,9 +119,9 @@ void main() {
 
     expect(await flutterValidator.validate(), _matchDoctorValidation(
       validationType: ValidationType.partial,
-      statusInfo: 'Channel unknown, 0.0.0, on Windows, locale en_US.UTF-8',
+      statusInfo: 'Channel beta, 0.0.0, on Windows, locale en_US.UTF-8',
       messages: containsAll(const <ValidationMessage>[
-        ValidationMessage('Flutter version 0.0.0 at sdk/flutter'),
+        ValidationMessage('Flutter version 0.0.0 on channel beta at sdk/flutter'),
         ValidationMessage.error('version error'),
       ]),
     ));
@@ -128,6 +130,7 @@ void main() {
   testWithoutContext('FlutterValidator shows mirrors on pub and flutter cloud storage', () async {
     final FakeFlutterVersion flutterVersion = FakeFlutterVersion(
       frameworkVersion: '1.0.0',
+      channel: 'beta',
     );
     final Platform platform = FakePlatform(
       operatingSystem: 'windows',
@@ -153,23 +156,26 @@ void main() {
 
     expect(await flutterValidator.validate(), _matchDoctorValidation(
       validationType: ValidationType.installed,
-      statusInfo: 'Channel unknown, 1.0.0, on Windows, locale en_US.UTF-8',
+      statusInfo: 'Channel beta, 1.0.0, on Windows, locale en_US.UTF-8',
       messages: containsAll(const <ValidationMessage>[
         ValidationMessage('Pub download mirror https://example.com/pub'),
         ValidationMessage('Flutter download mirror https://example.com/flutter'),
       ])
     ));
   });
 
-  testWithoutContext('FlutterValidator shows FLUTTER_GIT_URL environment variable when set', () async {
+  testWithoutContext('FlutterValidator shows FLUTTER_GIT_URL when set and fails if upstream is not the same', () async {
     final FlutterValidator flutterValidator = FlutterValidator(
       platform: FakePlatform(
         localeName: 'en_US.UTF-8',
         environment: <String, String> {
           'FLUTTER_GIT_URL': 'https://githubmirror.com/flutter.git',
         },
       ),
-      flutterVersion: () => FakeFlutterVersion(frameworkVersion: '1.0.0'),
+      flutterVersion: () => FakeFlutterVersion(
+        frameworkVersion: '1.0.0',
+        channel: 'beta'
+      ),
       devToolsVersion: () => '2.8.0',
       userMessages: UserMessages(),
       artifacts: Artifacts.test(),
@@ -180,17 +186,69 @@ void main() {
     );
 
     expect(await flutterValidator.validate(), _matchDoctorValidation(
-      validationType: ValidationType.installed,
+      validationType: ValidationType.partial,
+      statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
+      messages: containsAll(const <ValidationMessage>[
+        ValidationMessage.hint('Upstream repository https://github.com/flutter/flutter.git is not the same as FLUTTER_GIT_URL'),
+        ValidationMessage('FLUTTER_GIT_URL = https://githubmirror.com/flutter.git'),
+      ]),
+    ));
+  });
+
+  testWithoutContext('FlutterValidator fails when channel is unknown', () async {
+    final FlutterValidator flutterValidator = FlutterValidator(
+      platform: FakePlatform(localeName: 'en_US.UTF-8'),
+      flutterVersion: () => FakeFlutterVersion(
+        frameworkVersion: '1.0.0',
+        // channel is unknown by default
+      ),
+      devToolsVersion: () => '2.8.0',
+      userMessages: UserMessages(),
+      artifacts: Artifacts.test(),
+      fileSystem: MemoryFileSystem.test(),
+      processManager: FakeProcessManager.any(),
+      operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
+      flutterRoot: () => 'sdk/flutter',
+    );
+
+    expect(await flutterValidator.validate(), _matchDoctorValidation(
+      validationType: ValidationType.partial,
       statusInfo: 'Channel unknown, 1.0.0, on Linux, locale en_US.UTF-8',
-      messages: contains(const ValidationMessage('FLUTTER_GIT_URL = https://githubmirror.com/flutter.git')),
+      messages: contains(const ValidationMessage.hint('Flutter version 1.0.0 on channel unknown at sdk/flutter')),
+    ));
+  });
+
+  testWithoutContext('FlutterValidator fails when framework version is unknown', () async {
+    final FlutterValidator flutterValidator = FlutterValidator(
+      platform: FakePlatform(localeName: 'en_US.UTF-8'),
+      flutterVersion: () => FakeFlutterVersion(
+        frameworkVersion: '0.0.0-unknown',
+        channel: 'beta',
+      ),
+      devToolsVersion: () => '2.8.0',
+      userMessages: UserMessages(),
+      artifacts: Artifacts.test(),
+      fileSystem: MemoryFileSystem.test(),
+      processManager: FakeProcessManager.any(),
+      operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
+      flutterRoot: () => 'sdk/flutter',
+    );
+
+    expect(await flutterValidator.validate(), _matchDoctorValidation(
+      validationType: ValidationType.partial,
+      statusInfo: 'Channel beta, 0.0.0-unknown, on Linux, locale en_US.UTF-8',
+      messages: contains(const ValidationMessage.hint('Flutter version 0.0.0-unknown on channel beta at sdk/flutter')),
     ));
   });
 
   group('FlutterValidator shows flutter upstream remote', () {
-    testWithoutContext('default url', () async {
+    testWithoutContext('standard url', () async {
       final FlutterValidator flutterValidator = FlutterValidator(
         platform: FakePlatform(localeName: 'en_US.UTF-8'),
-        flutterVersion: () => FakeFlutterVersion(frameworkVersion: '1.0.0'),
+        flutterVersion: () => FakeFlutterVersion(
+          frameworkVersion: '1.0.0',
+          channel: 'beta'
+        ),
         devToolsVersion: () => '2.8.0',
         userMessages: UserMessages(),
         artifacts: Artifacts.test(),
@@ -202,16 +260,41 @@ void main() {
 
       expect(await flutterValidator.validate(), _matchDoctorValidation(
         validationType: ValidationType.installed,
-        statusInfo: 'Channel unknown, 1.0.0, on Linux, locale en_US.UTF-8',
+        statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
         messages: contains(const ValidationMessage('Upstream repository https://github.com/flutter/flutter.git')),
       ));
     });
 
-    testWithoutContext('unknown url if upstream is null', () async {
+    testWithoutContext('non-standard url', () async {
       final FlutterValidator flutterValidator = FlutterValidator(
         platform: FakePlatform(localeName: 'en_US.UTF-8'),
         flutterVersion: () => FakeFlutterVersion(
           frameworkVersion: '1.0.0',
+          channel: 'beta',
+          repositoryUrl: 'https://githubmirror.com/flutter.git'
+        ),
+        devToolsVersion: () => '2.8.0',
+        userMessages: UserMessages(),
+        artifacts: Artifacts.test(),
+        fileSystem: MemoryFileSystem.test(),
+        processManager: FakeProcessManager.any(),
+        operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
+        flutterRoot: () => 'sdk/flutter',
+      );
+
+      expect(await flutterValidator.validate(), _matchDoctorValidation(
+        validationType: ValidationType.partial,
+        statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
+        messages: contains(const ValidationMessage.hint('Upstream repository https://githubmirror.com/flutter.git is not a standard remote')),
+      ));
+    });
+
+    testWithoutContext('as unknown if upstream is null', () async {
+      final FlutterValidator flutterValidator = FlutterValidator(
+        platform: FakePlatform(localeName: 'en_US.UTF-8'),
+        flutterVersion: () => FakeFlutterVersion(
+          frameworkVersion: '1.0.0',
+          channel: 'beta',
           repositoryUrl: null,
         ),
         devToolsVersion: () => '2.8.0',
@@ -224,9 +307,9 @@ void main() {
       );
 
       expect(await flutterValidator.validate(), _matchDoctorValidation(
-        validationType: ValidationType.installed,
-        statusInfo: 'Channel unknown, 1.0.0, on Linux, locale en_US.UTF-8',
-        messages: contains(const ValidationMessage('Upstream repository unknown')),
+        validationType: ValidationType.partial,
+        statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
+        messages: contains(const ValidationMessage.hint('Upstream repository unknown')),
       ));
     });
   });
@@ -240,6 +323,9 @@ class FakeOperatingSystemUtils extends Fake implements OperatingSystemUtils {
 }
 
 class FakeThrowingFlutterVersion extends FakeFlutterVersion {
+  @override
+  String get channel => 'beta';
+
   @override
   String get frameworkCommitDate {
     throw VersionCheckError('version error');

packages/flutter_tools/test/general.shard/runner/runner_test.dart

@@ -191,13 +191,13 @@ void main() {
       expect(logContents, contains('flutter crash'));
       expect(logContents, contains('Exception: an exception % --'));
       expect(logContents, contains('CrashingFlutterCommand.runCommand'));
-      expect(logContents, contains('[✓] Flutter'));
+      expect(logContents, contains('[!] Flutter'));
 
       final CrashDetails sentDetails = (globals.crashReporter as WaitingCrashReporter)._details;
       expect(sentDetails.command, 'flutter crash');
       expect(sentDetails.error.toString(), 'Exception: an exception % --');
       expect(sentDetails.stackTrace.toString(), contains('CrashingFlutterCommand.runCommand'));
-      expect(await sentDetails.doctorText.text, contains('[✓] Flutter'));
+      expect(await sentDetails.doctorText.text, contains('[!] Flutter'));
     }, overrides: <Type, Generator>{
       Platform: () => FakePlatform(
         environment: <String, String>{