Skip to content

Commit 7fc07cf

Browse files
dnys1Dillon Nys
authored andcommitted
feat(aft): Validate local constraints in aft constraints and aft publish (#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.
1 parent 00a6039 commit 7fc07cf

File tree

6 files changed

+623
-129
lines changed

6 files changed

+623
-129
lines changed

packages/aft/lib/src/commands/constraints_command.dart

Lines changed: 33 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ import 'dart:convert';
55
import 'dart:io';
66

77
import 'package:aft/aft.dart';
8+
import 'package:aft/src/constraints_checker.dart';
89
import 'package:aft/src/options/glob_options.dart';
910
import 'package:collection/collection.dart';
1011
import 'package:pub_api_client/pub_api_client.dart';
1112
import 'package:pub_semver/pub_semver.dart';
12-
import 'package:pubspec_parse/pubspec_parse.dart';
1313

14-
enum _ConstraintsAction {
14+
enum ConstraintsAction {
1515
check(
1616
'Checks whether all constraints in the repo match the global config',
1717
'All constraints matched!',
@@ -26,7 +26,7 @@ enum _ConstraintsAction {
2626
'Constraints successfully applied!',
2727
);
2828

29-
const _ConstraintsAction(this.description, this.successMessage);
29+
const ConstraintsAction(this.description, this.successMessage);
3030

3131
final String description;
3232
final String successMessage;
@@ -35,8 +35,8 @@ enum _ConstraintsAction {
3535
/// Command to manage dependencies across all Dart/Flutter packages in the repo.
3636
class ConstraintsCommand extends AmplifyCommand {
3737
ConstraintsCommand() {
38-
addSubcommand(_ConstraintsSubcommand(_ConstraintsAction.check));
39-
addSubcommand(_ConstraintsSubcommand(_ConstraintsAction.apply));
38+
addSubcommand(_ConstraintsSubcommand(ConstraintsAction.check));
39+
addSubcommand(_ConstraintsSubcommand(ConstraintsAction.apply));
4040
addSubcommand(_ConstraintsUpdateCommand());
4141
addSubcommand(_ConstraintsPubVerifyCommand());
4242
}
@@ -52,129 +52,29 @@ class ConstraintsCommand extends AmplifyCommand {
5252
class _ConstraintsSubcommand extends AmplifyCommand with GlobOptions {
5353
_ConstraintsSubcommand(this.action);
5454

55-
final _ConstraintsAction action;
55+
final ConstraintsAction action;
5656

5757
@override
5858
String get description => action.description;
5959

6060
@override
6161
String get name => action.name;
6262

63-
final _mismatchedDependencies = <String>[];
64-
65-
/// Checks the [local] constraint against the [global] and returns whether
66-
/// an update is required.
67-
void _checkConstraint(
68-
PackageInfo package,
69-
List<String> dependencyPath,
70-
VersionConstraint global,
71-
VersionConstraint local,
72-
) {
73-
// Packages are not allowed to diverge from `aft.yaml`, even to specify
74-
// more precise constraints.
75-
final satisfiesGlobalConstraint = global == local;
76-
if (satisfiesGlobalConstraint) {
77-
return;
78-
}
79-
switch (action) {
80-
case _ConstraintsAction.check:
81-
final dependencyName = dependencyPath.last;
82-
_mismatchedDependencies.add(
83-
'${package.path}\n'
84-
'Mismatched `$dependencyName`:\n'
85-
'Expected $global\n'
86-
'Found $local\n',
87-
);
88-
return;
89-
case _ConstraintsAction.apply:
90-
case _ConstraintsAction.update:
91-
package.pubspecInfo.pubspecYamlEditor.update(
92-
dependencyPath,
93-
global.toString(),
94-
);
95-
}
96-
}
97-
98-
/// Checks the package's environment constraints against the global config.
99-
void _checkEnvironment(
100-
PackageInfo package,
101-
Map<String, VersionConstraint?> environment,
102-
Environment globalEnvironment,
103-
) {
104-
// Check Dart SDK contraint
105-
final globalSdkConstraint = globalEnvironment.sdk;
106-
final localSdkConstraint = environment['sdk'] ?? VersionConstraint.any;
107-
_checkConstraint(
108-
package,
109-
['environment', 'sdk'],
110-
globalSdkConstraint,
111-
localSdkConstraint,
112-
);
113-
114-
// Check Flutter SDK constraint
115-
if (package.flavor == PackageFlavor.flutter) {
116-
final globalFlutterConstraint = globalEnvironment.flutter;
117-
final localFlutterConstraint =
118-
environment['flutter'] ?? VersionConstraint.any;
119-
_checkConstraint(
120-
package,
121-
['environment', 'flutter'],
122-
globalFlutterConstraint,
123-
localFlutterConstraint,
124-
);
125-
}
126-
}
127-
128-
/// Checks the package's dependency constraints against the global config.
129-
void _checkDependency(
130-
PackageInfo package,
131-
Map<String, Dependency> dependencies,
132-
DependencyType dependencyType,
133-
MapEntry<String, VersionConstraint> globalDep,
134-
) {
135-
final dependencyName = globalDep.key;
136-
final globalDepConstraint = globalDep.value;
137-
final localDep = dependencies[dependencyName];
138-
if (localDep is! HostedDependency) {
139-
return;
140-
}
141-
final localDepConstraint = localDep.version;
142-
_checkConstraint(
143-
package,
144-
[dependencyType.key, dependencyName],
145-
globalDepConstraint,
146-
localDepConstraint,
147-
);
148-
}
149-
150-
Future<void> _run(_ConstraintsAction action) async {
151-
final globalDependencyConfig = aftConfig.dependencies;
152-
final globalEnvironmentConfig = aftConfig.environment;
63+
Future<void> _run(ConstraintsAction action) async {
64+
final constraintsCheckers = [
65+
GlobalConstraintChecker(
66+
action,
67+
repo.aftConfig.dependencies.asMap(),
68+
repo.aftConfig.environment,
69+
),
70+
PublishConstraintsChecker(
71+
action,
72+
repo.getPackageGraph(includeDevDependencies: true),
73+
),
74+
];
15375
for (final package in commandPackages.values) {
154-
_checkEnvironment(
155-
package,
156-
package.pubspecInfo.pubspec.environment ?? const {},
157-
globalEnvironmentConfig,
158-
);
159-
for (final globalDep in globalDependencyConfig.entries) {
160-
_checkDependency(
161-
package,
162-
package.pubspecInfo.pubspec.dependencies,
163-
DependencyType.dependency,
164-
globalDep,
165-
);
166-
_checkDependency(
167-
package,
168-
package.pubspecInfo.pubspec.dependencyOverrides,
169-
DependencyType.dependencyOverride,
170-
globalDep,
171-
);
172-
_checkDependency(
173-
package,
174-
package.pubspecInfo.pubspec.devDependencies,
175-
DependencyType.devDependency,
176-
globalDep,
177-
);
76+
for (final constraintsChecker in constraintsCheckers) {
77+
constraintsChecker.checkConstraints(package);
17878
}
17979

18080
if (package.pubspecInfo.pubspecYamlEditor.edits.isNotEmpty) {
@@ -183,9 +83,17 @@ class _ConstraintsSubcommand extends AmplifyCommand with GlobOptions {
18383
);
18484
}
18585
}
186-
if (_mismatchedDependencies.isNotEmpty) {
187-
for (final mismatched in _mismatchedDependencies) {
188-
logger.error(mismatched);
86+
final mismatchedDependencies = constraintsCheckers.expand(
87+
(checker) => checker.mismatchedDependencies,
88+
);
89+
if (mismatchedDependencies.isNotEmpty) {
90+
for (final mismatched in mismatchedDependencies) {
91+
final (:package, :dependencyName, :message) = mismatched;
92+
logger.error(
93+
'${package.path}\n'
94+
'Mismatched `$dependencyName`:\n'
95+
'$message\n',
96+
);
18997
}
19098
exit(1);
19199
}
@@ -200,7 +108,7 @@ class _ConstraintsSubcommand extends AmplifyCommand with GlobOptions {
200108
}
201109

202110
class _ConstraintsUpdateCommand extends _ConstraintsSubcommand {
203-
_ConstraintsUpdateCommand() : super(_ConstraintsAction.update);
111+
_ConstraintsUpdateCommand() : super(ConstraintsAction.update);
204112

205113
@override
206114
Future<void> run() async {
@@ -336,7 +244,7 @@ class _ConstraintsUpdateCommand extends _ConstraintsSubcommand {
336244
}
337245

338246
if (hasUpdates) {
339-
await _run(_ConstraintsAction.apply);
247+
await _run(ConstraintsAction.apply);
340248
}
341249
}
342250
}

packages/aft/lib/src/commands/publish_command.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import 'dart:convert';
55
import 'dart:io';
66

77
import 'package:aft/aft.dart';
8+
import 'package:aft/src/constraints_checker.dart';
89
import 'package:aft/src/options/glob_options.dart';
910
import 'package:aws_common/aws_common.dart';
1011
import 'package:collection/collection.dart';
@@ -27,6 +28,26 @@ mixin PublishHelpers on AmplifyCommand {
2728
.whereType<PackageInfo>()
2829
.toList();
2930

31+
final constraintsChecker = PublishConstraintsChecker(
32+
dryRun ? ConstraintsAction.update : ConstraintsAction.check,
33+
repo.getPackageGraph(includeDevDependencies: true),
34+
);
35+
for (final package in unpublishedPackages) {
36+
constraintsChecker.checkConstraints(package);
37+
}
38+
final mismatchedDependencies = constraintsChecker.mismatchedDependencies;
39+
if (mismatchedDependencies.isNotEmpty) {
40+
for (final mismatched in mismatchedDependencies) {
41+
final (:package, :dependencyName, :message) = mismatched;
42+
logger.error(
43+
'${package.path}\n'
44+
'Mismatched `$dependencyName`:\n'
45+
'$message\n',
46+
);
47+
}
48+
exit(1);
49+
}
50+
3051
try {
3152
sortPackagesTopologically<PackageInfo>(
3253
unpublishedPackages,

packages/aft/lib/src/config/config.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,21 @@ class PackageInfo
291291
return found;
292292
}
293293

294+
/// The type of dependency [package] is in `this`, or `null` if [package]
295+
/// is not listed in this package's pubspec.
296+
DependencyType? dependencyType(PackageInfo package) {
297+
if (pubspecInfo.pubspec.dependencies.containsKey(package.name)) {
298+
return DependencyType.dependency;
299+
}
300+
if (pubspecInfo.pubspec.devDependencies.containsKey(package.name)) {
301+
return DependencyType.devDependency;
302+
}
303+
if (pubspecInfo.pubspec.dependencyOverrides.containsKey(package.name)) {
304+
return DependencyType.dependencyOverride;
305+
}
306+
return null;
307+
}
308+
294309
/// The parsed `CHANGELOG.md`.
295310
Changelog get changelog {
296311
final changelogMd = File(p.join(path, 'CHANGELOG.md')).readAsStringSync();

0 commit comments

Comments
 (0)