expand the sdk prerelease override logic to work for any pre-release (#1705)

jakemac53 · web-flow · commit 04c7827915e1 · 2017-09-15T13:58:38.000-07:00
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart
@@ -196,7 +196,7 @@ class Entrypoint {
         lockFile: lockFile, useLatest: useLatest);
 
     // Log once about all overridden packages.
-    if (warnAboutPreReleaseTwoDotZeroSdkOverrides && result.pubspecs != null) {
+    if (warnAboutPreReleaseSdkOverrides && result.pubspecs != null) {
       var overriddenPackages = (result.pubspecs.values
               .where((pubspec) => pubspec.dartSdkWasOverridden)
               .map((pubspec) => pubspec.name)
@@ -205,8 +205,8 @@ class Entrypoint {
           .join(', ');
       if (overriddenPackages.isNotEmpty) {
         log.message(log.yellow(
-            'Overriding Dart SDK constraint from <2.0.0 to <2.0.0-dev.infinity'
-            ' for the following packages:\n\n${overriddenPackages}\n\n'
+            'Overriding the upper bound Dart SDK constraint to <=${sdk.version} '
+            'for the following packages:\n\n${overriddenPackages}\n\n'
             'To disable this you can set the PUB_ALLOW_PRERELEASE_SDK system '
             'environment variable to `false`, or you can silence this message '
             'by setting it to `quiet`.'));
@@ -677,7 +677,7 @@ class Entrypoint {
       // It's very unlikely that the lockfile is invalid here, but it's not
       // impossible—for example, the user may have a very old application
       // package with a checked-in lockfile that's newer than the pubspec, but
-      // that contains sdk dependencies.
+      // that contains SDK dependencies.
       if (lockFileId.source is UnknownSource) return false;
 
       var packagesFileUri = packages[lockFileId.name];
diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart
@@ -16,6 +16,7 @@ import 'feature.dart';
 import 'io.dart';
 import 'log.dart';
 import 'package_name.dart';
+import 'sdk.dart' as sdk;
 import 'source_registry.dart';
 import 'utils.dart';
 
@@ -33,9 +34,6 @@ final _packageName = new RegExp(
 final VersionRange _defaultUpperBoundSdkConstraint =
     new VersionConstraint.parse("<2.0.0");
 
-/// The upper bound contraint that matches the dev SDK.
-final _preReleaseTwoDotZeroSdkVersion = new Version.parse("2.0.0-dev.infinity");
-
 /// Whether or not to allow the pre-release SDK for packages that have an
 /// upper bound Dart SDK constraint of <2.0.0.
 ///
@@ -44,7 +42,7 @@ final _preReleaseTwoDotZeroSdkVersion = new Version.parse("2.0.0-dev.infinity");
 ///
 /// This has a default value of `true` but can be overridden with the
 /// PUB_ALLOW_PRERELEASE_SDK system environment variable.
-bool get allowPreReleaseTwoDotZeroSdk => allowPreReleaseSdkValue != 'false';
+bool get allowPreReleaseSdk => allowPreReleaseSdkValue != 'false';
 
 /// The value of the PUB_ALLOW_PRERELEASE_SDK environment variable, defaulted
 /// to `true`.
@@ -63,8 +61,7 @@ Using a default value of `true`.
 }();
 
 /// Whether or not to warn about pre-release SDK overrides.
-bool get warnAboutPreReleaseTwoDotZeroSdkOverrides =>
-    allowPreReleaseSdkValue != 'quiet';
+bool get warnAboutPreReleaseSdkOverrides => allowPreReleaseSdkValue != 'quiet';
 
 /// Whether or not `features` are enabled.
 ///
@@ -376,24 +373,45 @@ class Pubspec {
     var pair = _parseEnvironment(fields);
     var parsedDartSdkConstraint = pair.first;
 
-    if (allowPreReleaseTwoDotZeroSdk &&
-        parsedDartSdkConstraint is VersionRange &&
-        parsedDartSdkConstraint.max == _defaultUpperBoundSdkConstraint.max &&
-        !parsedDartSdkConstraint.includeMax) {
+    if (parsedDartSdkConstraint is VersionRange &&
+        _shouldEnableCurrentSdk(parsedDartSdkConstraint)) {
       _originalDartSdkConstraint = parsedDartSdkConstraint;
       _dartSdkWasOverridden = true;
       _dartSdkConstraint = new VersionRange(
           min: parsedDartSdkConstraint.min,
           includeMin: parsedDartSdkConstraint.includeMin,
-          max: _preReleaseTwoDotZeroSdkVersion,
-          includeMax: false);
+          max: sdk.version,
+          includeMax: true);
     } else {
       _dartSdkConstraint = parsedDartSdkConstraint;
     }
 
     _flutterSdkConstraint = pair.last;
   }
 
+  /// Whether or not we should override [sdkConstraint] to be <= the user's
+  /// current SDK version.
+  ///
+  /// This is true if the following conditions are met:
+  ///
+  ///   - [allowPreReleaseSdk] is `true`
+  ///   - The user's current SDK is a pre-release version.
+  ///   - The original [sdkConstraint] max version is exclusive (`includeMax`
+  ///     is `false`).
+  ///   - The original [sdkConstraint] is not a pre-release version.
+  ///   - The original [sdkConstraint] matches the exact same major, minor, and
+  ///     patch versions as the user's current SDK.
+  bool _shouldEnableCurrentSdk(VersionRange sdkConstraint) {
+    if (!allowPreReleaseSdk) return false;
+    if (!sdk.version.isPreRelease) return false;
+    if (sdkConstraint.includeMax) return false;
+    if (sdkConstraint.max == null) return false;
+    if (sdkConstraint.max.isPreRelease) return false;
+    return sdkConstraint.max.major == sdk.version.major &&
+        sdkConstraint.max.minor == sdk.version.minor &&
+        sdkConstraint.max.patch == sdk.version.patch;
+  }
+
   /// Parses the "environment" field in [parent] and returns the Dart and
   /// Flutter SDK constraints, respectively.
   Pair<VersionConstraint, VersionConstraint> _parseEnvironment(YamlMap parent) {
diff --git a/test/pubspec_test.dart b/test/pubspec_test.dart
@@ -5,6 +5,7 @@
 import 'package:pub/src/compiler.dart';
 import 'package:pub/src/package_name.dart';
 import 'package:pub/src/pubspec.dart';
+import 'package:pub/src/sdk.dart' as sdk;
 import 'package:pub/src/source.dart';
 import 'package:pub/src/source_registry.dart';
 import 'package:pub/src/system_cache.dart';
@@ -443,33 +444,46 @@ dependencies:
     });
 
     group("environment", () {
+      /// Checking for the default SDK constraint based on the current SDK.
+      void expectDefaultSdkConstraint(Pubspec pubspec) {
+        var sdkVersionString = sdk.version.toString();
+        if (sdkVersionString.startsWith('2.0.0') && sdk.version.isPreRelease) {
+          expect(
+              pubspec.dartSdkConstraint,
+              new VersionConstraint.parse(
+                  '${pubspec.dartSdkConstraint} <=$sdkVersionString'));
+        } else {
+          expect(
+              pubspec.dartSdkConstraint,
+              new VersionConstraint.parse(
+                  "${pubspec.dartSdkConstraint} <2.0.0"));
+        }
+      }
+
       test("allows an omitted environment", () {
         var pubspec = new Pubspec.parse('name: testing', sources);
-        expect(pubspec.dartSdkConstraint,
-            equals(new VersionConstraint.parse("<2.0.0-dev.infinity")));
+        expectDefaultSdkConstraint(pubspec);
         expect(pubspec.flutterSdkConstraint, isNull);
       });
 
-      test("default sdk constraint can be ommited with empty environment", () {
-        var pubspec =
-            new Pubspec.parse('', sources, includeDefaultSdkConstraint: false);
-        expect(pubspec.dartSdkConstraint, equals(VersionConstraint.any));
+      test("default SDK constraint can be omitted with empty environment", () {
+        var pubspec = new Pubspec.parse('', sources);
+        expectDefaultSdkConstraint(pubspec);
         expect(pubspec.flutterSdkConstraint, isNull);
       });
 
-      test("defaults the upper constraint for the sdk", () {
+      test("defaults the upper constraint for the SDK", () {
         var pubspec = new Pubspec.parse('''
   name: test
   environment:
     sdk: ">1.0.0"
   ''', sources);
-        expect(pubspec.dartSdkConstraint,
-            equals(new VersionConstraint.parse(">1.0.0 <2.0.0-dev.infinity")));
+        expectDefaultSdkConstraint(pubspec);
         expect(pubspec.flutterSdkConstraint, isNull);
       });
 
       test(
-          "default upper constraint for the sdk applies only if compatibile "
+          "default upper constraint for the SDK applies only if compatibile "
           "with the lower bound", () {
         var pubspec = new Pubspec.parse('''
   environment:
diff --git a/test/version_solver_test.dart b/test/version_solver_test.dart
@@ -1068,7 +1068,7 @@ void dartSdkConstraint() {
   });
 
   test(
-      "pub doesn't log about pre-release sdk overrides if "
+      "pub doesn't log about pre-release SDK overrides if "
       "PUB_ALLOW_PRERELEASE_SDK=quiet", () async {
     await d.dir('foo', [
       await d.pubspec({'name': 'foo'})
@@ -1134,9 +1134,116 @@ void dartSdkConstraint() {
 
     await expectResolves(
         environment: {'_PUB_TEST_SDK_VERSION': '2.0.0'},
-        error: 'Package foo requires SDK version <2.0.0-dev.infinity but the '
+        error: 'Package foo requires SDK version <2.0.0 but the '
             'current SDK is 2.0.0.');
   });
+
+  test("pre-release override requires major SDK versions to match", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<2.2.3'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3-dev.1.0'},
+        output: isNot(contains('PUB_ALLOW_PRERELEASE_SDK')));
+  });
+
+  test("pre-release override requires minor SDK versions to match", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<1.3.3'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3-dev.1.0'},
+        output: isNot(contains('PUB_ALLOW_PRERELEASE_SDK')));
+  });
+
+  test("pre-release override requires patch SDK versions to match", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<1.2.4'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3-dev.1.0'},
+        output: isNot(contains('PUB_ALLOW_PRERELEASE_SDK')));
+  });
+
+  test("pre-release override requires exclusive max", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<=1.2.3'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3-dev.1.0'},
+        output: isNot(contains('PUB_ALLOW_PRERELEASE_SDK')));
+  });
+
+  test("pre-release override requires pre-release SDK", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<1.2.3'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3'},
+        error: 'Package myapp requires SDK version <1.2.3 but the current '
+            'SDK is 1.2.3.');
+  });
+
+  test("pre-release override requires no existing pre-release constraints",
+      () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<1.2.3-dev.2.0'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3-dev.1.0'},
+        output: isNot(contains('PUB_ALLOW_PRERELEASE_SDK')));
+  });
+
+  test("pre-release override requires no build release constraints", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<1.2.3+1'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3'},
+        output: isNot(contains('PUB_ALLOW_PRERELEASE_SDK')));
+  });
+
+  test("pre-release override works generally", () async {
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'environment': {'sdk': '<1.2.3'}
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '1.2.3-dev.1.0'},
+        output: allOf(contains('PUB_ALLOW_PRERELEASE_SDK'),
+            contains('<=1.2.3-dev.1.0'), contains('myapp')));
+  });
 }
 
 void flutterSdkConstraint() {
