Skip to content

Commit

Permalink
fix(aft): Versioning algorithm and performance
Browse files Browse the repository at this point in the history 
commit-id:c45852ad
  • Loading branch information
Dillon Nys committed Aug 27, 2022
1 parent d7f7639 commit 55e00ba
Show file tree
Hide file tree
Showing 9 changed files with 267 additions and 127 deletions.
12 changes: 6 additions & 6 deletions packages/aft/lib/src/changelog/changelog.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ abstract class Changelog implements Built<Changelog, ChangelogBuilder> {
if (commits.isEmpty) {
// If there are no commits worth including, add a generic message about
// bug fixes/improvements.
nodes.add(Element.text('li', 'Minor bug fixes and improvements'));
nodes.add(Element.text('li', 'Minor bug fixes and improvements\n'));
} else {
for (final typedCommits in commitsByType.entries) {
nodes.add(Element.text('h3', typedCommits.key.header));
Expand All @@ -116,16 +116,13 @@ abstract class Changelog implements Built<Changelog, ChangelogBuilder> {
required Iterable<CommitMessage> commits,
Version? version,
}) {
if (commits.isEmpty) {
return ChangelogUpdate(originalText, commits: commits);
}
final nodes = makeVersionEntry(
commits: commits,
version: version,
);
// Replace the text in changelogMd so that the latest version matches
// `version`, if given, else `NEXT`.
final String keepText;
String keepText;
if (hasNextEntry || (version != null && latestVersion == version)) {
// Update latest entry, either to `version` or as a new `NEXT` entry.
keepText = LineSplitter.split(originalText)
Expand All @@ -138,6 +135,9 @@ abstract class Changelog implements Built<Changelog, ChangelogBuilder> {
// No `NEXT` or `version` entry exists yet.
keepText = originalText;
}
if (!keepText.endsWith('\n')) {
keepText = '$keepText\n';
}
return ChangelogUpdate(keepText, commits: commits, newText: render(nodes));
}

Expand All @@ -161,5 +161,5 @@ class ChangelogUpdate {
bool get hasUpdate => newText != null;

@override
String toString() => newText == null ? keepText : '$newText\n\n$keepText';
String toString() => newText == null ? keepText : '$newText$keepText';
}
10 changes: 9 additions & 1 deletion packages/aft/lib/src/changelog/printer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,15 @@ class _MarkdownRenderer implements NodeVisitor {
String get output => _builder.toString();

@override
void visitElementAfter(Element element) {}
void visitElementAfter(Element element) {
switch (element.type) {
case ElementType.ul:
_builder.writeln();
break;
default:
break;
}
}

@override
bool visitElementBefore(Element element) {
Expand Down
13 changes: 5 additions & 8 deletions packages/aft/lib/src/commands/changelog_command.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,12 @@ class ChangelogCommand extends AmplifyCommand {

abstract class _ChangelogBaseCommand extends AmplifyCommand
with GitRefOptions, GlobOptions {
@override
String get baseRef => super.baseRef ?? repo.latestTag('amplify_flutter')!;

late final changes = repo.changes(baseRef, headRef);

Future<void> _updateChangelogs({required bool preview}) async {
final publishablePackages =
allPackages.values.where((pkg) => !pkg.isExample);
for (final package in publishablePackages) {
for (final package in repo.publishablePackages) {
final baseRef = this.baseRef ??
repo.latestTag(package.name) ??
repo.latestTag('amplify_flutter')!;
final changes = repo.changes(baseRef, headRef);
final commits =
changes.commitsByPackage[package.name]?.toSet() ?? const {};
final changelogUpdate = package.changelog.update(commits: commits);
Expand Down
21 changes: 11 additions & 10 deletions packages/aft/lib/src/commands/publish_command.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class PublishCommand extends AmplifyCommand {
@override
Future<void> run() async {
// Gather packages which can be published.
final publishablePackages = (await Future.wait([
var publishablePackages = (await Future.wait([
for (final package in allPackages.values) _checkPublishable(package),
]))
.whereType<PackageInfo>()
Expand All @@ -165,7 +165,7 @@ class PublishCommand extends AmplifyCommand {
}

try {
sortPackagesTopologically<PackageInfo>(
publishablePackages = sortPackagesTopologically<PackageInfo>(
publishablePackages,
(pkg) => pkg.pubspecInfo.pubspec,
);
Expand Down Expand Up @@ -260,8 +260,8 @@ Future<void> runBuildRunner(
///
/// Packages with inter-dependencies cannot be topologically sorted and will
/// throw a [CycleException].
void sortPackagesTopologically<T>(
List<T> packages,
List<T> sortPackagesTopologically<T>(
Iterable<T> packages,
Pubspec Function(T) getPubspec,
) {
final pubspecs = packages.map(getPubspec);
Expand All @@ -271,10 +271,11 @@ void sortPackagesTopologically<T>(
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));
});
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));
});
}
142 changes: 87 additions & 55 deletions packages/aft/lib/src/commands/version_command.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ 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;
Expand Down Expand Up @@ -52,7 +54,7 @@ abstract class _VersionBaseCommand extends AmplifyCommand
final baseRef = this.baseRef ?? repo.latestTag(package.name);
if (baseRef == null) {
exitError(
'No tag exists for package. '
'No tag exists for package (${package.name}). '
'Supply a base ref manually using --base-ref',
);
}
Expand Down Expand Up @@ -126,87 +128,117 @@ Future<VersionChanges> bumpVersions({
required Repo repo,
required GitChanges Function(PackageInfo) changesForPackage,
}) async {
final logger = AWSLogger('updateVersions');

// Version updates, by component.
final versionUpdates = <String, Version>{};

// Changelog updates. by package.
final changelogUpdates = <PackageInfo, ChangelogUpdate>{};

Version? bumpVersion(PackageInfo package, {required bool isBreakingChange}) {
final component =
repo.aftConfig.componentForPackage(package) ?? package.name;
/// 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),
);
}
}

/// 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 > currentProposedVersion) {

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 ?? const Iterable.empty(),
commits: {
...?currentChangelogUpdate?.commits,
if (commit != null) commit,
},
version: newVersion,
);
return newVersion;
}
return null;
}

for (final package in repo.publishablePackages) {
final changes = changesForPackage(package);
final commits = changes.commitsByPackage[package.name]?.toSet() ?? const {};
if (propogate) {
// Propogate to all dependent packages.
dfs<PackageInfo>(
repo.reversedPackageGraph,
root: package,
(dependent) {
if (!dependent.isExample) {
bumpVersion(dependent, commit: commit);
bumpDependency(package, dependent);
}
},
);

var isBreakingChange = false;
final affectedPackages = <String>{};
for (final commit in commits) {
if (commit.isBreakingChange) {
isBreakingChange = true;
// Propogate to all component packages.
repo.components[component]?.forEach((componentPackage) {
bumpVersion(componentPackage, commit: commit);
dfs<PackageInfo>(
repo.reversedPackageGraph,
root: componentPackage,
(dependent) {
bumpDependency(componentPackage, dependent);
},
);
});
}
// Packages affected by the same commit.
affectedPackages.addAll(
changes.packagesByCommit[commit]?.where((pkg) => pkg != package.name) ??
const {},
);
}
final newVersion = bumpVersion(package, isBreakingChange: isBreakingChange);

final allAffectedPackages =
affectedPackages.map((packageName) => repo.allPackages[packageName]!);
for (final affectedPackage in allAffectedPackages) {
if (!affectedPackage.pubspecInfo.pubspec.dependencies
.containsKey(package.name)) {
continue;
}
affectedPackage.pubspecInfo.pubspecYamlEditor.update(
['dependencies', package.name],
'^$newVersion',
);
// Do a patch bump of the affected package and update its changelog.
bumpVersion(affectedPackage, isBreakingChange: false);
}
changelogUpdates[package] = package.changelog.update(
commits: commits,
version: newVersion,
);
return newVersion;
}

// Update pubspecs
for (final package in repo.publishablePackages) {
logger.info(package.name);
final newVersion = versionUpdates[package];
if (newVersion != null) {
package.pubspecInfo.pubspecYamlEditor.update(
['version'],
newVersion.toString(),
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,
);
logger
..info('Version')
..info('${package.version} --> $newVersion');
// Propogate the version change to all packages affected by the same
// commit.
changes.packagesByCommit[commit]?.forEach((commitPackage) {
bumpDependency(package, commitPackage);
});
}
}

Expand Down
48 changes: 35 additions & 13 deletions packages/aft/lib/src/models.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,22 +194,23 @@ class PubspecInfo {
}

extension AmplifyVersion on Version {
Version get nextPreRelease => Version(
major,
minor,
patch,
pre: preRelease.map((el) {
if (el is! int) return el;
return el + 1;
}).join('.'),
);

/// The next version according to Amplify rules for incrementing.
Version nextAmplifyVersion({bool isBreakingChange = false}) {
if (preRelease.isEmpty) {
return isBreakingChange ? nextMinor : nextPatch;
}
if (isBreakingChange) {
final newPrelease = preRelease.map((el) {
if (el is! int) return el;
return el + 1;
}).join('.');
return Version(
major,
minor,
patch,
pre: newPrelease,
);
return nextPreRelease;
}
final newBuild = (build.singleOrNull as int? ?? 0) + 1;
return Version(
Expand All @@ -220,6 +221,26 @@ extension AmplifyVersion on Version {
build: '$newBuild',
);
}

/// The constraint to use for this version in pubspecs.
String amplifyConstraint({Version? minVersion}) {
final Version maxVersion;
if (preRelease.isEmpty) {
final currentMinor = Version(major, minor, 0);
minVersion ??= currentMinor;
maxVersion = Version(major, minor + 1, 0);
} else {
final currentPreRelease = Version(
major,
minor,
patch,
pre: preRelease.join('.'),
);
minVersion ??= currentPreRelease;
maxVersion = nextPreRelease;
}
return '>=$minVersion <$maxVersion';
}
}

enum DependencyType {
Expand Down Expand Up @@ -308,10 +329,11 @@ class AftConfig {
final Map<String, List<String>> components;

/// Retrieves the component for [package], if any.
String? componentForPackage(PackageInfo package) {
String componentForPackage(String package) {
return components.entries.firstWhereOrNull((component) {
return component.value.contains(package.name);
})?.key;
return component.value.contains(package);
})?.key ??
package;
}

Map<String, Object?> toJson() => _$AftConfigToJson(this);
Expand Down
Loading

0 comments on commit 55e00ba

Please sign in to comment.