diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 00caeb30ef42..96dd881bfe00 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -92,6 +92,18 @@ abstract class PackageLoopingCommand extends PluginCommand { /// arguments are invalid), and to set up any run-level state. Future initializeRun() async {} + /// Returns the packages to process. By default, this returns the packages + /// defined by the standard tooling flags and the [inculdeSubpackages] option, + /// but can be overridden for custom package enumeration. + /// + /// Note: Consistent behavior across commands whenever possibel is a goal for + /// this tool, so this should be overridden only in rare cases. + Stream getPackagesToProcess() async* { + yield* includeSubpackages + ? getTargetPackagesAndSubpackages(filterExcluded: false) + : getTargetPackages(filterExcluded: false); + } + /// Runs the command for [package], returning a list of errors. /// /// Errors may either be an empty string if there is no context that should @@ -138,6 +150,9 @@ abstract class PackageLoopingCommand extends PluginCommand { /// context. String get failureListFooter => 'See above for full details.'; + /// The summary string used for a successful run in the final overview output. + String get successSummaryMessage => 'ran'; + /// If true, all printing (including the summary) will be redirected to a /// buffer, and provided in a call to [handleCapturedOutput] at the end of /// the run. @@ -206,9 +221,8 @@ abstract class PackageLoopingCommand extends PluginCommand { await initializeRun(); - final List targetPackages = includeSubpackages - ? await getTargetPackagesAndSubpackages(filterExcluded: false).toList() - : await getTargetPackages(filterExcluded: false).toList(); + final List targetPackages = + await getPackagesToProcess().toList(); final Map results = {}; @@ -346,7 +360,7 @@ abstract class PackageLoopingCommand extends PluginCommand { summary = 'skipped'; style = hadWarning ? Styles.LIGHT_YELLOW : Styles.DARK_GRAY; } else { - summary = 'ran'; + summary = successSummaryMessage; style = hadWarning ? Styles.YELLOW : Styles.GREEN; } if (hadWarning) { diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index aafe7868d8d0..0db400a1d32a 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -7,7 +7,6 @@ import 'dart:convert'; import 'dart:io' as io; import 'package:file/file.dart'; -import 'package:flutter_plugin_tools/src/common/repository_package.dart'; import 'package:git/git.dart'; import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; @@ -19,9 +18,11 @@ import 'package:yaml/yaml.dart'; import 'common/core.dart'; import 'common/git_version_finder.dart'; +import 'common/package_looping_command.dart'; import 'common/plugin_command.dart'; import 'common/process_runner.dart'; import 'common/pub_version_finder.dart'; +import 'common/repository_package.dart'; @immutable class _RemoteInfo { @@ -45,7 +46,7 @@ class _RemoteInfo { /// usage information. /// /// [processRunner], [print], and [stdin] can be overriden for easier testing. -class PublishPluginCommand extends PluginCommand { +class PublishPluginCommand extends PackageLoopingCommand { /// Creates an instance of the publish command. PublishPluginCommand( Directory packagesDir, { @@ -116,38 +117,45 @@ class PublishPluginCommand extends PluginCommand { StreamSubscription? _stdinSubscription; final PubVersionFinder _pubVersionFinder; + // Tags that already exist in the repository. + List _existingGitTags = []; + // The remote to push tags to. + late _RemoteInfo _remote; + + @override + String get successSummaryMessage => 'published'; + + @override + String get failureListHeader => + 'The following packages had failures during publishing:'; + @override - Future run() async { + Future initializeRun() async { print('Checking local repo...'); - final GitDir repository = await gitDir; + + // Ensure that the requested remote is present. final String remoteName = getStringArg(_remoteOption); final String? remoteUrl = await _verifyRemote(remoteName); if (remoteUrl == null) { printError('Unable to find URL for remote $remoteName; cannot push tags'); throw ToolExit(1); } - final _RemoteInfo remote = _RemoteInfo(name: remoteName, url: remoteUrl); + _remote = _RemoteInfo(name: remoteName, url: remoteUrl); + + // Pre-fetch all the repository's tags, to check against when publishing. + final GitDir repository = await gitDir; + final io.ProcessResult existingTagsResult = + await repository.runCommand(['tag', '--sort=-committerdate']); + _existingGitTags = (existingTagsResult.stdout as String).split('\n') + ..removeWhere((String element) => element.isEmpty); - print('Local repo is ready!'); if (getBoolArg(_dryRunFlag)) { - print('=============== DRY RUN ==============='); + print('=============== DRY RUN ==============='); } - - final List packages = await _getPackagesToProcess() - .where((PackageEnumerationEntry entry) => !entry.excluded) - .toList(); - bool successful = true; - - successful = await _publishPackages( - packages, - baseGitDir: repository, - remoteForTagPush: remote, - ); - - await _finish(successful); } - Stream _getPackagesToProcess() async* { + @override + Stream getPackagesToProcess() async* { if (getBoolArg(_allChangedFlag)) { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); final List changedPubspecs = @@ -169,92 +177,52 @@ class PublishPluginCommand extends PluginCommand { } } - Future _publishPackages( - List packages, { - required GitDir baseGitDir, - required _RemoteInfo remoteForTagPush, - }) async { - if (packages.isEmpty) { - print('No version updates in this commit.'); - return true; + @override + Future runForPackage(RepositoryPackage package) async { + final PackageResult? checkResult = await _checkNeedsRelease(package); + if (checkResult != null) { + return checkResult; } - final io.ProcessResult existingTagsResult = - await baseGitDir.runCommand(['tag', '--sort=-committerdate']); - final List existingTags = (existingTagsResult.stdout as String) - .split('\n') - ..removeWhere((String element) => element.isEmpty); - - final List packagesReleased = []; - final List packagesFailed = []; - - for (final PackageEnumerationEntry entry in packages) { - final RepositoryPackage package = entry.package; - - final _CheckNeedsReleaseResult result = await _checkNeedsRelease( - package: package, - existingTags: existingTags, - ); - switch (result) { - case _CheckNeedsReleaseResult.release: - break; - case _CheckNeedsReleaseResult.noRelease: - continue; - case _CheckNeedsReleaseResult.failure: - packagesFailed.add(package.displayName); - continue; - } - print('\n'); - if (await _publishAndTagPackage(package, - remoteForTagPush: remoteForTagPush)) { - packagesReleased.add(package.displayName); - } else { - packagesFailed.add(package.displayName); - } - print('\n'); + if (!await _checkGitStatus(package)) { + return PackageResult.fail(['uncommitted changes']); } - if (packagesReleased.isNotEmpty) { - print('Packages released: ${packagesReleased.join(', ')}'); + + if (!await _publish(package)) { + return PackageResult.fail(['publish failed']); } - if (packagesFailed.isNotEmpty) { - printError( - 'Failed to release the following packages: ${packagesFailed.join(', ')}, see above for details.'); + + if (!await _tagRelease(package)) { + return PackageResult.fail(['tagging failed']); } - return packagesFailed.isEmpty; + + print('\nPublished ${package.directory.basename} successfully!'); + return PackageResult.success(); } - // Publish the package to pub with `pub publish`, then git tag the release - // and push the tag to [remoteForTagPush]. - // Returns `true` if publishing and tagging are successful. - Future _publishAndTagPackage( - RepositoryPackage package, { - _RemoteInfo? remoteForTagPush, - }) async { - if (!await _publishPackage(package)) { - return false; - } - if (!await _tagRelease( - package, - remoteForPush: remoteForTagPush, - )) { - return false; - } - print('Published ${package.directory.basename} successfully.'); - return true; + @override + Future completeRun() async { + _pubVersionFinder.httpClient.close(); + await _stdinSubscription?.cancel(); + _stdinSubscription = null; } - // Returns a [_CheckNeedsReleaseResult] that indicates the result. - Future<_CheckNeedsReleaseResult> _checkNeedsRelease({ - required RepositoryPackage package, - required List existingTags, - }) async { + /// Checks whether [package] needs to be released, printing check status and + /// returning one of: + /// - PackageResult.fail if the check could not be completed + /// - PackageResult.skip if no release is necessary + /// - null if releasing should proceed + /// + /// In cases where a non-null result is returned, that should be returned + /// as the final result for the package, without further processing. + Future _checkNeedsRelease(RepositoryPackage package) async { final File pubspecFile = package.pubspecFile; if (!pubspecFile.existsSync()) { - print(''' + logWarning(''' The pubspec file at ${pubspecFile.path} does not exist. Publishing will not happen for ${pubspecFile.parent.basename}. Safe to ignore if the package is deleted in this commit. '''); - return _CheckNeedsReleaseResult.noRelease; + return PackageResult.skip('package deleted'); } final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync()); @@ -263,17 +231,18 @@ Safe to ignore if the package is deleted in this commit. // Ignore flutter_plugin_tools package when running publishing through flutter_plugin_tools. // TODO(cyanglaz): Make the tool also auto publish flutter_plugin_tools package. // https://github.com/flutter/flutter/issues/85430 - return _CheckNeedsReleaseResult.noRelease; + return PackageResult.skip( + 'publishing flutter_plugin_tools via the tool is not supported'); } if (pubspec.publishTo == 'none') { - return _CheckNeedsReleaseResult.noRelease; + return PackageResult.skip('publish_to: none'); } if (pubspec.version == null) { printError( 'No version found. A package that intentionally has no version should be marked "publish_to: none"'); - return _CheckNeedsReleaseResult.failure; + return PackageResult.fail(['no version']); } // Check if the package named `packageName` with `version` has already @@ -282,49 +251,29 @@ Safe to ignore if the package is deleted in this commit. final PubVersionFinderResponse pubVersionFinderResponse = await _pubVersionFinder.getPackageVersion(packageName: pubspec.name); if (pubVersionFinderResponse.versions.contains(version)) { - final String tagsForPackageWithSameVersion = existingTags.firstWhere( + final String tagsForPackageWithSameVersion = _existingGitTags.firstWhere( (String tag) => tag.split('-v').first == pubspec.name && tag.split('-v').last == version.toString(), orElse: () => ''); - print( - 'The version $version of ${pubspec.name} has already been published'); if (tagsForPackageWithSameVersion.isEmpty) { printError( - 'However, the git release tag for this version (${pubspec.name}-v$version) is not found. Please manually fix the tag then run the command again.'); - return _CheckNeedsReleaseResult.failure; + '${pubspec.name} $version has already been published, however ' + 'the git release tag (${pubspec.name}-v$version) was not found. ' + 'Please manually fix the tag then run the command again.'); + return PackageResult.fail(['published but untagged']); } else { - print('skip.'); - return _CheckNeedsReleaseResult.noRelease; + print('${pubspec.name} $version has already been published.'); + return PackageResult.skip('already published'); } } - return _CheckNeedsReleaseResult.release; - } - - // Publish the package. - // - // Returns `true` if successful, `false` otherwise. - Future _publishPackage(RepositoryPackage package) async { - final bool gitStatusOK = await _checkGitStatus(package); - if (!gitStatusOK) { - return false; - } - final bool publishOK = await _publish(package); - if (!publishOK) { - return false; - } - print('Package published!'); - return true; + return null; } - // Tag the release with -v, and, if [remoteForTagPush] - // is provided, push it to that remote. + // Tag the release with -v, and push it to the remote. // // Return `true` if successful, `false` otherwise. - Future _tagRelease( - RepositoryPackage package, { - _RemoteInfo? remoteForPush, - }) async { + Future _tagRelease(RepositoryPackage package) async { final String tag = _getTag(package); print('Tagging release $tag...'); if (!getBoolArg(_dryRunFlag)) { @@ -337,27 +286,15 @@ Safe to ignore if the package is deleted in this commit. } } - if (remoteForPush == null) { - return true; - } - - print('Pushing tag to ${remoteForPush.name}...'); - return await _pushTagToRemote( + print('Pushing tag to ${_remote.name}...'); + final bool success = await _pushTagToRemote( tag: tag, - remote: remoteForPush, + remote: _remote, ); - } - - Future _finish(bool successful) async { - _pubVersionFinder.httpClient.close(); - await _stdinSubscription?.cancel(); - _stdinSubscription = null; - if (successful) { - print('Done!'); - } else { - printError('Failed, see above for details.'); - throw ToolExit(1); + if (success) { + print('Release tagged!'); } + return success; } Future _checkGitStatus(RepositoryPackage package) async { @@ -396,6 +333,7 @@ Safe to ignore if the package is deleted in this commit. } Future _publish(RepositoryPackage package) async { + print('Publishing...'); final List publishFlags = getStringListArg(_pubFlagsOption); print('Running `pub publish ${publishFlags.join(' ')}` in ' '${package.directory.absolute.path}...\n'); @@ -423,6 +361,8 @@ Safe to ignore if the package is deleted in this commit. printError('Publishing ${package.directory.basename} failed.'); return false; } + + print('Package published!'); return true; } @@ -518,14 +458,3 @@ final String _credentialsPath = () { return p.join(cacheDir, 'credentials.json'); }(); - -enum _CheckNeedsReleaseResult { - // The package needs to be released. - release, - - // The package does not need to be released. - noRelease, - - // There's an error when trying to determine whether the package needs to be released. - failure, -} diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart index ae3d768fcc70..2ea4fc753460 100644 --- a/script/tool/test/publish_plugin_command_test.dart +++ b/script/tool/test/publish_plugin_command_test.dart @@ -108,7 +108,8 @@ void main() { '?? /packages/foo/tmp\n\n' 'If the directory should be clean, you can run `git clean -xdf && ' 'git reset --hard HEAD` to wipe all local changes.'), - contains('Failed, see above for details.'), + contains('foo:\n' + ' uncommitted changes'), ])); }); @@ -264,10 +265,13 @@ void main() { isNot(contains('git-push'))); expect( output, - containsAllInOrder([ - '=============== DRY RUN ===============', - 'Running `pub publish ` in ${pluginDir.path}...\n', - 'Done!' + containsAllInOrder([ + contains('=============== DRY RUN ==============='), + contains('Running for foo'), + contains('Running `pub publish ` in ${pluginDir.path}...'), + contains('Tagging release foo-v0.0.1...'), + contains('Pushing tag to upstream...'), + contains('Published foo successfully!'), ])); }); @@ -353,7 +357,8 @@ void main() { expect( output, containsAllInOrder([ - contains('Published foo successfully.'), + contains('Pushing tag to upstream...'), + contains('Published foo successfully!'), ])); }); @@ -376,7 +381,7 @@ void main() { expect( output, containsAllInOrder([ - contains('Published foo successfully.'), + contains('Published foo successfully!'), ])); }); @@ -395,12 +400,12 @@ void main() { isNot(contains('git-push'))); expect( output, - containsAllInOrder([ - '=============== DRY RUN ===============', - 'Running `pub publish ` in ${pluginDir.path}...\n', - 'Tagging release foo-v0.0.1...', - 'Pushing tag to upstream...', - 'Done!' + containsAllInOrder([ + contains('=============== DRY RUN ==============='), + contains('Running `pub publish ` in ${pluginDir.path}...'), + contains('Tagging release foo-v0.0.1...'), + contains('Pushing tag to upstream...'), + contains('Published foo successfully!'), ])); }); @@ -424,7 +429,7 @@ void main() { expect( output, containsAllInOrder([ - contains('Published foo successfully.'), + contains('Published foo successfully!'), ])); }); }); @@ -460,13 +465,11 @@ void main() { expect( output, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - 'Running `pub publish ` in ${pluginDir1.path}...\n', - 'Running `pub publish ` in ${pluginDir2.path}...\n', - 'Packages released: plugin1, plugin2/plugin2', - 'Done!' + containsAllInOrder([ + contains('Running `pub publish ` in ${pluginDir1.path}...'), + contains('Running `pub publish ` in ${pluginDir2.path}...'), + contains('plugin1 - \x1B[32mpublished\x1B[0m'), + contains('plugin2/plugin2 - \x1B[32mpublished\x1B[0m'), ])); expect( processRunner.recordedCalls, @@ -522,12 +525,8 @@ void main() { expect( output, containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', 'Running `pub publish ` in ${pluginDir1.path}...\n', 'Running `pub publish ` in ${pluginDir2.path}...\n', - 'Packages released: plugin1, plugin2/plugin2', - 'Done!' ])); expect( processRunner.recordedCalls, @@ -573,18 +572,16 @@ void main() { expect( output, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - '=============== DRY RUN ===============', - 'Running `pub publish ` in ${pluginDir1.path}...\n', - 'Tagging release plugin1-v0.0.1...', - 'Pushing tag to upstream...', - 'Running `pub publish ` in ${pluginDir2.path}...\n', - 'Tagging release plugin2-v0.0.1...', - 'Pushing tag to upstream...', - 'Packages released: plugin1, plugin2/plugin2', - 'Done!' + containsAllInOrder([ + contains('=============== DRY RUN ==============='), + contains('Running `pub publish ` in ${pluginDir1.path}...'), + contains('Tagging release plugin1-v0.0.1...'), + contains('Pushing tag to upstream...'), + contains('Published plugin1 successfully!'), + contains('Running `pub publish ` in ${pluginDir2.path}...'), + contains('Tagging release plugin2-v0.0.1...'), + contains('Pushing tag to upstream...'), + contains('Published plugin2 successfully!'), ])); expect( processRunner.recordedCalls @@ -623,13 +620,11 @@ void main() { ['publish-plugin', '--all-changed', '--base-sha=HEAD~']); expect( output2, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - 'Running `pub publish ` in ${pluginDir1.path}...\n', - 'Running `pub publish ` in ${pluginDir2.path}...\n', - 'Packages released: plugin1, plugin2/plugin2', - 'Done!' + containsAllInOrder([ + contains('Running `pub publish ` in ${pluginDir1.path}...'), + contains('Published plugin1 successfully!'), + contains('Running `pub publish ` in ${pluginDir2.path}...'), + contains('Published plugin2 successfully!'), ])); expect( processRunner.recordedCalls, @@ -642,7 +637,7 @@ void main() { }); test( - 'delete package will not trigger publish but exit the command successfully.', + 'delete package will not trigger publish but exit the command successfully!', () async { mockHttpResponses['plugin1'] = { 'name': 'plugin1', @@ -674,13 +669,13 @@ void main() { ['publish-plugin', '--all-changed', '--base-sha=HEAD~']); expect( output2, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - 'Running `pub publish ` in ${pluginDir1.path}...\n', - 'The pubspec file at ${pluginDir2.childFile('pubspec.yaml').path} does not exist. Publishing will not happen for plugin2.\nSafe to ignore if the package is deleted in this commit.\n', - 'Packages released: plugin1', - 'Done!' + containsAllInOrder([ + contains('Running `pub publish ` in ${pluginDir1.path}...'), + contains('Published plugin1 successfully!'), + contains( + 'The pubspec file at ${pluginDir2.childFile('pubspec.yaml').path} does not exist. Publishing will not happen for plugin2.\nSafe to ignore if the package is deleted in this commit.\n'), + contains('SKIPPING: package deleted'), + contains('skipped (with warning)'), ])); expect( processRunner.recordedCalls, @@ -724,14 +719,11 @@ void main() { expect( output, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - 'The version 0.0.2 of plugin1 has already been published', - 'skip.', - 'The version 0.0.2 of plugin2 has already been published', - 'skip.', - 'Done!' + containsAllInOrder([ + contains('plugin1 0.0.2 has already been published'), + contains('SKIPPING: already published'), + contains('plugin2 0.0.2 has already been published'), + contains('SKIPPING: already published'), ])); expect( @@ -778,12 +770,10 @@ void main() { expect( output, containsAllInOrder([ - contains('The version 0.0.2 of plugin1 has already been published'), - contains( - 'However, the git release tag for this version (plugin1-v0.0.2) is not found.'), - contains('The version 0.0.2 of plugin2 has already been published'), - contains( - 'However, the git release tag for this version (plugin2-v0.0.2) is not found.'), + contains('plugin1 0.0.2 has already been published, ' + 'however the git release tag (plugin1-v0.0.2) was not found.'), + contains('plugin2 0.0.2 has already been published, ' + 'however the git release tag (plugin2-v0.0.2) was not found.'), ])); expect( processRunner.recordedCalls @@ -807,14 +797,7 @@ void main() { final List output = await runCapturingPrint(commandRunner, ['publish-plugin', '--all-changed', '--base-sha=HEAD~']); - expect( - output, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - 'No version updates in this commit.', - 'Done!' - ])); + expect(output, containsAllInOrder(['Ran for 0 package(s)'])); expect( processRunner.recordedCalls .map((ProcessCall call) => call.executable), @@ -838,14 +821,13 @@ void main() { expect( output, - containsAllInOrder([ - 'Checking local repo...', - 'Local repo is ready!', - 'Done!' + containsAllInOrder([ + contains( + 'SKIPPING: publishing flutter_plugin_tools via the tool is not supported') ])); expect( output.contains( - 'Running `pub publish ` in ${flutterPluginTools.path}...\n', + 'Running `pub publish ` in ${flutterPluginTools.path}...', ), isFalse); expect(