From 048f8367567e38a005784010f1f38c3c9d8a4f56 Mon Sep 17 00:00:00 2001 From: Dillon Nys Date: Sat, 27 Aug 2022 15:03:56 -0700 Subject: [PATCH] chore(aft): Clean up commit-id:de10a06e --- aft.yaml | 7 +- packages/aft/lib/src/changelog/changelog.dart | 4 +- .../aft/lib/src/changelog/commit_message.dart | 8 +- packages/aft/lib/src/changelog/parser.dart | 4 +- packages/aft/lib/src/changelog/printer.dart | 1 + .../aft/lib/src/commands/amplify_command.dart | 56 ++++- .../lib/src/commands/changelog_command.dart | 33 ++- .../aft/lib/src/commands/pub_command.dart | 1 - .../aft/lib/src/commands/publish_command.dart | 31 +-- .../aft/lib/src/commands/version_command.dart | 171 ++------------ packages/aft/lib/src/models.dart | 20 +- packages/aft/lib/src/models.g.dart | 22 +- .../aft/lib/src/options/git_ref_options.dart | 5 - .../aft/lib/src/options/glob_options.dart | 40 ++-- packages/aft/lib/src/repo.dart | 220 ++++++++++++++---- packages/aft/lib/src/util.dart | 26 +++ packages/aft/test/e2e_test.dart | 36 ++- ...blish_command_test.dart => util_test.dart} | 2 +- 18 files changed, 362 insertions(+), 325 deletions(-) rename packages/aft/test/{publish_command_test.dart => util_test.dart} (98%) diff --git a/aft.yaml b/aft.yaml index 67de8cd0d2f..d219385fa30 100644 --- a/aft.yaml +++ b/aft.yaml @@ -16,12 +16,7 @@ dependencies: ignore: - synthetic_package -# Branch names which map to pub.dev stable and prerelease tracks. -branches: - stable: stable - prerelease: next - -# Strongly connected components which should have minor/major version bumps happen +# Strongly connected components which should have major version bumps happen # in unison, i.e. a version bump to one package cascades to all. components: amplify: diff --git a/packages/aft/lib/src/changelog/changelog.dart b/packages/aft/lib/src/changelog/changelog.dart index a19cf60062d..00b728d5e0c 100644 --- a/packages/aft/lib/src/changelog/changelog.dart +++ b/packages/aft/lib/src/changelog/changelog.dart @@ -16,9 +16,9 @@ import 'dart:convert'; import 'package:aft/src/changelog/commit_message.dart'; import 'package:aft/src/changelog/printer.dart'; +import 'package:aws_common/aws_common.dart'; import 'package:built_collection/built_collection.dart'; import 'package:built_value/built_value.dart'; -import 'package:cli_util/cli_logging.dart'; import 'package:collection/collection.dart'; import 'package:markdown/markdown.dart'; import 'package:pub_semver/pub_semver.dart'; @@ -46,7 +46,7 @@ abstract class Changelog implements Built { /// changelog. factory Changelog.parse( String changelogMd, { - Logger? logger, + AWSLogger? logger, }) { final parser = Document(); final lines = LineSplitter.split(changelogMd).toList(); diff --git a/packages/aft/lib/src/changelog/commit_message.dart b/packages/aft/lib/src/changelog/commit_message.dart index e9bb4b1afee..1df5b3a36f0 100644 --- a/packages/aft/lib/src/changelog/commit_message.dart +++ b/packages/aft/lib/src/changelog/commit_message.dart @@ -28,9 +28,9 @@ enum CommitTypeGroup { features('Features'), other('Other Changes'); - final String header; - const CommitTypeGroup(this.header); + + final String header; } enum CommitType { @@ -49,11 +49,11 @@ enum CommitType { style.other(), test.other(); - final CommitTypeGroup group; - const CommitType.fixes() : group = CommitTypeGroup.fixes; const CommitType.features() : group = CommitTypeGroup.features; const CommitType.other() : group = CommitTypeGroup.other; + + final CommitTypeGroup group; } /// {@template aft.changelog.commit_message} diff --git a/packages/aft/lib/src/changelog/parser.dart b/packages/aft/lib/src/changelog/parser.dart index 245a8465a6d..77bae1b93cb 100644 --- a/packages/aft/lib/src/changelog/parser.dart +++ b/packages/aft/lib/src/changelog/parser.dart @@ -20,7 +20,7 @@ final RegExp semverRegex = RegExp(r'\d+\.\d+\.\d+[\d\w\.\+\-]*'); class _ChangelogParser implements NodeVisitor { _ChangelogParser(this.logger); - final Logger? logger; + final AWSLogger? logger; final builder = ChangelogBuilder(); @@ -40,7 +40,7 @@ class _ChangelogParser implements NodeVisitor { } final versionMatch = semverRegex.firstMatch(versionText)?.group(0); if (versionMatch == null) { - logger?.trace('Could not parse version: $versionText'); + logger?.debug('Could not parse version: $versionText'); break; } _currentVersion = Version.parse(versionMatch); diff --git a/packages/aft/lib/src/changelog/printer.dart b/packages/aft/lib/src/changelog/printer.dart index 672f35f4653..f5d65c04265 100644 --- a/packages/aft/lib/src/changelog/printer.dart +++ b/packages/aft/lib/src/changelog/printer.dart @@ -15,6 +15,7 @@ import 'package:aft/src/changelog/changelog.dart'; import 'package:markdown/markdown.dart'; +/// Renders the [markdown] AST as a string. String render(Iterable markdown) { final renderer = _MarkdownRenderer(); for (final node in markdown) { diff --git a/packages/aft/lib/src/commands/amplify_command.dart b/packages/aft/lib/src/commands/amplify_command.dart index e394abb61d2..1b626bb46d2 100644 --- a/packages/aft/lib/src/commands/amplify_command.dart +++ b/packages/aft/lib/src/commands/amplify_command.dart @@ -12,12 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'dart:collection'; import 'dart:io'; import 'package:aft/aft.dart'; import 'package:aft/src/repo.dart'; import 'package:args/command_runner.dart'; import 'package:aws_common/aws_common.dart'; +import 'package:checked_yaml/checked_yaml.dart'; +import 'package:git/git.dart' as git; import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; @@ -105,10 +108,57 @@ abstract class AmplifyCommand extends Command ); }(); - late final Repo repo = Repo(rootDir, logger: logger); + /// All packages in the Amplify Flutter repo. + late final Map allPackages = () { + final allDirs = rootDir + .listSync(recursive: true, followLinks: false) + .whereType(); + final allPackages = []; + for (final dir in allDirs) { + final pubspecInfo = dir.pubspec; + if (pubspecInfo == null) { + continue; + } + final pubspec = pubspecInfo.pubspec; + if (aftConfig.ignore.contains(pubspec.name)) { + continue; + } + allPackages.add( + PackageInfo( + name: pubspec.name, + path: dir.path, + usesMonoRepo: dir.usesMonoRepo, + pubspecInfo: pubspecInfo, + flavor: pubspec.flavor, + ), + ); + } + return UnmodifiableMapView({ + for (final package in allPackages..sort()) package.name: package, + }); + }(); + + /// The global `aft` configuration for the repo. + late final AftConfig aftConfig = () { + final configFile = File(p.join(rootDir.path, 'aft.yaml')); + assert(configFile.existsSync(), 'Could not find aft.yaml'); + final configYaml = configFile.readAsStringSync(); + return checkedYamlDecode(configYaml, AftConfig.fromJson); + }(); - Map get allPackages => repo.allPackages; - AftConfig get aftConfig => repo.aftConfig; + late final Repo repo = Repo( + rootDir, + allPackages: allPackages, + aftConfig: aftConfig, + logger: logger, + ); + + /// Runs `git` with the given [args] from the repo's root directory. + Future runGit(List args) => git.runGit( + args, + processWorkingDir: rootDir.path, + throwOnError: true, + ); /// A command runner for `pub`. PubCommandRunner createPubRunner() => PubCommandRunner( diff --git a/packages/aft/lib/src/commands/changelog_command.dart b/packages/aft/lib/src/commands/changelog_command.dart index 0687663106d..c0e7046aa53 100644 --- a/packages/aft/lib/src/commands/changelog_command.dart +++ b/packages/aft/lib/src/commands/changelog_command.dart @@ -38,14 +38,29 @@ class ChangelogCommand extends AmplifyCommand { abstract class _ChangelogBaseCommand extends AmplifyCommand with GitRefOptions, GlobOptions { + _ChangelogBaseCommand() { + argParser.addFlag( + 'yes', + abbr: 'y', + help: 'Responds "yes" to all prompts', + defaultsTo: false, + negatable: false, + ); + } + + late final bool yes = argResults!['yes'] as bool; + Future _updateChangelogs({required bool preview}) async { for (final package in repo.publishablePackages) { - final baseRef = this.baseRef ?? - repo.latestTag(package.name) ?? - repo.latestTag('amplify_flutter')!; + final baseRef = this.baseRef ?? repo.latestTag(package.name); + if (baseRef == null) { + exitError( + 'No tag exists for package (${package.name}). ' + 'Supply a base ref manually using --base-ref', + ); + } final changes = repo.changes(baseRef, headRef); - final commits = - changes.commitsByPackage[package.name]?.toSet() ?? const {}; + final commits = changes.commitsByPackage[package]?.toSet() ?? const {}; final changelogUpdate = package.changelog.update(commits: commits); if (preview) { if (changelogUpdate.hasUpdate) { @@ -83,6 +98,12 @@ class _ChangelogUpdateCommand extends _ChangelogBaseCommand { @override Future run() async { - return _updateChangelogs(preview: false); + await _updateChangelogs(preview: false); + + logger.info('Changelogs successfully updated'); + if (yes || prompt('Commit changes? (y/N) ').toLowerCase() == 'y') { + await runGit(['add', '.']); + await runGit(['commit', '-m', 'chore(version): Update changelogs']); + } } } diff --git a/packages/aft/lib/src/commands/pub_command.dart b/packages/aft/lib/src/commands/pub_command.dart index 4ae623d8ad0..7f0fb19cc44 100644 --- a/packages/aft/lib/src/commands/pub_command.dart +++ b/packages/aft/lib/src/commands/pub_command.dart @@ -18,7 +18,6 @@ import 'dart:io'; import 'package:aft/aft.dart'; import 'package:async/async.dart'; import 'package:aws_common/aws_common.dart'; -import 'package:cli_util/cli_logging.dart'; import 'package:http/http.dart' as http; import 'package:pub/src/http.dart' as pub_http; diff --git a/packages/aft/lib/src/commands/publish_command.dart b/packages/aft/lib/src/commands/publish_command.dart index 51ea8ce0da2..79b3a01e53d 100644 --- a/packages/aft/lib/src/commands/publish_command.dart +++ b/packages/aft/lib/src/commands/publish_command.dart @@ -17,9 +17,7 @@ import 'dart:io'; import 'package:aft/aft.dart'; import 'package:aft/src/util.dart'; import 'package:aws_common/aws_common.dart'; -import 'package:cli_util/cli_logging.dart'; import 'package:graphs/graphs.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; /// Command to publish all Dart/Flutter packages in the repo. class PublishCommand extends AmplifyCommand { @@ -146,7 +144,7 @@ class PublishCommand extends AmplifyCommand { @override Future run() async { // Gather packages which can be published. - var publishablePackages = (await Future.wait([ + final publishablePackages = (await Future.wait([ for (final package in allPackages.values) _checkPublishable(package), ])) .whereType() @@ -165,7 +163,7 @@ class PublishCommand extends AmplifyCommand { } try { - publishablePackages = sortPackagesTopologically( + sortPackagesTopologically( publishablePackages, (pkg) => pkg.pubspecInfo.pubspec, ); @@ -254,28 +252,3 @@ Future runBuildRunner( exit(1); } } - -/// Sorts packages in topological order so they may be published in the order -/// they're sorted. -/// -/// Packages with inter-dependencies cannot be topologically sorted and will -/// throw a [CycleException]. -List sortPackagesTopologically( - Iterable packages, - Pubspec Function(T) getPubspec, -) { - final pubspecs = packages.map(getPubspec); - final packageNames = pubspecs.map((el) => el.name).toList(); - final graph = >{ - for (var package in pubspecs) - package.name: package.dependencies.keys.where(packageNames.contains), - }; - final ordered = topologicalSort(graph.keys, (key) => graph[key]!); - return packages.toList() - ..sort((a, b) { - // `ordered` is in reverse ordering to our desired publish precedence. - return ordered - .indexOf(getPubspec(b).name) - .compareTo(ordered.indexOf(getPubspec(a).name)); - }); -} diff --git a/packages/aft/lib/src/commands/version_command.dart b/packages/aft/lib/src/commands/version_command.dart index 1f7ebb99dd1..9efe4c1ac1e 100644 --- a/packages/aft/lib/src/commands/version_command.dart +++ b/packages/aft/lib/src/commands/version_command.dart @@ -15,16 +15,10 @@ import 'dart:io'; import 'package:aft/aft.dart'; -import 'package:aft/src/changelog/changelog.dart'; -import 'package:aft/src/changelog/commit_message.dart'; import 'package:aft/src/options/git_ref_options.dart'; import 'package:aft/src/options/glob_options.dart'; import 'package:aft/src/repo.dart'; -import 'package:aft/src/util.dart'; -import 'package:aws_common/aws_common.dart'; -import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; -import 'package:pub_semver/pub_semver.dart'; /// Command for bumping package versions across the repo. class VersionCommand extends AmplifyCommand { @@ -43,13 +37,18 @@ class VersionCommand extends AmplifyCommand { abstract class _VersionBaseCommand extends AmplifyCommand with GitRefOptions, GlobOptions { - @override - Map get allPackages { - return Map.fromEntries( - super.allPackages.entries.where((entry) => !entry.value.isExample), + _VersionBaseCommand() { + argParser.addFlag( + 'yes', + abbr: 'y', + help: 'Responds "yes" to all prompts', + defaultsTo: false, + negatable: false, ); } + late final bool yes = argResults!['yes'] as bool; + GitChanges _changesForPackage(PackageInfo package) { final baseRef = this.baseRef ?? repo.latestTag(package.name); if (baseRef == null) { @@ -62,13 +61,12 @@ abstract class _VersionBaseCommand extends AmplifyCommand } Future _updateVersions({required bool preview}) async { - final updates = await bumpVersions( - repo: repo, + repo.bumpAllVersions( changesForPackage: _changesForPackage, ); - final changelogUpdates = updates.updatedChangelogs; + final changelogUpdates = repo.changelogUpdates; - for (final package in allPackages.values) { + for (final package in repo.publishablePackages) { final edits = package.pubspecInfo.pubspecYamlEditor.edits; if (edits.isNotEmpty) { if (preview) { @@ -120,140 +118,17 @@ class _VersionUpdateCommand extends _VersionBaseCommand { @override Future run() async { - return _updateVersions(preview: false); - } -} - -Future bumpVersions({ - required Repo repo, - required GitChanges Function(PackageInfo) changesForPackage, -}) async { - // Version updates, by component. - final versionUpdates = {}; - - // Changelog updates. by package. - final changelogUpdates = {}; - - /// Bumps the dependency for [package] in [dependent]. - void bumpDependency(PackageInfo package, PackageInfo dependent) { - final component = repo.aftConfig.componentForPackage(package.name); - final newVersion = versionUpdates[component] ?? package.version; - if (dependent.pubspecInfo.pubspec.dependencies.containsKey(package.name)) { - dependent.pubspecInfo.pubspecYamlEditor.update( - ['dependencies', package.name], - newVersion.amplifyConstraint(minVersion: newVersion), - ); + await _updateVersions(preview: false); + + logger.info('Version successfully bumped'); + if (yes || prompt('Commit changes? (y/N) ').toLowerCase() == 'y') { + // Commit and tag changes + await runGit(['add', '.']); + await runGit(['commit', '-m', 'chore(version): Bump version']); + await Future.wait([ + for (final changeEntry in repo.versionChanges.entries) + runGit(['tag', '${changeEntry.key}_v${changeEntry.value}']), + ]); } } - - /// Bumps the version and changelog in [package] using [commit]. - /// - /// Returns the new version. - Version bumpVersion( - PackageInfo package, { - bool propogate = false, - CommitMessage? commit, - }) { - final component = repo.aftConfig.componentForPackage(package.name); - final currentVersion = package.version; - final currentProposedVersion = versionUpdates[component] ?? currentVersion; - final isBreakingChange = commit?.isBreakingChange ?? false; - final newProposedVersion = currentVersion.nextAmplifyVersion( - isBreakingChange: isBreakingChange, - ); - final newVersion = versionUpdates[component] = maxBy( - [currentProposedVersion, newProposedVersion], - (version) => version, - )!; - - if (newVersion > currentVersion) { - repo.logger.debug( - 'Bumping ${package.name} from $currentProposedVersion to $newVersion: ' - '${commit?.summary}', - ); - package.pubspecInfo.pubspecYamlEditor.update( - ['version'], - newVersion.toString(), - ); - final currentChangelogUpdate = changelogUpdates[package]; - changelogUpdates[package] = package.changelog.update( - commits: { - ...?currentChangelogUpdate?.commits, - if (commit != null) commit, - }, - version: newVersion, - ); - - if (propogate) { - // Propogate to all dependent packages. - dfs( - repo.reversedPackageGraph, - root: package, - (dependent) { - if (!dependent.isExample) { - bumpVersion(dependent, commit: commit); - bumpDependency(package, dependent); - } - }, - ); - - // Propogate to all component packages. - repo.components[component]?.forEach((componentPackage) { - bumpVersion(componentPackage, commit: commit); - dfs( - repo.reversedPackageGraph, - root: componentPackage, - (dependent) { - bumpDependency(componentPackage, dependent); - }, - ); - }); - } - } - - return newVersion; - } - - final sortedPackages = repo.publishablePackages; - sortPackagesTopologically( - sortedPackages, - (PackageInfo pkg) => pkg.pubspecInfo.pubspec, - ); - for (final package in sortedPackages) { - final changes = changesForPackage(package); - final commits = changes.commitsByPackage[package]?.toSet() ?? const {}; - for (final commit in commits) { - // TODO(dnys1): Define full set of commit types which should be ignored - // when considering version changes. - if (commit.isVersionBump || - commit.type == CommitType.merge && commit.taggedPr == null) { - continue; - } - bumpVersion( - package, - commit: commit, - propogate: commit.isBreakingChange, - ); - // Propogate the version change to all packages affected by the same - // commit. - changes.packagesByCommit[commit]?.forEach((commitPackage) { - bumpDependency(package, commitPackage); - }); - } - } - - return VersionChanges( - updatedChangelogs: changelogUpdates, - updatedVersions: versionUpdates, - ); -} - -class VersionChanges { - const VersionChanges({ - required this.updatedChangelogs, - required this.updatedVersions, - }); - - final Map updatedVersions; - final Map updatedChangelogs; } diff --git a/packages/aft/lib/src/models.dart b/packages/aft/lib/src/models.dart index 455b19962ba..17a97bdabc1 100644 --- a/packages/aft/lib/src/models.dart +++ b/packages/aft/lib/src/models.dart @@ -297,8 +297,6 @@ const yamlSerializable = JsonSerializable( disallowUnrecognizedKeys: true, ); -enum PubTrack { stable, prerelease } - /// The typed representation of the `aft.yaml` file. @yamlSerializable @_VersionConstraintConverter() @@ -306,7 +304,6 @@ class AftConfig { const AftConfig({ this.dependencies = const {}, this.ignore = const [], - this.branches = const {PubTrack.stable: 'main'}, this.components = const {}, }); @@ -321,19 +318,18 @@ class AftConfig { /// Packages to ignore in all repo operations. final List ignore; - /// Branch names which map to pub.dev stable and prerelease tracks. - final Map branches; - /// Strongly connected components which should have minor/major version bumps /// happen in unison, i.e. a version bump to one package cascades to all. final Map> components; - /// Retrieves the component for [package], if any. - String componentForPackage(String package) { - return components.entries.firstWhereOrNull((component) { - return component.value.contains(package); - })?.key ?? - package; + /// Retrieves the component for [packageName], if any. + String componentForPackage(String packageName) { + return components.entries + .firstWhereOrNull( + (component) => component.value.contains(packageName), + ) + ?.key ?? + packageName; } Map toJson() => _$AftConfigToJson(this); diff --git a/packages/aft/lib/src/models.g.dart b/packages/aft/lib/src/models.g.dart index 3a2fc9be6e4..63c0755c6e2 100644 --- a/packages/aft/lib/src/models.g.dart +++ b/packages/aft/lib/src/models.g.dart @@ -12,12 +12,7 @@ AftConfig _$AftConfigFromJson(Map json) => $checkedCreate( ($checkedConvert) { $checkKeys( json, - allowedKeys: const [ - 'dependencies', - 'ignore', - 'branches', - 'components' - ], + allowedKeys: const ['dependencies', 'ignore', 'components'], ); final val = AftConfig( dependencies: $checkedConvert( @@ -35,14 +30,6 @@ AftConfig _$AftConfigFromJson(Map json) => $checkedCreate( (v) => (v as List?)?.map((e) => e as String).toList() ?? const []), - branches: $checkedConvert( - 'branches', - (v) => - (v as Map?)?.map( - (k, e) => MapEntry( - $enumDecode(_$PubTrackEnumMap, k), e as String), - ) ?? - const {PubTrack.stable: 'main'}), components: $checkedConvert( 'components', (v) => @@ -60,16 +47,9 @@ Map _$AftConfigToJson(AftConfig instance) => { 'dependencies': instance.dependencies.map( (k, e) => MapEntry(k, const _VersionConstraintConverter().toJson(e))), 'ignore': instance.ignore, - 'branches': - instance.branches.map((k, e) => MapEntry(_$PubTrackEnumMap[k]!, e)), 'components': instance.components, }; -const _$PubTrackEnumMap = { - PubTrack.stable: 'stable', - PubTrack.prerelease: 'prerelease', -}; - SdkConfig _$SdkConfigFromJson(Map json) => $checkedCreate( 'SdkConfig', json, diff --git a/packages/aft/lib/src/options/git_ref_options.dart b/packages/aft/lib/src/options/git_ref_options.dart index c7a8c3b6650..fd6cd9879db 100644 --- a/packages/aft/lib/src/options/git_ref_options.dart +++ b/packages/aft/lib/src/options/git_ref_options.dart @@ -12,14 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -import 'dart:convert'; import 'dart:io'; import 'package:aft/aft.dart'; -import 'package:aft/src/changelog/changelog.dart'; -import 'package:aft/src/changelog/commit_message.dart'; -import 'package:aft/src/changelog/printer.dart'; -import 'package:pub_semver/pub_semver.dart'; /// Adds git ref options and functionality to a command. mixin GitRefOptions on AmplifyCommand { diff --git a/packages/aft/lib/src/options/glob_options.dart b/packages/aft/lib/src/options/glob_options.dart index 09386ae5bb9..98168bd2b1a 100644 --- a/packages/aft/lib/src/options/glob_options.dart +++ b/packages/aft/lib/src/options/glob_options.dart @@ -13,7 +13,6 @@ // limitations under the License. import 'package:aft/aft.dart'; -import 'package:glob/glob.dart'; /// Adds globbing options to a command. mixin GlobOptions on AmplifyCommand { @@ -21,32 +20,41 @@ mixin GlobOptions on AmplifyCommand { void init() { super.init(); argParser - ..addMultiOption('include', help: 'Glob of packages to include') - ..addMultiOption('exclude', help: 'Glob of packages to exclude'); + ..addMultiOption( + 'include', + help: 'Package or component names to include', + ) + ..addMultiOption( + 'exclude', + help: 'Package or component names to exclude', + ); } - /// Globs of packages which should be included in versioning. - late final include = (argResults?['include'] as List? ?? const []) - .map(Glob.new) - .toList(); + /// List of packages or components which should be included in versioning. + late final include = argResults?['include'] as List? ?? const []; - /// Globs of packages which should be excluded from versioning. - late final exclude = (argResults?['exclude'] as List? ?? const []) - .map(Glob.new) - .toList(); + /// List of packages or components which should be excluded from versioning. + late final exclude = argResults?['exclude'] as List? ?? const []; @override Map get allPackages { return Map.fromEntries( super.allPackages.entries.where((entry) { final package = entry.value; - for (final glob in include) { - if (glob.matches(package.path)) { - return true; + if (include.isNotEmpty) { + for (final packageOrComponent in include) { + if (package.name == packageOrComponent || + aftConfig.componentForPackage(package.name) == + packageOrComponent) { + return true; + } } + return false; } - for (final glob in exclude) { - if (glob.matches(package.path)) { + for (final packageOrComponent in exclude) { + if (package.name == packageOrComponent || + aftConfig.componentForPackage(package.name) == + packageOrComponent) { return false; } } diff --git a/packages/aft/lib/src/repo.dart b/packages/aft/lib/src/repo.dart index acbb77524e8..27c91a9578f 100644 --- a/packages/aft/lib/src/repo.dart +++ b/packages/aft/lib/src/repo.dart @@ -12,11 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'dart:collection'; import 'dart:io'; import 'package:aft/aft.dart'; import 'package:aft/src/changelog/changelog.dart'; import 'package:aft/src/changelog/commit_message.dart'; +import 'package:aft/src/util.dart'; import 'package:aws_common/aws_common.dart'; import 'package:built_collection/built_collection.dart'; import 'package:checked_yaml/checked_yaml.dart'; @@ -28,55 +30,27 @@ import 'package:pub_semver/pub_semver.dart'; /// Encapsulates all repository functionality including package and Git /// management. class Repo { - Repo(this.rootDir, {AWSLogger? logger}) - : logger = logger ?? AWSLogger('Repo'); + Repo( + this.rootDir, { + required this.allPackages, + required this.aftConfig, + AWSLogger? logger, + }) : logger = logger ?? AWSLogger('Repo'); /// The root directory of the repository. final Directory rootDir; final AWSLogger logger; - /// All packages in the Amplify Flutter repo. - late final Map allPackages = () { - final allDirs = rootDir - .listSync(recursive: true, followLinks: false) - .whereType(); - final allPackages = []; - for (final dir in allDirs) { - final pubspecInfo = dir.pubspec; - if (pubspecInfo == null) { - continue; - } - final pubspec = pubspecInfo.pubspec; - if (aftConfig.ignore.contains(pubspec.name)) { - continue; - } - allPackages.add( - PackageInfo( - name: pubspec.name, - path: dir.path, - usesMonoRepo: dir.usesMonoRepo, - pubspecInfo: pubspecInfo, - flavor: pubspec.flavor, - ), - ); - } - return UnmodifiableMapView({ - for (final package in allPackages..sort()) package.name: package, - }); - }(); + final Map allPackages; - late final List publishablePackages = - allPackages.values.where((pkg) => !pkg.isExample).toList(); + final AftConfig aftConfig; - /// The global `aft` configuration for the repo. - late final AftConfig aftConfig = () { - final configFile = File(p.join(rootDir.path, 'aft.yaml')); - assert(configFile.existsSync(), 'Could not find aft.yaml'); - final configYaml = configFile.readAsStringSync(); - return checkedYamlDecode(configYaml, AftConfig.fromJson); - }(); + late final List publishablePackages = UnmodifiableListView( + allPackages.values.where((pkg) => !pkg.isExample).toList(), + ); + /// The components of the late final Map> components = aftConfig.components.map((component, packages) { return MapEntry( @@ -114,13 +88,14 @@ class Repo { ); /// The directed graph of packages to the packages it depends on. - late final Map> packageGraph = { + late final Map> packageGraph = + UnmodifiableMapView({ for (final package in allPackages.values) package: package.pubspecInfo.pubspec.dependencies.keys .map((packageName) => allPackages[packageName]) .whereType() .toList(), - }; + }); /// The reversed (transposed) [packageGraph]. /// @@ -136,15 +111,9 @@ class Repo { reversedPackageGraph[dep]!.add(entry.key); } } - return reversedPackageGraph; + return UnmodifiableMapView(reversedPackageGraph); }(); - /// The git diff between [baseRef] and [headRef]. - Diff diffRefs(String baseRef, String headRef) => diffTrees( - RevParse.single(repo: repo, spec: '$baseRef^{tree}') as Tree, - RevParse.single(repo: repo, spec: '$headRef^{tree}') as Tree, - ); - /// The git diff between [oldTree] and [newTree]. Diff diffTrees(Tree oldTree, Tree newTree) => Diff.treeToTree( repo: repo, @@ -157,6 +126,8 @@ class Repo { /// Collect all the packages which have changed between [baseRef]..[headRef] /// and the commits which changed them. GitChanges changes(String baseRef, String headRef) { + // TODO(dnys1): Diff with index if headRef is null to include uncommitted + // changes? final baseTree = RevParse.single( repo: repo, spec: '$baseRef^{tree}', @@ -229,6 +200,125 @@ class Repo { packagesByCommit: packagesByCommit.build(), ); } + + late final versionChanges = VersionChanges(this); + + /// Changelog updates. by package. + final Map changelogUpdates = {}; + + /// Bumps the version for all packages in the repo. + void bumpAllVersions({ + required GitChanges Function(PackageInfo) changesForPackage, + }) { + final sortedPackages = List.of(publishablePackages); + sortPackagesTopologically( + sortedPackages, + (PackageInfo pkg) => pkg.pubspecInfo.pubspec, + ); + for (final package in sortedPackages) { + final changes = changesForPackage(package); + changes.commitsByPackage[package]?.forEach((commit) { + // TODO(dnys1): Define full set of commit types which should be ignored + // when considering version changes. + if (commit.isVersionBump || + commit.type == CommitType.merge && commit.taggedPr == null) { + return; + } + bumpVersion( + package, + commit: commit, + propagate: commit.isBreakingChange, + ); + // Propagate the version change to all packages affected by the same + // commit as if they were a component. + changes.packagesByCommit[commit]?.forEach((commitPackage) { + bumpDependency(package, commitPackage); + }); + }); + } + } + + /// Bumps the version and changelog in [package] using [commit] and returns + /// the new version. + /// + /// If [propagate] is `true`, the version change is propagated to all packages + /// which depend on [package]. + Version bumpVersion( + PackageInfo package, { + bool propagate = false, + CommitMessage? commit, + }) { + final component = aftConfig.componentForPackage(package.name); + final currentVersion = package.version; + final currentProposedVersion = versionChanges.newVersion(package); + final isBreakingChange = commit?.isBreakingChange ?? false; + final newProposedVersion = currentVersion.nextAmplifyVersion( + isBreakingChange: isBreakingChange, + ); + final newVersion = maxBy( + [currentProposedVersion, newProposedVersion], + (version) => version, + )!; + versionChanges.updateVersion(package, newVersion); + + if (newVersion > currentVersion) { + logger.debug( + 'Bumping ${package.name} from $currentProposedVersion to $newVersion: ' + '${commit?.summary}', + ); + package.pubspecInfo.pubspecYamlEditor.update( + ['version'], + newVersion.toString(), + ); + final currentChangelogUpdate = changelogUpdates[package]; + changelogUpdates[package] = package.changelog.update( + commits: { + ...?currentChangelogUpdate?.commits, + if (commit != null) commit, + }, + version: newVersion, + ); + + if (propagate) { + // Propagate to all dependent packages. + dfs( + reversedPackageGraph, + root: package, + (dependent) { + if (!dependent.isExample) { + bumpVersion(dependent, commit: commit); + } + bumpDependency(package, dependent); + }, + ); + + // Propagate to all component packages. + components[component]?.forEach((componentPackage) { + bumpVersion(componentPackage, commit: commit); + dfs( + reversedPackageGraph, + root: componentPackage, + (dependent) { + bumpDependency(componentPackage, dependent); + }, + ); + }); + } + } + + return newVersion; + } + + /// Bumps the dependency for [package] in [dependent]. + void bumpDependency(PackageInfo package, PackageInfo dependent) { + final newVersion = versionChanges.newVersion(package); + if (dependent.pubspecInfo.pubspec.dependencies.containsKey(package.name)) { + dependent.pubspecInfo.pubspecYamlEditor.update( + ['dependencies', package.name], + newVersion.amplifyConstraint(minVersion: newVersion), + ); + } + } } class GitChanges { @@ -250,3 +340,35 @@ class _DiffMarker with AWSEquatable<_DiffMarker> { @override List get props => [baseTree, headTree]; } + +class VersionChanges extends MapBase + with UnmodifiableMapMixin { + VersionChanges(this._repo); + + final Repo _repo; + + /// Version updates, by component. + final Map _versionUpdates = {}; + + /// The latest proposed version for [package]. + Version newVersion(PackageInfo package) { + final component = _repo.aftConfig.componentForPackage(package.name); + return _versionUpdates[component] ?? package.version; + } + + /// Updates the proposed version for [package]. + void updateVersion(PackageInfo package, Version version) { + final currentVersion = newVersion(package); + if (version <= currentVersion) { + return; + } + final component = _repo.aftConfig.componentForPackage(package.name); + _versionUpdates[component] = version; + } + + @override + Version? operator [](Object? key) => _versionUpdates[key]; + + @override + Iterable get keys => _versionUpdates.keys; +} diff --git a/packages/aft/lib/src/util.dart b/packages/aft/lib/src/util.dart index 8151878cd92..efd65c7ba75 100644 --- a/packages/aft/lib/src/util.dart +++ b/packages/aft/lib/src/util.dart @@ -16,6 +16,8 @@ import 'dart:convert'; import 'dart:io'; import 'package:async/async.dart'; +import 'package:graphs/graphs.dart'; +import 'package:pubspec_parse/pubspec_parse.dart'; typedef ProcessSink = void Function(String); @@ -74,3 +76,27 @@ void dfs( graph.forEach(search); } } + +/// Sorts packages in topological order so they may be published in the order +/// they're sorted. +/// +/// Packages with inter-dependencies cannot be topologically sorted and will +/// throw a [CycleException]. +void sortPackagesTopologically( + List packages, + Pubspec Function(T) getPubspec, +) { + final pubspecs = packages.map(getPubspec); + final packageNames = pubspecs.map((el) => el.name).toList(); + final graph = >{ + for (var package in pubspecs) + package.name: package.dependencies.keys.where(packageNames.contains), + }; + final ordered = topologicalSort(graph.keys, (key) => graph[key]!); + packages.sort((a, b) { + // `ordered` is in reverse ordering to our desired publish precedence. + return ordered + .indexOf(getPubspec(b).name) + .compareTo(ordered.indexOf(getPubspec(a).name)); + }); +} diff --git a/packages/aft/test/e2e_test.dart b/packages/aft/test/e2e_test.dart index 343fb3d8b1f..f1d91fd8596 100644 --- a/packages/aft/test/e2e_test.dart +++ b/packages/aft/test/e2e_test.dart @@ -25,23 +25,15 @@ import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; class MockRepo extends Repo { - MockRepo(super.rootDir, {required this.repo, super.logger}); + MockRepo( + super.rootDir, { + required this.repo, + required super.aftConfig, + super.logger, + }) : super(allPackages: {}); @override final Repository repo; - - @override - final AftConfig aftConfig = const AftConfig( - components: { - 'amplify': [ - 'amplify_auth_cognito', - 'amplify_auth_cognito_ios', - ], - }, - ); - - @override - final Map allPackages = {}; } void main() { @@ -51,7 +43,6 @@ void main() { Future runGit(List args) => git.runGit( args, processWorkingDir: repo.rootDir.path, - throwOnError: true, ); PackageInfo createPackage( @@ -120,6 +111,14 @@ Initial version. repo = MockRepo( gitDir, repo: Repository.init(path: gitDir.path), + aftConfig: const AftConfig( + components: { + 'amplify': [ + 'amplify_auth_cognito', + 'amplify_auth_cognito_ios', + ], + }, + ), ); await runGit(['commit', '--allow-empty', '-m', 'Initial commit']); }); @@ -297,10 +296,8 @@ Initial version. 'amplify_auth_cognito_ios': '1.0.0-next.1', }; - late VersionChanges updates; setUp(() async { - updates = await bumpVersions( - repo: repo, + repo.bumpAllVersions( changesForPackage: changesForPackage, ); }); @@ -309,8 +306,7 @@ Initial version. final packageName = check.key; test(packageName, () { final package = repo.allPackages[packageName]!; - final component = repo.aftConfig.componentForPackage(package.name); - final newVersion = updates.updatedVersions[component]!; + final newVersion = repo.versionChanges.newVersion(package); expect(newVersion.toString(), finalVersions[packageName]); }); } diff --git a/packages/aft/test/publish_command_test.dart b/packages/aft/test/util_test.dart similarity index 98% rename from packages/aft/test/publish_command_test.dart rename to packages/aft/test/util_test.dart index bc808687f42..b180467c31f 100644 --- a/packages/aft/test/publish_command_test.dart +++ b/packages/aft/test/util_test.dart @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import 'package:aft/aft.dart'; +import 'package:aft/src/util.dart'; import 'package:graphs/graphs.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart';