chore(aft): Better handling of pre-release Dart SDK usage (#3569)

dnys1 · Dillon Nys · commit c383ab9c14c2 · 2023-08-28T16:09:59.000-07:00
Some packages in the repo (for example, the `actions` pkg) set a pre-release Dart SDK constraint so that preview features of Dart can be leveraged. This updates `aft` so that: 1) `bootstrap` skips packages which set this constraint if the current Dart SDK doesn't support it; and 2) Ignores preview constraints in the constraints checker, since these packages do not need to conform to the global setting.
diff --git a/packages/aft/lib/src/commands/amplify_command.dart b/packages/aft/lib/src/commands/amplify_command.dart
@@ -190,7 +190,9 @@ abstract class AmplifyCommand extends Command<void>
     if (globalResults?['verbose'] as bool? ?? false) {
       AWSLogger().logLevel = LogLevel.verbose;
     }
-    logger.verbose('Got configuration: $aftConfig');
+    logger
+      ..info('Found Dart SDK: $activeDartSdkVersion')
+      ..verbose('Got configuration: $aftConfig');
     repo = await Repo.open(aftConfig, logger: logger);
   }
 
diff --git a/packages/aft/lib/src/commands/bootstrap_command.dart b/packages/aft/lib/src/commands/bootstrap_command.dart
@@ -79,6 +79,24 @@ const amplifyEnvironments = <String, String>{};
           // command is significantly newer/older than the embedded one.
           (pkg) => pkg.name != 'aft',
         )
+        .where(
+          // Skip bootstrapping packages which set incompatible Dart SDK constraints,
+          // e.g. packages which are leveraging preview features.
+          //
+          // The problem of packages setting incorrect constraints, for example setting
+          // `^3.0.5` when the current repo constraint is `^3.0.0` and we're running
+          // `aft` with `3.0.1` is a different issue handled by the constraints commands.
+          (pkg) {
+            final compatibleWithActiveSdk = pkg.compatibleWithActiveSdk;
+            if (!compatibleWithActiveSdk) {
+              logger.info(
+                'Skipping package ${pkg.name} since it sets an incompatible Dart SDK constraint: '
+                '${pkg.dartSdkConstraint}',
+              );
+            }
+            return compatibleWithActiveSdk;
+          },
+        )
         .expand((pkg) => [pkg, pkg.example, pkg.docs])
         .nonNulls;
     for (final package in bootstrapPackages) {
diff --git a/packages/aft/lib/src/config/config.dart b/packages/aft/lib/src/config/config.dart
@@ -317,9 +317,6 @@ class PackageInfo
       pubspecInfo.pubspec.version ??
       (throw StateError('No version for package: $name'));
 
-  @override
-  String get runtimeTypeName => 'PackageInfo';
-
   /// Skip package checks for Flutter packages when running in CI without
   /// Flutter, which may happen when testing Dart-only packages or specific
   /// Dart versions.
@@ -330,9 +327,21 @@ class PackageInfo
         flavor == PackageFlavor.flutter;
   }
 
+  /// Whether this package is compatible with the active Dart SDK
+  /// in the current environment.
+  bool get compatibleWithActiveSdk =>
+      dartSdkConstraint.allows(activeDartSdkVersion);
+
+  /// The Dart SDK constraint set by the package.
+  VersionConstraint get dartSdkConstraint =>
+      pubspecInfo.pubspec.environment!['sdk']!;
+
   @override
   List<Object?> get props => [name];
 
+  @override
+  String get runtimeTypeName => 'PackageInfo';
+
   @override
   int compareTo(PackageInfo other) {
     return path.compareTo(other.path);
@@ -403,3 +412,30 @@ extension DirectoryX on Directory {
     }
   }
 }
+
+/// The version of the current environment's Dart SDK.
+///
+/// This parses the version from calling `dart --version`.
+final Version activeDartSdkVersion = () {
+  final ProcessResult(
+    :exitCode,
+    :stdout,
+    :stderr,
+  ) = Process.runSync('dart', ['--version']);
+  if (exitCode != 0) {
+    throw Exception(
+      'Error running `dart --version` ($exitCode): $stderr',
+    );
+  }
+  // Example output:
+  // Dart SDK version: 3.1.0 (stable) (Tue Aug 15 21:33:36 2023 +0000) on "macos_arm64"
+  final activeSdkVersionRaw = _versionRegex.firstMatch(stdout as String);
+  if (activeSdkVersionRaw == null) {
+    throw Exception(
+      'Running `dart --version` produced unexpected output: $stdout',
+    );
+  }
+  return Version.parse(activeSdkVersionRaw.group(0)!);
+}();
+
+final _versionRegex = RegExp(r'\d+\.\d+\.\d+(-[a-zA-Z\d]+)?');
diff --git a/packages/aft/lib/src/constraints_checker.dart b/packages/aft/lib/src/constraints_checker.dart
@@ -48,14 +48,15 @@ sealed class ConstraintsChecker {
             message: errorMessage,
           ),
         );
+        return false;
       case ConstraintsAction.apply:
       case ConstraintsAction.update:
         package.pubspecInfo.pubspecYamlEditor.update(
           dependencyPath,
           expectedConstraint.toString(),
         );
+        return true;
     }
-    return false;
   }
 }
 
@@ -111,7 +112,16 @@ final class GlobalConstraintChecker extends ConstraintsChecker {
     // Check Dart SDK contraint
     final globalSdkConstraint = globalEnvironment.sdk;
     final localSdkConstraint = environment['sdk'];
-    final satisfiesSdkConstraint = globalSdkConstraint == localSdkConstraint;
+    final satisfiesSdkConstraint = switch (localSdkConstraint) {
+      // In the special case where we've set a constraint like `^3.2.0-0`
+      // (i.e. a pre-release constraint), the package is allowed to differ
+      // from all other packages iff it's not a publishable package.
+      VersionRange(:final min?) when min.isPreRelease => !package.isPublishable,
+
+      // Otherwise, in all other cases, the package's listed SDK constraint
+      // must exactly match all other packages.
+      _ => globalSdkConstraint == localSdkConstraint,
+    };
     if (!satisfiesSdkConstraint) {
       _processConstraint(
         package: package,
@@ -140,7 +150,11 @@ final class GlobalConstraintChecker extends ConstraintsChecker {
       }
     }
 
-    return satisfiesSdkConstraint && satisfiesFlutterConstraint;
+    return switch (action) {
+      ConstraintsAction.check =>
+        satisfiesSdkConstraint && satisfiesFlutterConstraint,
+      _ => true,
+    };
   }
 
   /// Runs [action] for each dependency in [package].
diff --git a/packages/aft/test/common.dart b/packages/aft/test/common.dart
@@ -0,0 +1,70 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import 'package:aft/aft.dart';
+import 'package:pub_semver/pub_semver.dart';
+import 'package:pubspec_parse/pubspec_parse.dart';
+import 'package:yaml/yaml.dart';
+import 'package:yaml_edit/yaml_edit.dart';
+
+/// Creates a dummy package for testing repo operations.
+MapEntry<PackageInfo, List<PackageInfo>> dummyPackage(
+  String name, {
+  Version? version,
+  bool publishable = true,
+  VersionConstraint? sdkConstraint,
+  Map<PackageInfo, VersionConstraint> deps = const {},
+  Map<PackageInfo, VersionConstraint> devDeps = const {},
+}) {
+  final path = 'packages/$name';
+  sdkConstraint ??= VersionConstraint.compatibleWith(Version(3, 0, 0));
+
+  final pubspecEditor = YamlEditor('''
+name: $name
+
+environment:
+  sdk: $sdkConstraint
+
+dependencies: {}
+
+dev_dependencies: {}
+''');
+
+  if (version != null) {
+    pubspecEditor.update(['version'], version.toString());
+  }
+
+  void addConstraints(
+    Map<PackageInfo, VersionConstraint> constraints,
+    DependencyType type,
+  ) {
+    for (final MapEntry(key: dep, value: constraint) in constraints.entries) {
+      final path = <String>[type.key, dep.name];
+      pubspecEditor.update(path, constraint.toString());
+    }
+  }
+
+  addConstraints(deps, DependencyType.dependency);
+  addConstraints(devDeps, DependencyType.devDependency);
+
+  if (!publishable) {
+    pubspecEditor.update(['publish_to'], 'none');
+  }
+
+  final pubspecYaml = pubspecEditor.toString();
+  final pubspec = Pubspec.parse(pubspecYaml);
+  final pubspecMap = loadYamlNode(pubspecYaml) as YamlMap;
+
+  final package = PackageInfo(
+    name: name,
+    path: path,
+    pubspecInfo: PubspecInfo(
+      pubspec: pubspec,
+      pubspecYaml: pubspecYaml,
+      pubspecMap: pubspecMap,
+      uri: Uri.base.resolve(path),
+    ),
+    flavor: PackageFlavor.dart,
+  );
+  return MapEntry(package, [...deps.keys, ...devDeps.keys]);
+}
diff --git a/packages/aft/test/constraints_checker_test.dart b/packages/aft/test/constraints_checker_test.dart
@@ -4,12 +4,97 @@
 import 'package:aft/aft.dart';
 import 'package:aft/src/constraints_checker.dart';
 import 'package:pub_semver/pub_semver.dart';
-import 'package:pubspec_parse/pubspec_parse.dart';
 import 'package:test/test.dart';
-import 'package:yaml/yaml.dart';
 import 'package:yaml_edit/yaml_edit.dart';
 
+import 'common.dart';
+
 void main() {
+  group('GlobalConstraintsChecker', () {
+    for (final action in ConstraintsAction.values) {
+      test(
+        'handles SDK constraints for preview Dart versions (${action.name})',
+        () {
+          final preReleaseConstraint = VersionConstraint.compatibleWith(
+            Version(3, 2, 0, pre: '0'),
+          );
+          final actions = dummyPackage(
+            'actions',
+            publishable: false,
+            sdkConstraint: preReleaseConstraint,
+          );
+          final amplifyFlutter = dummyPackage(
+            'amplify_flutter',
+            sdkConstraint: preReleaseConstraint,
+          );
+          final checker = GlobalConstraintChecker(
+            action,
+            const {},
+            Environment(
+              sdk: VersionConstraint.compatibleWith(Version(3, 0, 0)),
+            ),
+          );
+
+          {
+            expect(
+              checker.checkConstraints(actions.key),
+              isTrue,
+              reason:
+                  'Package is not publishable and can take a prerelease constraint '
+                  'to leverage new Dart features',
+            );
+            expect(checker.mismatchedDependencies, isEmpty);
+            expect(actions.key.pubspecInfo.pubspecYamlEditor.edits, isEmpty);
+          }
+
+          {
+            switch (action) {
+              case ConstraintsAction.apply || ConstraintsAction.update:
+                expect(
+                  checker.checkConstraints(amplifyFlutter.key),
+                  isTrue,
+                );
+                expect(
+                  amplifyFlutter.key.pubspecInfo.pubspecYamlEditor.edits.single,
+                  isA<SourceEdit>().having(
+                    (edit) => edit.replacement.trim(),
+                    'replacement',
+                    '^3.0.0',
+                  ),
+                );
+                expect(checker.mismatchedDependencies, isEmpty);
+              case ConstraintsAction.check:
+                expect(
+                  checker.checkConstraints(amplifyFlutter.key),
+                  isFalse,
+                  reason:
+                      'Package is publishable and must match the global SDK constraint',
+                );
+                expect(
+                  checker.mismatchedDependencies.single,
+                  isA<MismatchedDependency>()
+                      .having(
+                        (err) => err.package.name,
+                        'packageName',
+                        'amplify_flutter',
+                      )
+                      .having(
+                        (err) => err.dependencyName,
+                        'dependencyName',
+                        'sdk',
+                      ),
+                );
+                expect(
+                  amplifyFlutter.key.pubspecInfo.pubspecYamlEditor.edits,
+                  isEmpty,
+                );
+            }
+          }
+        },
+      );
+    }
+  });
+
   group('PublishConstraintsChecker', () {
     for (final action in ConstraintsAction.values) {
       final result = switch (action) {
@@ -119,62 +204,3 @@ void main() {
     }
   });
 }
-
-MapEntry<PackageInfo, List<PackageInfo>> dummyPackage(
-  String name, {
-  Version? version,
-  bool publishable = true,
-  Map<PackageInfo, VersionConstraint> deps = const {},
-  Map<PackageInfo, VersionConstraint> devDeps = const {},
-}) {
-  final path = 'packages/$name';
-
-  final pubspecEditor = YamlEditor('''
-name: $name
-
-environment:
-  sdk: ^3.0.0
-
-dependencies: {}
-
-dev_dependencies: {}
-''');
-
-  if (version != null) {
-    pubspecEditor.update(['version'], version.toString());
-  }
-
-  void addConstraints(
-    Map<PackageInfo, VersionConstraint> constraints,
-    DependencyType type,
-  ) {
-    for (final MapEntry(key: dep, value: constraint) in constraints.entries) {
-      final path = <String>[type.key, dep.name];
-      pubspecEditor.update(path, constraint.toString());
-    }
-  }
-
-  addConstraints(deps, DependencyType.dependency);
-  addConstraints(devDeps, DependencyType.devDependency);
-
-  if (!publishable) {
-    pubspecEditor.update(['publish_to'], 'none');
-  }
-
-  final pubspecYaml = pubspecEditor.toString();
-  final pubspec = Pubspec.parse(pubspecYaml);
-  final pubspecMap = loadYamlNode(pubspecYaml) as YamlMap;
-
-  final package = PackageInfo(
-    name: name,
-    path: path,
-    pubspecInfo: PubspecInfo(
-      pubspec: pubspec,
-      pubspecYaml: pubspecYaml,
-      pubspecMap: pubspecMap,
-      uri: Uri.base.resolve(path),
-    ),
-    flavor: PackageFlavor.dart,
-  );
-  return MapEntry(package, [...deps.keys, ...devDeps.keys]);
-}
diff --git a/packages/aft/test/model_test.dart b/packages/aft/test/model_test.dart

Original file line number	Diff line number	Diff line change
`@@ -190,7 +190,9 @@ abstract class AmplifyCommand extends Command<void>`
`190`	`190`	`if (globalResults?['verbose'] as bool? ?? false) {`
`191`	`191`	`AWSLogger().logLevel = LogLevel.verbose;`
`192`	`192`	`}`
`193`		`- logger.verbose('Got configuration: $aftConfig');`
	`193`	`+ logger`
	`194`	`+ ..info('Found Dart SDK: $activeDartSdkVersion')`
	`195`	`+ ..verbose('Got configuration: $aftConfig');`
`194`	`196`	`repo = await Repo.open(aftConfig, logger: logger);`
`195`	`197`	`}`
`196`	`198`