Skip to content

Commit

Permalink
feat(aft): Validate local constraints in aft constraints and `aft p…
Browse files Browse the repository at this point in the history
…ublish` (#3568)

There are bespoke, often confusing, rules to how inter-repo constraints must be handled such that `pub` allows packages to be published. See #2885, #2366, #2865.

This introduces a checker into both `aft constraints` and `aft publish` which validates that package constraints are configured to allow publishing and updates them (or reports an error) if not.
  • Loading branch information
dnys1 authored Aug 25, 2023
1 parent 0bf09b7 commit 239b84b
Show file tree
Hide file tree
Showing 6 changed files with 623 additions and 129 deletions.
158 changes: 33 additions & 125 deletions packages/aft/lib/src/commands/constraints_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import 'dart:convert';
import 'dart:io';

import 'package:aft/aft.dart';
import 'package:aft/src/constraints_checker.dart';
import 'package:aft/src/options/glob_options.dart';
import 'package:collection/collection.dart';
import 'package:pub_api_client/pub_api_client.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:pubspec_parse/pubspec_parse.dart';

enum _ConstraintsAction {
enum ConstraintsAction {
check(
'Checks whether all constraints in the repo match the global config',
'All constraints matched!',
Expand All @@ -26,7 +26,7 @@ enum _ConstraintsAction {
'Constraints successfully applied!',
);

const _ConstraintsAction(this.description, this.successMessage);
const ConstraintsAction(this.description, this.successMessage);

final String description;
final String successMessage;
Expand All @@ -35,8 +35,8 @@ enum _ConstraintsAction {
/// Command to manage dependencies across all Dart/Flutter packages in the repo.
class ConstraintsCommand extends AmplifyCommand {
ConstraintsCommand() {
addSubcommand(_ConstraintsSubcommand(_ConstraintsAction.check));
addSubcommand(_ConstraintsSubcommand(_ConstraintsAction.apply));
addSubcommand(_ConstraintsSubcommand(ConstraintsAction.check));
addSubcommand(_ConstraintsSubcommand(ConstraintsAction.apply));
addSubcommand(_ConstraintsUpdateCommand());
addSubcommand(_ConstraintsPubVerifyCommand());
}
Expand All @@ -52,129 +52,29 @@ class ConstraintsCommand extends AmplifyCommand {
class _ConstraintsSubcommand extends AmplifyCommand with GlobOptions {
_ConstraintsSubcommand(this.action);

final _ConstraintsAction action;
final ConstraintsAction action;

@override
String get description => action.description;

@override
String get name => action.name;

final _mismatchedDependencies = <String>[];

/// Checks the [local] constraint against the [global] and returns whether
/// an update is required.
void _checkConstraint(
PackageInfo package,
List<String> dependencyPath,
VersionConstraint global,
VersionConstraint local,
) {
// Packages are not allowed to diverge from `aft.yaml`, even to specify
// more precise constraints.
final satisfiesGlobalConstraint = global == local;
if (satisfiesGlobalConstraint) {
return;
}
switch (action) {
case _ConstraintsAction.check:
final dependencyName = dependencyPath.last;
_mismatchedDependencies.add(
'${package.path}\n'
'Mismatched `$dependencyName`:\n'
'Expected $global\n'
'Found $local\n',
);
return;
case _ConstraintsAction.apply:
case _ConstraintsAction.update:
package.pubspecInfo.pubspecYamlEditor.update(
dependencyPath,
global.toString(),
);
}
}

/// Checks the package's environment constraints against the global config.
void _checkEnvironment(
PackageInfo package,
Map<String, VersionConstraint?> environment,
Environment globalEnvironment,
) {
// Check Dart SDK contraint
final globalSdkConstraint = globalEnvironment.sdk;
final localSdkConstraint = environment['sdk'] ?? VersionConstraint.any;
_checkConstraint(
package,
['environment', 'sdk'],
globalSdkConstraint,
localSdkConstraint,
);

// Check Flutter SDK constraint
if (package.flavor == PackageFlavor.flutter) {
final globalFlutterConstraint = globalEnvironment.flutter;
final localFlutterConstraint =
environment['flutter'] ?? VersionConstraint.any;
_checkConstraint(
package,
['environment', 'flutter'],
globalFlutterConstraint,
localFlutterConstraint,
);
}
}

/// Checks the package's dependency constraints against the global config.
void _checkDependency(
PackageInfo package,
Map<String, Dependency> dependencies,
DependencyType dependencyType,
MapEntry<String, VersionConstraint> globalDep,
) {
final dependencyName = globalDep.key;
final globalDepConstraint = globalDep.value;
final localDep = dependencies[dependencyName];
if (localDep is! HostedDependency) {
return;
}
final localDepConstraint = localDep.version;
_checkConstraint(
package,
[dependencyType.key, dependencyName],
globalDepConstraint,
localDepConstraint,
);
}

Future<void> _run(_ConstraintsAction action) async {
final globalDependencyConfig = aftConfig.dependencies;
final globalEnvironmentConfig = aftConfig.environment;
Future<void> _run(ConstraintsAction action) async {
final constraintsCheckers = [
GlobalConstraintChecker(
action,
repo.aftConfig.dependencies.asMap(),
repo.aftConfig.environment,
),
PublishConstraintsChecker(
action,
repo.getPackageGraph(includeDevDependencies: true),
),
];
for (final package in commandPackages.values) {
_checkEnvironment(
package,
package.pubspecInfo.pubspec.environment ?? const {},
globalEnvironmentConfig,
);
for (final globalDep in globalDependencyConfig.entries) {
_checkDependency(
package,
package.pubspecInfo.pubspec.dependencies,
DependencyType.dependency,
globalDep,
);
_checkDependency(
package,
package.pubspecInfo.pubspec.dependencyOverrides,
DependencyType.dependencyOverride,
globalDep,
);
_checkDependency(
package,
package.pubspecInfo.pubspec.devDependencies,
DependencyType.devDependency,
globalDep,
);
for (final constraintsChecker in constraintsCheckers) {
constraintsChecker.checkConstraints(package);
}

if (package.pubspecInfo.pubspecYamlEditor.edits.isNotEmpty) {
Expand All @@ -183,9 +83,17 @@ class _ConstraintsSubcommand extends AmplifyCommand with GlobOptions {
);
}
}
if (_mismatchedDependencies.isNotEmpty) {
for (final mismatched in _mismatchedDependencies) {
logger.error(mismatched);
final mismatchedDependencies = constraintsCheckers.expand(
(checker) => checker.mismatchedDependencies,
);
if (mismatchedDependencies.isNotEmpty) {
for (final mismatched in mismatchedDependencies) {
final (:package, :dependencyName, :message) = mismatched;
logger.error(
'${package.path}\n'
'Mismatched `$dependencyName`:\n'
'$message\n',
);
}
exit(1);
}
Expand All @@ -200,7 +108,7 @@ class _ConstraintsSubcommand extends AmplifyCommand with GlobOptions {
}

class _ConstraintsUpdateCommand extends _ConstraintsSubcommand {
_ConstraintsUpdateCommand() : super(_ConstraintsAction.update);
_ConstraintsUpdateCommand() : super(ConstraintsAction.update);

@override
Future<void> run() async {
Expand Down Expand Up @@ -336,7 +244,7 @@ class _ConstraintsUpdateCommand extends _ConstraintsSubcommand {
}

if (hasUpdates) {
await _run(_ConstraintsAction.apply);
await _run(ConstraintsAction.apply);
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions packages/aft/lib/src/commands/publish_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'dart:convert';
import 'dart:io';

import 'package:aft/aft.dart';
import 'package:aft/src/constraints_checker.dart';
import 'package:aft/src/options/glob_options.dart';
import 'package:aws_common/aws_common.dart';
import 'package:collection/collection.dart';
Expand All @@ -27,6 +28,26 @@ mixin PublishHelpers on AmplifyCommand {
.whereType<PackageInfo>()
.toList();

final constraintsChecker = PublishConstraintsChecker(
dryRun ? ConstraintsAction.update : ConstraintsAction.check,
repo.getPackageGraph(includeDevDependencies: true),
);
for (final package in unpublishedPackages) {
constraintsChecker.checkConstraints(package);
}
final mismatchedDependencies = constraintsChecker.mismatchedDependencies;
if (mismatchedDependencies.isNotEmpty) {
for (final mismatched in mismatchedDependencies) {
final (:package, :dependencyName, :message) = mismatched;
logger.error(
'${package.path}\n'
'Mismatched `$dependencyName`:\n'
'$message\n',
);
}
exit(1);
}

try {
sortPackagesTopologically<PackageInfo>(
unpublishedPackages,
Expand Down
15 changes: 15 additions & 0 deletions packages/aft/lib/src/config/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,21 @@ class PackageInfo
return found;
}

/// The type of dependency [package] is in `this`, or `null` if [package]
/// is not listed in this package's pubspec.
DependencyType? dependencyType(PackageInfo package) {
if (pubspecInfo.pubspec.dependencies.containsKey(package.name)) {
return DependencyType.dependency;
}
if (pubspecInfo.pubspec.devDependencies.containsKey(package.name)) {
return DependencyType.devDependency;
}
if (pubspecInfo.pubspec.dependencyOverrides.containsKey(package.name)) {
return DependencyType.dependencyOverride;
}
return null;
}

/// The parsed `CHANGELOG.md`.
Changelog get changelog {
final changelogMd = File(p.join(path, 'CHANGELOG.md')).readAsStringSync();
Expand Down
Loading

0 comments on commit 239b84b

Please sign in to comment.