Skip to content

Commit

Permalink
chore(aft): Better handling of pre-release Dart SDK usage
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Dillon Nys committed Aug 16, 2023
1 parent 790f58e commit 57f7e09
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 67 deletions.
9 changes: 9 additions & 0 deletions packages/aft/lib/src/commands/bootstrap_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ 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) => pkg.compatibleWithCurrentSdk,
)
.expand((pkg) => [pkg, pkg.example, pkg.docs])
.nonNulls;
for (final package in bootstrapPackages) {
Expand Down
17 changes: 14 additions & 3 deletions packages/aft/lib/src/config/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -330,9 +327,23 @@ class PackageInfo
flavor == PackageFlavor.flutter;
}

/// Whether this package is compatible with the current Dart version
/// running `aft`.
bool get compatibleWithCurrentSdk {
final dartSdkConstraint = pubspecInfo.pubspec.environment!['sdk']!;
return dartSdkConstraint.allows(
Version.parse(Platform.version.split(_whitespace).first),
);
}

static final _whitespace = RegExp(r'\s+');

@override
List<Object?> get props => [name];

@override
String get runtimeTypeName => 'PackageInfo';

@override
int compareTo(PackageInfo other) {
return path.compareTo(other.path);
Expand Down
16 changes: 13 additions & 3 deletions packages/aft/lib/src/constraints_checker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -111,7 +112,12 @@ 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) {
// Ignore constraints where a pre-release version of Dart is specified
// and the package is not publishable.
VersionRange(:final min?) when min.isPreRelease => !package.isPublishable,
_ => globalSdkConstraint == localSdkConstraint,
};
if (!satisfiesSdkConstraint) {
_processConstraint(
package: package,
Expand Down Expand Up @@ -140,7 +146,11 @@ final class GlobalConstraintChecker extends ConstraintsChecker {
}
}

return satisfiesSdkConstraint && satisfiesFlutterConstraint;
return switch (action) {
ConstraintsAction.check =>
satisfiesSdkConstraint && satisfiesFlutterConstraint,
_ => true,
};
}

/// Runs [action] for each dependency in [package].
Expand Down
67 changes: 67 additions & 0 deletions packages/aft/test/common.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
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]);
}
148 changes: 87 additions & 61 deletions packages/aft/test/constraints_checker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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]);
}
27 changes: 27 additions & 0 deletions packages/aft/test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import 'dart:io';

import 'package:aft/src/models.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:test/test.dart';

import 'common.dart';

void main() {
group('AmplifyVersion', () {
const proagation = VersionPropagation.minor;
Expand Down Expand Up @@ -110,4 +114,27 @@ void main() {
expect(proagation.propagateToComponent(version, breaking), true);
});
});

group('PackageInfo', () {
test('compatibleWithCurrentSdk', () {
final currentDartVersion = Version.parse(
Platform.version.split(RegExp(r'\s+')).first,
);
final stablePackage = dummyPackage('stable_pkg');
final previewPackage = dummyPackage(
'preview_pkg',
sdkConstraint: VersionConstraint.compatibleWith(
Version(3, 2, 0, pre: '0'),
),
);
expect(
stablePackage.key.compatibleWithCurrentSdk,
isTrue,
);
expect(
previewPackage.key.compatibleWithCurrentSdk,
currentDartVersion.isPreRelease,
);
});
});
}

0 comments on commit 57f7e09

Please sign in to comment.