From 0d7d30bb07e0f3e7831a0165d41603bdd27c2b1a Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Fri, 22 Sep 2023 14:28:09 -0700 Subject: [PATCH] feat: add `cutler validate` command (#1309) Adds a new validate command to cutler, that when passed a flutter release version string validates that Shorebird branches are all set up correctly for that release version. --- packages/cutler/lib/checkout.dart | 91 +++++++++-- packages/cutler/lib/commands/commands.dart | 3 +- .../cutler/lib/commands/rebase_command.dart | 17 +- .../cutler/lib/commands/validate_command.dart | 151 ++++++++++++++++++ packages/cutler/lib/cutler.dart | 1 + packages/cutler/pubspec.lock | 8 + packages/cutler/pubspec.yaml | 1 + 7 files changed, 253 insertions(+), 19 deletions(-) create mode 100644 packages/cutler/lib/commands/validate_command.dart diff --git a/packages/cutler/lib/checkout.dart b/packages/cutler/lib/checkout.dart index 45016fd9..cf8373e9 100644 --- a/packages/cutler/lib/checkout.dart +++ b/packages/cutler/lib/checkout.dart @@ -8,7 +8,8 @@ import 'package:path/path.dart' as p; /// That lets the models be pure data objects, and keeps the command-running /// code separate. Unsure if this is a good design or not. -String runCommand( +/// Runs a command and returns the result. +ProcessResult runCommandInner( String executable, List arguments, { String? workingDirectory, @@ -23,7 +24,20 @@ String runCommand( : ' (in $workingDirectory)'; logger.detail("$executable ${arguments.join(' ')}$workingDirectoryString"); - final result = Process.runSync( + return Process.runSync( + executable, + arguments, + workingDirectory: workingDirectory, + ); +} + +/// Runs a command and returns stdout, trimmed. +String runCommand( + String executable, + List arguments, { + String? workingDirectory, +}) { + final result = runCommandInner( executable, arguments, workingDirectory: workingDirectory, @@ -110,6 +124,70 @@ class Checkout { ); } + /// Returns a [Version] for the given [branch] in the given [remote]. + Version remoteBranch({required String branch, required String remote}) { + final output = runCommand( + 'git', + ['ls-remote', '--refs', remote, branch], + workingDirectory: workingDirectory, + ); + final hash = output.split('\t').first; + final name = output.split('\t').last; + return Version( + hash: hash, + repo: repo, + aliases: [name], + ); + } + + /// Returns a [Version] for the given [tag] in the given [remote]. + Version remoteTag({ + required String remote, + required String tag, + }) { + final tags = remoteTags(remote: remote, pattern: tag); + if (tags.isEmpty) { + throw Exception('No tags found for $tag in $remote'); + } + if (tags.length > 1) { + throw Exception('Multiple tags found for $tag in $remote'); + } + return tags.first; + } + + /// Returns a list of Versions for the given [pattern] in the given [remote]. + Iterable remoteTags({ + required String remote, + String? pattern, + }) { + final args = ['ls-remote', '--tags', remote]; + if (pattern != null) { + args.add(pattern); + } + final output = runCommand('git', args, workingDirectory: workingDirectory); + // split lines + final lines = output.split('\n'); + return lines.map((line) { + final hash = line.split('\t').first; + final name = line.split('\t').last; + return Version( + hash: hash, + repo: repo, + aliases: [name], + ); + }); + } + + /// Returns true if [ancestor] is an ancestor of [descendant] in this repo. + bool isAncestor({required String ancestor, required String descendant}) { + final result = runCommandInner( + 'git', + ['merge-base', '--is-ancestor', ancestor, descendant], + workingDirectory: workingDirectory, + ); + return result.exitCode == 0; + } + /// Returns a count of commits between two commits in this repo. int countCommits({required String from, required String to}) { final output = runCommand( @@ -191,12 +269,3 @@ class Checkout { ); } } - -/// Extension methods for [Version] to do actual `git` actions. -// extension VersionCommands on Version { -// /// Returns the contents of a file at a given [path] in this repo at this -// /// version. -// String contentsAtPath(String path) { -// return Checkout(repo).contentsAtPath(hash, path); -// } -// } diff --git a/packages/cutler/lib/commands/commands.dart b/packages/cutler/lib/commands/commands.dart index 14e218da..f4019579 100644 --- a/packages/cutler/lib/commands/commands.dart +++ b/packages/cutler/lib/commands/commands.dart @@ -1,2 +1,3 @@ -export 'versions_command.dart'; export 'rebase_command.dart'; +export 'validate_command.dart'; +export 'versions_command.dart'; diff --git a/packages/cutler/lib/commands/rebase_command.dart b/packages/cutler/lib/commands/rebase_command.dart index 7c2e000b..9e36e932 100644 --- a/packages/cutler/lib/commands/rebase_command.dart +++ b/packages/cutler/lib/commands/rebase_command.dart @@ -36,6 +36,9 @@ class RebaseContext { // FIXME(eseidel): This should move onto Repo. final String devBranch; + /// Returns the fully qualified branch name for `shorebird/dev`. + String get fullyQualifiedBranch => 'origin/$devBranch'; + /// Print the commands needed to rebase `shorebird/dev` to a new revision. void printRebase(Repo repo) { final path = repo.path; @@ -51,7 +54,7 @@ class RebaseContext { ..info('# ${repo.name}') ..info('git -C $path fetch --all --tags') ..info('git -C $path rebase --onto $upstreamRef ' - '$forkpointRef $devBranch') + '$forkpointRef $fullyQualifiedBranch') ..info('# Handle conflicts'); } @@ -114,10 +117,11 @@ class RebaseContext { for (final repo in forks) { if (needsPush(repo)) { final path = repo.path; - // TODO(eseidel): This uses `shorebird/dev` not `origin/shorebird/dev` + final releaseBranch = 'flutter_release/$flutterVersionName'; logger - ..info('git -C $path push origin -f HEAD:shorebird/dev') + ..info('git -C $path push origin -f HEAD:$devBranch') ..info('git -C $path push --tags') + ..info('git -C $path push origin -f $devBranch:$releaseBranch') ..info(''); } } @@ -146,9 +150,8 @@ class RebaseCommand extends CutlerCommand { // Figure out our current versions, use `shorebird/dev` as our main branch // This isn't necessarily a self-consistent set of versions. - // TODO(eseidel): move devBranch onto Repo? - const devBranch = 'origin/shorebird/dev'; - final dev = getHeadVersions(checkouts, devBranch); + const devBranch = 'shorebird/dev'; + final dev = getHeadVersions(checkouts, 'origin/$devBranch'); printVersions(checkouts, dev); // Check if the VersionSet described by our Flutter fork actually matches @@ -159,7 +162,7 @@ class RebaseCommand extends CutlerCommand { final flutterVersions = getFlutterVersions(checkouts, dev.flutter.hash); if (flutterVersions != dev) { logger.warn('shorebirdtech/flutter:HEAD version set does not match ' - 'the latest `shorebird/dev` version set, this means this script ' + 'the latest `$devBranch` version set, this means this script ' 'will include new commits in its resulting VersionSet which were ' 'not previously included in the Flutter described by ' 'shorebirdtech/flutter:HEAD.'); diff --git a/packages/cutler/lib/commands/validate_command.dart b/packages/cutler/lib/commands/validate_command.dart new file mode 100644 index 00000000..205efd3e --- /dev/null +++ b/packages/cutler/lib/commands/validate_command.dart @@ -0,0 +1,151 @@ +import 'package:cutler/checkout.dart'; +import 'package:cutler/commands/base.dart'; +import 'package:cutler/logger.dart'; +import 'package:cutler/model.dart'; +import 'package:cutler/versions.dart'; +import 'package:io/io.dart'; +import 'package:version/version.dart' as semver; + +/// Check that a tag matches its upstream. +bool ensureTagMatch(Checkout checkout, String tag) { + final origin = checkout.remoteTag(tag: tag, remote: 'origin'); + final upstream = checkout.remoteTag(tag: tag, remote: 'upstream'); + final paddedName = checkout.name.padRight(10); + if (origin != upstream) { + logger.err('❌ $paddedName tag $origin does not match upstream $upstream'); + return false; + } + logger.info('✅ $paddedName ${origin.ref} matches upstream $upstream'); + return true; +} + +/// Check for a flutter_release/$[flutterVersionName] branch in all of our fork +/// repositories and that our Flutter forkpoint is included in the release +/// branch. +bool ensureReleaseBranchesIncludeForkpoint( + Checkouts checkouts, + String flutterVersionName, +) { + final forkpoint = getFlutterVersions(checkouts, flutterVersionName); + final releaseBranch = 'flutter_release/$flutterVersionName'; + final forks = [ + Repo.buildroot, + Repo.dart, + Repo.engine, + Repo.flutter, + ]; + for (final repo in forks) { + final checkout = checkouts[repo]; + final remote = + checkout.remoteBranch(branch: releaseBranch, remote: 'origin'); + final paddedName = checkout.name.padRight(10); + + // Check that the versions at flutter_release/$version include the + // expected flutter forkpoint hash. + final forkpointVersion = forkpoint[repo]; + if (!checkout.isAncestor( + ancestor: forkpointVersion.hash, + descendant: remote.hash, + )) { + logger.err( + '❌ $paddedName $releaseBranch does not include ' + 'forkpoint $forkpointVersion', + ); + return false; + } + logger.info('✅ $paddedName correctly branched'); + } + return true; +} + +/// Validate that a Shorebird release for a given [flutterVersionName] +/// exists and is consistent with its Flutter release. +bool validateRelease(Checkouts checkouts, String flutterVersionName) { + // Check that our flutter fork has the expected tag. + if (!ensureTagMatch(checkouts.flutter, flutterVersionName)) { + return false; + } + // Check that the engine has the expected tag. + if (!ensureTagMatch(checkouts.engine, flutterVersionName)) { + return false; + } + + // Check for flutter_release/$version in all of our forks + // and that our flutter forkpoint is included in the release branch. + if (!ensureReleaseBranchesIncludeForkpoint( + checkouts, + flutterVersionName, + )) { + return false; + } + return true; +} + +/// Return all Flutter versions that are tagged in the upstream Flutter repo. +/// Does not include pre-releases. +/// Returns in descending order (latest release first). +Iterable allFlutterVersions(Checkout flutter) { + final versionTags = flutter.remoteTags(remote: 'upstream'); + final semvers = []; + for (final tag in versionTags) { + // e.g. refs/tags/v1.9.7-hotfix.4 + final tagName = tag.ref; + final versionName = tagName.split('/').last; + // ignore old versions + if (versionName[0] != '3') { + continue; + } + final version = semver.Version.parse(versionName); + logger.info('Found flutter tag $version'); + if (version.isPreRelease) { + continue; + } + semvers.add(version); + } + semvers.sort(); + // Default sort is ascending, we want descending so call reversed. + return semvers.reversed.map((version) => version.toString()); +} + +/// Validate that a Shorebird release is consistent with its Flutter release. +class ValidateCommand extends CutlerCommand { + /// Constructs a new [ValidateCommand]. + ValidateCommand() { + argParser.addFlag( + 'all', + help: 'Validate all Flutter releases.', + ); + } + @override + final name = 'validate'; + @override + final description = + 'Validate that a Shorebird release is consistent with its Flutter ' + 'release.'; + + @override + int run() { + checkouts = Checkouts(config.checkoutsRoot); + + // If they passed --all, validate all Flutter releases. + // Otherwise, validate the versions they passed. + final versions = argResults!['all'] as bool + ? allFlutterVersions(checkouts.flutter) + : argResults!.rest; + + if (versions.isEmpty) { + logger + ..err('No versions to validate.') + ..info(argParser.usage); + return ExitCode.usage.code; + } + + for (final version in versions) { + logger.info('Validating $version'); + if (!validateRelease(checkouts, version)) { + return ExitCode.software.code; + } + } + return ExitCode.success.code; + } +} diff --git a/packages/cutler/lib/cutler.dart b/packages/cutler/lib/cutler.dart index 7c25a398..a4d68f21 100644 --- a/packages/cutler/lib/cutler.dart +++ b/packages/cutler/lib/cutler.dart @@ -15,6 +15,7 @@ class Cutler extends CommandRunner { Cutler() : super('cutler', 'A tool for maintaining forks of Flutter.') { addCommand(VersionsCommand()); addCommand(RebaseCommand()); + addCommand(ValidateCommand()); argParser ..addFlag('verbose', abbr: 'v') diff --git a/packages/cutler/pubspec.lock b/packages/cutler/pubspec.lock index fd0252f1..4e228d9c 100644 --- a/packages/cutler/pubspec.lock +++ b/packages/cutler/pubspec.lock @@ -352,6 +352,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.3.2" + version: + dependency: "direct main" + description: + name: version + sha256: "3d4140128e6ea10d83da32fef2fa4003fccbf6852217bb854845802f04191f94" + url: "https://pub.dev" + source: hosted + version: "3.0.2" very_good_analysis: dependency: "direct dev" description: diff --git a/packages/cutler/pubspec.yaml b/packages/cutler/pubspec.yaml index f11d529a..aef7dee3 100644 --- a/packages/cutler/pubspec.yaml +++ b/packages/cutler/pubspec.yaml @@ -21,3 +21,4 @@ dependencies: path: ^1.8.3 scoped: path: ../scoped + version: ^3.0.2