Fix bug parsing Gradle version. (#8326)

* Fix bug parsing Gradle version.

Version from pub_semver requires versions of the format X.Y.Z. Gradle
doesn't follow semantic versioning, though, so version parsing would
fail on versions like '3.2'. Fixed by writing a custom Version class.

Also removed a check for apksigner when building Gradle-based projects.

Fixes #8298

packages/flutter_tools/lib/src/android/android_sdk.dart

@@ -120,14 +120,14 @@ class AndroidSdk {
 
   /// Validate the Android SDK. This returns an empty list if there are no
   /// issues; otherwise, it returns a list of issues found.
-  List<String> validateSdkWellFormed() {
+  List<String> validateSdkWellFormed({bool requireApkSigner = true}) {
     if (!processManager.canRun(adbPath))
       return <String>['Android SDK file not found: $adbPath.'];
 
     if (sdkVersions.isEmpty || latestVersion == null)
       return <String>['Android SDK is missing command line tools; download from https://goo.gl/XxQghQ'];
 
-    return latestVersion.validateSdkWellFormed();
+    return latestVersion.validateSdkWellFormed(requireApkSigner: requireApkSigner);
   }
 
   String getPlatformToolsPath(String binaryName) {
@@ -228,7 +228,7 @@ class AndroidSdkVersion implements Comparable<AndroidSdkVersion> {
 
   String get apksignerPath => getBuildToolsPath('apksigner');
 
-  List<String> validateSdkWellFormed() {
+  List<String> validateSdkWellFormed({bool requireApkSigner = true}) {
     if (_exists(androidJarPath) != null)
       return <String>[_exists(androidJarPath)];
 
@@ -241,7 +241,7 @@ class AndroidSdkVersion implements Comparable<AndroidSdkVersion> {
     if (_canRun(zipalignPath) != null)
       return <String>[_canRun(zipalignPath)];
 
-    if (_canRun(apksignerPath) != null)
+    if (requireApkSigner && _canRun(apksignerPath) != null)
       return <String>[_canRun(apksignerPath) + '\napksigner requires Android SDK Build Tools 24.0.3 or newer.'];
 
     return <String>[];

packages/flutter_tools/lib/src/android/android_studio.dart

@@ -2,14 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'package:pub_semver/pub_semver.dart';
-
 import '../base/common.dart';
 import '../base/context.dart';
 import '../base/file_system.dart';
 import '../base/os.dart';
 import '../base/platform.dart';
 import '../base/process_manager.dart';
+import '../base/version.dart';
 import '../globals.dart';
 import '../ios/plist_utils.dart';
 
@@ -42,12 +41,13 @@ String get gradleExecutable {
 }
 
 class AndroidStudio implements Comparable<AndroidStudio> {
-  AndroidStudio(this.directory, {this.version = '0.0', this.configured}) {
+  AndroidStudio(this.directory, {Version version, this.configured})
+      : this.version = version ?? Version.unknown {
     _init();
   }
 
   final String directory;
-  final String version;
+  final Version version;
   final String configured;
 
   String _gradlePath;
@@ -57,13 +57,17 @@ class AndroidStudio implements Comparable<AndroidStudio> {
   factory AndroidStudio.fromMacOSBundle(String bundlePath) {
     String studioPath = fs.path.join(bundlePath, 'Contents');
     String plistFile = fs.path.join(studioPath, 'Info.plist');
-    String version =
+    String versionString =
         getValueFromFile(plistFile, kCFBundleShortVersionStringKey);
+    Version version;
+    if (versionString != null)
+      version = new Version.parse(versionString);
     return new AndroidStudio(studioPath, version: version);
   }
 
   factory AndroidStudio.fromHomeDot(Directory homeDotDir) {
-    String version = homeDotDir.basename.substring('.AndroidStudio'.length);
+    Version version = new Version.parse(
+        homeDotDir.basename.substring('.AndroidStudio'.length));
     String installPath;
     try {
       installPath = fs
@@ -162,7 +166,7 @@ class AndroidStudio implements Comparable<AndroidStudio> {
   static List<AndroidStudio> _allLinuxOrWindows() {
     List<AndroidStudio> studios = <AndroidStudio>[];
 
-    bool _hasStudioAt(String path, {String newerThan}) {
+    bool _hasStudioAt(String path, {Version newerThan}) {
       return studios.any((AndroidStudio studio) {
         if (studio.directory != path) return false;
         if (newerThan != null) {
@@ -254,6 +258,5 @@ class AndroidStudio implements Comparable<AndroidStudio> {
   }
 
   @override
-  String toString() =>
-      version == '0.0' ? 'Android Studio (unknown)' : 'Android Studio $version';
+  String toString() => 'Android Studio ($version)';
 }

packages/flutter_tools/lib/src/android/android_studio_validator.dart

@@ -4,14 +4,13 @@
 
 import 'dart:async';
 
-import 'package:pub_semver/pub_semver.dart';
-
 import '../base/file_system.dart';
 import '../base/io.dart';
 import '../doctor.dart';
 import '../globals.dart';
 import '../base/platform.dart';
 import '../base/process_manager.dart';
+import '../base/version.dart';
 import 'android_studio.dart';
 
 class AndroidStudioValidator extends DoctorValidator {
@@ -39,9 +38,7 @@ class AndroidStudioValidator extends DoctorValidator {
   Future<ValidationResult> validate() async {
     List<ValidationMessage> messages = <ValidationMessage>[];
     ValidationType type = ValidationType.missing;
-    String studioVersionText = _studio.version == '0.0'
-        ? 'unknown version'
-        : 'version ${_studio.version}';
+    String studioVersionText = 'version ${_studio.version}';
     messages
         .add(new ValidationMessage('Android Studio at ${_studio.directory}'));
     if (_studio.isValid) {

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

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+class Version implements Comparable<Version> {
+  static final RegExp versionPattern =
+      new RegExp(r'^(\d+)(\.(\d+)(\.(\d+))?)?');
+
+  /// The major version number: "1" in "1.2.3".
+  final int major;
+
+  /// The minor version number: "2" in "1.2.3".
+  final int minor;
+
+  /// The patch version number: "3" in "1.2.3".
+  final int patch;
+
+  /// The original string representation of the version number.
+  ///
+  /// This preserves textual artifacts like leading zeros that may be left out
+  /// of the parsed version.
+  final String _text;
+
+  /// Creates a new [Version] object.
+  factory Version(int major, int minor, int patch, {String text}) {
+    if (text == null) {
+      text = major == null ? '0' : '$major';
+      if (minor != null) text = '$text.$minor';
+      if (patch != null) text = '$text.$patch';
+    }
+
+    return new Version._(major ?? 0, minor ?? 0, patch ?? 0, text);
+  }
+
+  Version._(this.major, this.minor, this.patch, this._text) {
+    if (major < 0)
+      throw new ArgumentError('Major version must be non-negative.');
+    if (minor < 0)
+      throw new ArgumentError('Minor version must be non-negative.');
+    if (patch < 0)
+      throw new ArgumentError('Patch version must be non-negative.');
+  }
+
+  /// Creates a new [Version] by parsing [text].
+  factory Version.parse(String text) {
+    Match match = versionPattern.firstMatch(text);
+    if (match == null) {
+      throw new FormatException('Could not parse "$text".');
+    }
+
+    try {
+      int major = int.parse(match[1] ?? '0');
+      int minor = int.parse(match[3] ?? '0');
+      int patch = int.parse(match[5] ?? '0');
+      return new Version._(major, minor, patch, text);
+    } on FormatException {
+      throw new FormatException('Could not parse "$text".');
+    }
+  }
+
+  static Version get unknown => new Version(0, 0, 0, text: 'unknown');
+
+  /// Two [Version]s are equal if their version numbers are. The version text
+  /// is ignored.
+  @override
+  bool operator ==(dynamic other) {
+    if (other is! Version)
+      return false;
+    return major == other.major && minor == other.minor && patch == other.patch;
+  }
+
+  @override
+  int get hashCode => major ^ minor ^ patch;
+
+  bool operator <(Version other) => compareTo(other) < 0;
+  bool operator >(Version other) => compareTo(other) > 0;
+  bool operator <=(Version other) => compareTo(other) <= 0;
+  bool operator >=(Version other) => compareTo(other) >= 0;
+
+  @override
+  int compareTo(Version other) {
+    if (major != other.major) return major.compareTo(other.major);
+    if (minor != other.minor) return minor.compareTo(other.minor);
+    return patch.compareTo(other.patch);
+  }
+
+  @override
+  String toString() => _text;
+}

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

@@ -583,7 +583,7 @@ Future<Null> buildAndroidWithGradle(
   if (androidSdk == null)
     throwToolExit('No Android SDK found. Try setting the ANDROID_HOME environment variable.');
 
-  List<String> validationResult = androidSdk.validateSdkWellFormed();
+  List<String> validationResult = androidSdk.validateSdkWellFormed(requireApkSigner: false);
   if (validationResult.isNotEmpty) {
     validationResult.forEach(printError);
     throwToolExit('Try re-installing or updating your Android SDK.');

packages/flutter_tools/test/utils_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:flutter_tools/src/base/utils.dart';
+import 'package:flutter_tools/src/base/version.dart';
 import 'package:test/test.dart';
 
 void main() {
@@ -79,4 +80,37 @@ baz=qux
       }
     });
   });
+
+  group('Version', () {
+    test('can parse and compare', () {
+      expect(Version.unknown.toString(), equals('unknown'));
+      expect(new Version(null, null, null).toString(), equals('0'));
+
+      Version v1 = new Version.parse('1');
+      expect(v1.major, equals(1));
+      expect(v1.minor, equals(0));
+      expect(v1.patch, equals(0));
+
+      expect(v1, greaterThan(Version.unknown));
+
+      Version v2 = new Version.parse('1.2');
+      expect(v2.major, equals(1));
+      expect(v2.minor, equals(2));
+      expect(v2.patch, equals(0));
+
+      Version v3 = new Version.parse('1.2.3');
+      expect(v3.major, equals(1));
+      expect(v3.minor, equals(2));
+      expect(v3.patch, equals(3));
+
+      Version v4 = new Version.parse('1.12');
+      expect(v4, greaterThan(v2));
+
+      expect(v3, greaterThan(v2));
+      expect(v2, greaterThan(v1));
+
+      Version v5 = new Version(1, 2, 0, text: 'foo');
+      expect(v5, equals(v2));
+    });
+  });
 }