From bc3e0ecf857fab495098040606a9f357336b3d2c Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Fri, 27 Aug 2021 11:36:03 -0400 Subject: [PATCH] Remove support for bypassing, or prompting for, git tagging (#4275) We never want a plugin to be published without tagging the release, so there's no reason to support the added complexity of these flags. Similarly, once someone has confirmed publishing, we don't want to give them an opt-out for doing the tag. --- script/tool/CHANGELOG.md | 2 + .../tool/lib/src/publish_plugin_command.dart | 69 +++----- .../test/publish_plugin_command_test.dart | 150 ++++-------------- script/tool/test/util.dart | 3 +- 4 files changed, 51 insertions(+), 173 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index a32fb0016c..b10237b459 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -5,6 +5,8 @@ - Pubspec validation now checks for `implements` in implementation packages. - Pubspec valitation now checks the full relative path of `repository` entries. - `build-examples` now supports UWP plugins via a `--winuwp` flag. +- **Breaking change**: `publish` no longer accepts `--no-tag-release` or + `--no-push-flags`. Releases now always tag and push. ## 0.5.0 diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index be9e6d3001..8432e342cd 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -66,23 +66,9 @@ class PublishPluginCommand extends PluginCommand { argParser.addMultiOption(_pubFlagsOption, help: 'A list of options that will be forwarded on to pub. Separate multiple flags with commas.'); - argParser.addFlag( - _tagReleaseOption, - help: 'Whether or not to tag the release.', - defaultsTo: true, - negatable: true, - ); - argParser.addFlag( - _pushTagsOption, - help: - 'Whether or not tags should be pushed to a remote after creation. Ignored if tag-release is false.', - defaultsTo: true, - negatable: true, - ); argParser.addOption( _remoteOption, - help: - 'The name of the remote to push the tags to. Ignored if push-tags or tag-release is false.', + help: 'The name of the remote to push the tags to.', // Flutter convention is to use "upstream" for the single source of truth, and "origin" for personal forks. defaultsTo: 'upstream', ); @@ -104,15 +90,12 @@ class PublishPluginCommand extends PluginCommand { ); argParser.addFlag(_skipConfirmationFlag, help: 'Run the command without asking for Y/N inputs.\n' - 'This command will add a `--force` flag to the `pub publish` command if it is not added with $_pubFlagsOption\n' - 'It also skips the y/n inputs when pushing tags to remote.\n', + 'This command will add a `--force` flag to the `pub publish` command if it is not added with $_pubFlagsOption\n', defaultsTo: false, negatable: true); } static const String _packageOption = 'package'; - static const String _tagReleaseOption = 'tag-release'; - static const String _pushTagsOption = 'push-tags'; static const String _pubFlagsOption = 'pub-publish-flags'; static const String _remoteOption = 'remote'; static const String _allChangedFlag = 'all-changed'; @@ -150,19 +133,14 @@ class PublishPluginCommand extends PluginCommand { print('Checking local repo...'); final GitDir repository = await gitDir; - - final bool shouldPushTag = getBoolArg(_pushTagsOption); - _RemoteInfo? remote; - if (shouldPushTag) { - 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); - } - remote = _RemoteInfo(name: remoteName, url: remoteUrl); + 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); + print('Local repo is ready!'); if (getBoolArg(_dryRunFlag)) { print('=============== DRY RUN ==============='); @@ -187,7 +165,7 @@ class PublishPluginCommand extends PluginCommand { Future _publishAllChangedPackages({ required GitDir baseGitDir, - _RemoteInfo? remoteForTagPush, + required _RemoteInfo remoteForTagPush, }) async { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); final List changedPubspecs = @@ -249,24 +227,21 @@ class PublishPluginCommand extends PluginCommand { return packagesFailed.isEmpty; } - // Publish the package to pub with `pub publish`. - // If `_tagReleaseOption` is on, git tag the release. - // If `remoteForTagPush` is non-null, the tag will be pushed to that remote. + // 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({ required Directory packageDir, - _RemoteInfo? remoteForTagPush, + required _RemoteInfo remoteForTagPush, }) async { if (!await _publishPlugin(packageDir: packageDir)) { return false; } - if (getBoolArg(_tagReleaseOption)) { - if (!await _tagRelease( - packageDir: packageDir, - remoteForPush: remoteForTagPush, - )) { - return false; - } + if (!await _tagRelease( + packageDir: packageDir, + remoteForPush: remoteForTagPush, + )) { + return false; } print('Released [${packageDir.basename}] successfully.'); return true; @@ -479,14 +454,6 @@ Safe to ignore if the package is deleted in this commit. required _RemoteInfo remote, }) async { assert(remote != null && tag != null); - if (!getBoolArg(_skipConfirmationFlag)) { - print('Ready to push $tag to ${remote.url} (y/n)?'); - final String? input = _stdin.readLineSync(); - if (input?.toLowerCase() != 'y') { - print('Tag push canceled.'); - return false; - } - } if (!getBoolArg(_dryRunFlag)) { final io.ProcessResult result = await (await gitDir).runCommand( ['push', remote.name, tag], diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart index 663c2633a9..927c146a87 100644 --- a/script/tool/test/publish_plugin_command_test.dart +++ b/script/tool/test/publish_plugin_command_test.dart @@ -80,8 +80,8 @@ void main() { test('requires an existing flag', () async { Error? commandError; - final List output = await runCapturingPrint(commandRunner, - ['publish-plugin', '--package', 'iamerror', '--no-push-tags'], + final List output = await runCapturingPrint( + commandRunner, ['publish-plugin', '--package', 'iamerror'], errorHandler: (Error e) { commandError = e; }); @@ -100,8 +100,8 @@ void main() { ]; Error? commandError; - final List output = await runCapturingPrint(commandRunner, - ['publish-plugin', '--package', 'foo', '--no-push-tags'], + final List output = await runCapturingPrint( + commandRunner, ['publish-plugin', '--package', 'foo'], errorHandler: (Error e) { commandError = e; }); @@ -141,56 +141,6 @@ void main() { 'Unable to find URL for remote upstream; cannot push tags'), ])); }); - - test("doesn't validate the remote if it's not pushing tags", () async { - createFakePlugin('foo', packagesDir, examples: []); - - // Checking the remote should fail. - processRunner.mockProcessesForExecutable['git-remote'] = [ - MockProcess(exitCode: 1), - ]; - - final List output = await runCapturingPrint( - commandRunner, [ - 'publish-plugin', - '--package', - 'foo', - '--no-push-tags', - '--no-tag-release' - ]); - - expect( - output, - containsAllInOrder([ - contains('Running `pub publish ` in /packages/foo...'), - contains('Package published!'), - contains('Released [foo] successfully.'), - ])); - }); - - test('can publish non-flutter package', () async { - const String packageName = 'a_package'; - createFakePackage(packageName, packagesDir); - - final List output = await runCapturingPrint( - commandRunner, [ - 'publish-plugin', - '--package', - packageName, - '--no-push-tags', - '--no-tag-release' - ]); - - expect( - output, - containsAllInOrder( - [ - contains('Running `pub publish ` in /packages/a_package...'), - contains('Package published!'), - ], - ), - ); - }); }); group('Publishes package', () { @@ -206,13 +156,7 @@ void main() { ]; final List output = await runCapturingPrint( - commandRunner, [ - 'publish-plugin', - '--package', - 'foo', - '--no-push-tags', - '--no-tag-release' - ]); + commandRunner, ['publish-plugin', '--package', 'foo']); expect( output, @@ -227,13 +171,8 @@ void main() { mockStdin.mockUserInputs.add(utf8.encode('user input')); - await runCapturingPrint(commandRunner, [ - 'publish-plugin', - '--package', - 'foo', - '--no-push-tags', - '--no-tag-release' - ]); + await runCapturingPrint( + commandRunner, ['publish-plugin', '--package', 'foo']); expect(processRunner.mockPublishProcess.stdinMock.lines, contains('user input')); @@ -247,8 +186,6 @@ void main() { 'publish-plugin', '--package', 'foo', - '--no-push-tags', - '--no-tag-release', '--pub-publish-flags', '--dry-run,--server=foo' ]); @@ -272,8 +209,6 @@ void main() { 'publish-plugin', '--package', 'foo', - '--no-push-tags', - '--no-tag-release', '--skip-confirmation', '--pub-publish-flags', '--server=foo' @@ -300,8 +235,6 @@ void main() { 'publish-plugin', '--package', 'foo', - '--no-push-tags', - '--no-tag-release', ], errorHandler: (Error e) { commandError = e; }); @@ -324,8 +257,6 @@ void main() { '--package', 'foo', '--dry-run', - '--no-push-tags', - '--no-tag-release', ]); expect( @@ -340,6 +271,28 @@ void main() { 'Done!' ])); }); + + test('can publish non-flutter package', () async { + const String packageName = 'a_package'; + createFakePackage(packageName, packagesDir); + + final List output = + await runCapturingPrint(commandRunner, [ + 'publish-plugin', + '--package', + packageName, + ]); + + expect( + output, + containsAllInOrder( + [ + contains('Running `pub publish ` in /packages/a_package...'), + contains('Package published!'), + ], + ), + ); + }); }); group('Tags release', () { @@ -349,7 +302,6 @@ void main() { 'publish-plugin', '--package', 'foo', - '--no-push-tags', ]); expect(processRunner.recordedCalls, @@ -369,7 +321,6 @@ void main() { 'publish-plugin', '--package', 'foo', - '--no-push-tags', ], errorHandler: (Error e) { commandError = e; }); @@ -388,25 +339,6 @@ void main() { }); group('Pushes tags', () { - test('requires user confirmation', () async { - createFakePlugin('foo', packagesDir, examples: []); - - mockStdin.readLineOutput = 'help'; - - Error? commandError; - final List output = - await runCapturingPrint(commandRunner, [ - 'publish-plugin', - '--package', - 'foo', - ], errorHandler: (Error e) { - commandError = e; - }); - - expect(commandError, isA()); - expect(output, contains('Tag push canceled.')); - }); - test('to upstream by default', () async { createFakePlugin('foo', packagesDir, examples: []); @@ -502,30 +434,6 @@ void main() { contains('Released [foo] successfully.'), ])); }); - - test('only if tagging and pushing to remotes are both enabled', () async { - createFakePlugin('foo', packagesDir, examples: []); - - final List output = - await runCapturingPrint(commandRunner, [ - 'publish-plugin', - '--package', - 'foo', - '--no-tag-release', - ]); - - expect( - processRunner.recordedCalls - .map((ProcessCall call) => call.executable), - isNot(contains('git-push'))); - expect( - output, - containsAllInOrder([ - contains('Running `pub publish ` in /packages/foo...'), - contains('Package published!'), - contains('Released [foo] successfully.'), - ])); - }); }); group('Auto release (all-changed flag)', () { diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index 9b92a5d94a..74c0364892 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -107,7 +107,8 @@ Directory createFakePackage( final Directory packageDirectory = parentDirectory.childDirectory(name); packageDirectory.createSync(recursive: true); - createFakePubspec(packageDirectory, name: name, isFlutter: isFlutter); + createFakePubspec(packageDirectory, + name: name, isFlutter: isFlutter, version: version); createFakeCHANGELOG(packageDirectory, ''' ## $version * Some changes.