Prefer stable versions over pre-release versions in the solver.

R=nweiz@google.com

Review URL: https://codereview.chromium.org//64213004

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@30125 260f80e4-7a28-3924-810f-c04153c831b5

lib/src/solver/version_solver.dart

@@ -121,7 +121,12 @@ class PubspecCache {
   /// or returns `null` if not in the cache.
   Pubspec getCachedPubspec(PackageId id) => _pubspecs[id];
 
-  /// Gets the list of versions for [package] in descending order.
+  /// Gets the list of versions for [package].
+  ///
+  /// Packages are sorted in descending version order with all "stable"
+  /// versions (i.e. ones without a prerelease suffix) before pre-release
+  /// versions. This ensures that the solver prefers stable packages over
+  /// unstable ones.
   Future<List<PackageId>> getVersions(PackageRef package) {
     if (package.isRoot) {
       throw new StateError("Cannot get versions for root package $package.");
@@ -140,7 +145,14 @@ class PubspecCache {
     return source.getVersions(package.name, package.description)
         .then((versions) {
       // Sort by descending version so we try newer versions first.
-      versions.sort((a, b) => b.compareTo(a));
+      versions.sort((a, b) {
+        // Sort all prerelease versions after all normal versions. This way
+        // the solver will prefer stable packages over unstable ones.
+        if (a.isPreRelease && !b.isPreRelease) return 1;
+        if (!a.isPreRelease && b.isPreRelease) return -1;
+
+        return b.compareTo(a);
+      });
 
       var ids = versions.map((version) => package.atVersion(version)).toList();
       _versions[package] = ids;

test/version_solver_test.dart

@@ -40,6 +40,7 @@ main() {
   group('bad source', badSource);
   group('backtracking', backtracking);
   group('SDK constraint', sdkConstraint);
+  group('pre-release', prerelease);
 }
 
 void basicGraph() {
@@ -800,6 +801,60 @@ sdkConstraint() {
   }, useBleedingEdgeSdkVersion: true);
 }
 
+void prerelease() {
+  testResolve('prefer stable versions over unstable', {
+    'myapp 0.0.0': {
+      'a': 'any'
+    },
+    'a 1.0.0': {},
+    'a 1.1.0-dev': {},
+    'a 2.0.0-dev': {},
+    'a 3.0.0-dev': {}
+  }, result: {
+    'myapp from root': '0.0.0',
+    'a': '1.0.0'
+  });
+
+  testResolve('use latest allowed prerelease if no stable versions match', {
+    'myapp 0.0.0': {
+      'a': '<2.0.0'
+    },
+    'a 1.0.0-dev': {},
+    'a 1.1.0-dev': {},
+    'a 2.0.0-dev': {},
+    'a 3.0.0': {}
+  }, result: {
+    'myapp from root': '0.0.0',
+    'a': '2.0.0-dev'
+  });
+
+  testResolve('use an earlier stable version on a < constraint', {
+    'myapp 0.0.0': {
+      'a': '<2.0.0'
+    },
+    'a 1.0.0': {},
+    'a 1.1.0': {},
+    'a 2.0.0-dev': {},
+    'a 2.0.0': {}
+  }, result: {
+    'myapp from root': '0.0.0',
+    'a': '1.1.0'
+  });
+
+  testResolve('prefer a stable version even if constraint mentions unstable', {
+    'myapp 0.0.0': {
+      'a': '<=2.0.0-dev'
+    },
+    'a 1.0.0': {},
+    'a 1.1.0': {},
+    'a 2.0.0-dev': {},
+    'a 2.0.0': {}
+  }, result: {
+    'myapp from root': '0.0.0',
+    'a': '1.1.0'
+  });
+}
+
 testResolve(description, packages, {
              lockfile, result, FailMatcherBuilder error, int maxTries,
              bool useBleedingEdgeSdkVersion}) {