Reinterpret dart sdk constraints when read from a lockfile (#3897) * Reinterpret dart sdk constraints when read from a lockfile
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index eaa068c..93d6678 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart
@@ -427,9 +427,15 @@ dataError('${log.bold(name)} as globally activated requires ' 'unknown SDK "$name".'); } else if (sdkName == 'dart') { - if (constraint.allows((sdk as DartSdk).version)) return; - dataError("${log.bold(name)} as globally activated doesn't " - 'support Dart ${sdk.version}, try: $topLevelProgram pub global activate $name'); + if (constraint.effectiveConstraint.allows((sdk as DartSdk).version)) { + return; + } + dataError(''' +${log.bold(name)} as globally activated doesn't support Dart ${sdk.version}. + +try: +`$topLevelProgram pub global activate $name` to reactivate. +'''); } else { dataError('${log.bold(name)} as globally activated requires the ' '${sdk.name} SDK, which is unsupported for global executables.');
diff --git a/lib/src/lock_file.dart b/lib/src/lock_file.dart index bdafe4e..f725260 100644 --- a/lib/src/lock_file.dart +++ b/lib/src/lock_file.dart
@@ -15,6 +15,7 @@ import 'language_version.dart'; import 'package_config.dart'; import 'package_name.dart'; +import 'pubspec.dart'; import 'sdk.dart' show sdk; import 'system_cache.dart'; import 'utils.dart'; @@ -26,7 +27,7 @@ /// The intersections of all SDK constraints for all locked packages, indexed /// by SDK identifier. - Map<String, VersionConstraint> sdkConstraints; + Map<String, SdkConstraint> sdkConstraints; /// Dependency names that appeared in the root package's `dependencies` /// section. @@ -49,7 +50,7 @@ /// analysis server to provide better auto-completion. LockFile( Iterable<PackageId> ids, { - Map<String, VersionConstraint>? sdkConstraints, + Map<String, SdkConstraint>? sdkConstraints, Set<String>? mainDependencies, Set<String>? devDependencies, Set<String>? overriddenDependencies, @@ -58,7 +59,7 @@ ids.where((id) => !id.isRoot), key: (id) => id.name, ), - sdkConstraints ?? {'dart': VersionConstraint.any}, + sdkConstraints ?? {'dart': SdkConstraint(VersionConstraint.any)}, mainDependencies ?? const UnmodifiableSetView.empty(), devDependencies ?? const UnmodifiableSetView.empty(), overriddenDependencies ?? const UnmodifiableSetView.empty(), @@ -74,7 +75,7 @@ LockFile.empty() : packages = const {}, - sdkConstraints = {'dart': VersionConstraint.any}, + sdkConstraints = {'dart': SdkConstraint(VersionConstraint.any)}, mainDependencies = const UnmodifiableSetView.empty(), devDependencies = const UnmodifiableSetView.empty(), overriddenDependencies = const UnmodifiableSetView.empty(); @@ -114,14 +115,17 @@ 'YAML mapping', ); - final sdkConstraints = <String, VersionConstraint>{}; + final sdkConstraints = <String, SdkConstraint>{}; final sdkNode = _getEntry<YamlScalar?>(parsed, 'sdk', 'string', required: false); if (sdkNode != null) { // Lockfiles produced by pub versions from 1.14.0 through 1.18.0 included // a top-level "sdk" field which encoded the unified constraint on the // Dart SDK. They had no way of specifying constraints on other SDKs. - sdkConstraints['dart'] = _parseVersionConstraint(sdkNode); + sdkConstraints['dart'] = SdkConstraint.interpretDartSdkConstraint( + _parseVersionConstraint(sdkNode), + defaultUpperBoundConstraint: null, + ); } final sdksField = @@ -131,7 +135,19 @@ _parseEachEntry<String, YamlScalar>( sdksField, (name, constraint) { - sdkConstraints[name] = _parseVersionConstraint(constraint); + final originalConstraint = _parseVersionConstraint(constraint); + // Reinterpret the sdk constraints here, in case they were written by + // an old sdk that did not do reinterpretations. + // TODO(sigurdm): push the switching into `SdkConstraint`. + sdkConstraints[name] = switch (name) { + 'dart' => SdkConstraint.interpretDartSdkConstraint( + originalConstraint, + defaultUpperBoundConstraint: null, + ), + 'flutter' => + SdkConstraint.interpretFlutterSdkConstraint(originalConstraint), + _ => SdkConstraint(originalConstraint), + }; }, 'string', 'string', @@ -411,7 +427,7 @@ var data = { 'sdks': mapMap( sdkConstraints, - value: (_, constraint) => constraint.toString(), + value: (_, constraint) => constraint.effectiveConstraint.toString(), ), 'packages': packageMap };
diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart index 5f27abc..c27fdb7 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart
@@ -145,56 +145,6 @@ /// pubspec. final bool _includeDefaultSdkConstraint; - SdkConstraint _interpretDartSdkConstraint( - VersionConstraint originalConstraint, { - required VersionConstraint? defaultUpperBoundConstraint, - }) { - VersionConstraint constraint = originalConstraint; - if (defaultUpperBoundConstraint != null && - constraint is VersionRange && - constraint.max == null && - defaultUpperBoundConstraint.allowsAny(constraint)) { - constraint = VersionConstraint.intersection( - [constraint, defaultUpperBoundConstraint], - ); - } - // If a package is null safe it should also be compatible with dart 3. - // Therefore we rewrite a null-safety enabled constraint with the upper - // bound <3.0.0 to be have upper bound <4.0.0 - // - // Only do this rewrite after dart 3. - if (sdk.version.major >= 3 && - constraint is VersionRange && - LanguageVersion.fromSdkConstraint(constraint) >= - LanguageVersion.firstVersionWithNullSafety && - // <3.0.0 is parsed into a max of 3.0.0-0, so that is what we look for - // here. - constraint.max == Version(3, 0, 0).firstPreRelease && - constraint.includeMax == false) { - constraint = VersionRange( - min: constraint.min, - includeMin: constraint.includeMin, - // We don't have to use .firstPreRelease as the constructor will do that - // if needed. - max: Version(4, 0, 0), - ); - } - return SdkConstraint(constraint, originalConstraint: originalConstraint); - } - - // Flutter constraints get special treatment, as Flutter won't be using - // semantic versioning to mark breaking releases. We simply ignore upper - // bounds. - SdkConstraint _interpretFlutterSdkConstraint(VersionConstraint constraint) { - if (constraint is VersionRange) { - return SdkConstraint( - VersionRange(min: constraint.min, includeMin: constraint.includeMin), - originalConstraint: constraint, - ); - } - return SdkConstraint(constraint); - } - /// Parses the "environment" field in [parent] and returns a map from SDK /// identifiers to constraints on those SDKs. Map<String, SdkConstraint> _parseEnvironment(YamlMap parent) { @@ -215,7 +165,7 @@ ); } var constraints = { - 'dart': _interpretDartSdkConstraint( + 'dart': SdkConstraint.interpretDartSdkConstraint( originalDartSdkConstraint, defaultUpperBoundConstraint: _includeDefaultSdkConstraint ? _defaultUpperBoundSdkConstraint @@ -239,7 +189,7 @@ _FileType.pubspec, ); constraints[name] = name == 'flutter' - ? _interpretFlutterSdkConstraint(constraint) + ? SdkConstraint.interpretFlutterSdkConstraint(constraint) : SdkConstraint(constraint); }); } @@ -669,6 +619,62 @@ VersionConstraint? originalConstraint, }) : originalConstraint = originalConstraint ?? effectiveConstraint; + /// Implement support for down to 2.12 in the dart 3 series. Note that this + /// function has to be be idempotent, because we apply it both when we write + /// and read lock-file constraints, so applying this function a second time + /// should have no further effect. + factory SdkConstraint.interpretDartSdkConstraint( + VersionConstraint originalConstraint, { + required VersionConstraint? defaultUpperBoundConstraint, + }) { + VersionConstraint constraint = originalConstraint; + if (defaultUpperBoundConstraint != null && + constraint is VersionRange && + constraint.max == null && + defaultUpperBoundConstraint.allowsAny(constraint)) { + constraint = VersionConstraint.intersection( + [constraint, defaultUpperBoundConstraint], + ); + } + // If a package is null safe it should also be compatible with dart 3. + // Therefore we rewrite a null-safety enabled constraint with the upper + // bound <3.0.0 to be have upper bound <4.0.0 + // + // Only do this rewrite after dart 3. + if (sdk.version.major >= 3 && + constraint is VersionRange && + LanguageVersion.fromSdkConstraint(constraint) >= + LanguageVersion.firstVersionWithNullSafety && + // <3.0.0 is parsed into a max of 3.0.0-0, so that is what we look for + // here. + constraint.max == Version(3, 0, 0).firstPreRelease && + constraint.includeMax == false) { + constraint = VersionRange( + min: constraint.min, + includeMin: constraint.includeMin, + // We don't have to use .firstPreRelease as the constructor will do that + // if needed. + max: Version(4, 0, 0), + ); + } + return SdkConstraint(constraint, originalConstraint: originalConstraint); + } + + // Flutter constraints get special treatment, as Flutter won't be using + // semantic versioning to mark breaking releases. We simply ignore upper + // bounds. + factory SdkConstraint.interpretFlutterSdkConstraint( + VersionConstraint constraint, + ) { + if (constraint is VersionRange) { + return SdkConstraint( + VersionRange(min: constraint.min, includeMin: constraint.includeMin), + originalConstraint: constraint, + ); + } + return SdkConstraint(constraint); + } + /// The language version of a constraint is determined from how it is written. LanguageVersion get languageVersion => LanguageVersion.fromSdkConstraint(originalConstraint);
diff --git a/lib/src/solver/result.dart b/lib/src/solver/result.dart index ce841fb..f7a5d41 100644 --- a/lib/src/solver/result.dart +++ b/lib/src/solver/result.dart
@@ -97,7 +97,10 @@ } return LockFile( resolvedPackageIds, - sdkConstraints: sdkConstraints, + sdkConstraints: { + for (final MapEntry(:key, :value) in sdkConstraints.entries) + key: SdkConstraint(value), + }, mainDependencies: MapKeySet(_root.dependencies), devDependencies: MapKeySet(_root.devDependencies), overriddenDependencies: MapKeySet(_root.dependencyOverrides),
diff --git a/test/global/run/fails_if_sdk_constraint_is_unmet_test.dart b/test/global/run/fails_if_sdk_constraint_is_unmet_test.dart index deb31a7..ec2e8c6 100644 --- a/test/global/run/fails_if_sdk_constraint_is_unmet_test.dart +++ b/test/global/run/fails_if_sdk_constraint_is_unmet_test.dart
@@ -2,8 +2,12 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:io'; + +import 'package:path/path.dart' as p; import 'package:pub/src/exit_codes.dart' as exit_codes; import 'package:test/test.dart'; +import 'package:yaml_edit/yaml_edit.dart'; import '../../descriptor.dart' as d; import '../../test_pub.dart'; @@ -110,9 +114,47 @@ environment: {'_PUB_TEST_SDK_VERSION': '3.0.0'}, args: ['global', 'run', 'foo:script'], error: contains( - "foo as globally activated doesn't support Dart 3.0.0, try: dart pub global activate foo", + """ +foo as globally activated doesn't support Dart 3.0.0. + +try: +`dart pub global activate foo` to reactivate.""", ), exitCode: exit_codes.DATA, ); }); + + test('succeeds if SDK is upgraded from 2.19 to 3.0', () async { + final server = await servePackages(); + server.serve( + 'foo', + '1.0.0', + sdk: '^2.19.0', + contents: [ + d.dir( + 'bin', + [d.file('script.dart', "main(args) => print('123-OK');")], + ) + ], + ); + + await runPub( + environment: {'_PUB_TEST_SDK_VERSION': '2.19.0'}, + args: ['global', 'activate', 'foo'], + ); + + final lockFile = File( + p.join(d.sandbox, cachePath, 'global_packages', 'foo', 'pubspec.lock'), + ); + final editor = YamlEditor(lockFile.readAsStringSync()); + // This corresponds to what an older sdk would write, before the dart 3 hack + // was introduced: + editor.update(['sdks', 'dart'], '^2.19.0'); + + await runPub( + environment: {'_PUB_TEST_SDK_VERSION': '3.0.0'}, + args: ['global', 'run', 'foo:script'], + output: contains('123-OK'), + ); + }); }
diff --git a/test/lock_file_test.dart b/test/lock_file_test.dart index ce76003..3dbcf92 100644 --- a/test/lock_file_test.dart +++ b/test/lock_file_test.dart
@@ -94,8 +94,8 @@ test('allows an old-style SDK constraint', () { var lockFile = LockFile.parse('sdk: ">=1.2.3 <4.0.0"', sources); expect( - lockFile.sdkConstraints, - containsPair('dart', VersionConstraint.parse('>=1.2.3 <4.0.0')), + lockFile.sdkConstraints['dart']!.effectiveConstraint, + VersionConstraint.parse('>=1.2.3 <4.0.0'), ); expect(lockFile.sdkConstraints, isNot(contains('flutter'))); expect(lockFile.sdkConstraints, isNot(contains('fuchsia'))); @@ -112,16 +112,16 @@ sources, ); expect( - lockFile.sdkConstraints, - containsPair('dart', VersionConstraint.parse('>=1.2.3 <4.0.0')), + lockFile.sdkConstraints['dart']!.effectiveConstraint, + VersionConstraint.parse('>=1.2.3 <4.0.0'), ); expect( - lockFile.sdkConstraints, - containsPair('flutter', VersionConstraint.parse('^0.1.2')), + lockFile.sdkConstraints['flutter']!.effectiveConstraint, + VersionConstraint.parse('>=0.1.2'), ); expect( - lockFile.sdkConstraints, - containsPair('fuchsia', VersionConstraint.parse('^5.6.7')), + lockFile.sdkConstraints['fuchsia']!.effectiveConstraint, + VersionConstraint.parse('^5.6.7'), ); });