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

@@ -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
 

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<bool> _publishAllChangedPackages({
     required GitDir baseGitDir,
-    _RemoteInfo? remoteForTagPush,
+    required _RemoteInfo remoteForTagPush,
   }) async {
     final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
     final List<String> 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<bool> _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(
         <String>['push', remote.name, tag],

script/tool/test/publish_plugin_command_test.dart

@@ -80,8 +80,8 @@ void main() {
 
     test('requires an existing flag', () async {
       Error? commandError;
-      final List<String> output = await runCapturingPrint(commandRunner,
-          <String>['publish-plugin', '--package', 'iamerror', '--no-push-tags'],
+      final List<String> output = await runCapturingPrint(
+          commandRunner, <String>['publish-plugin', '--package', 'iamerror'],
           errorHandler: (Error e) {
         commandError = e;
       });
@@ -100,8 +100,8 @@ void main() {
       ];
 
       Error? commandError;
-      final List<String> output = await runCapturingPrint(commandRunner,
-          <String>['publish-plugin', '--package', 'foo', '--no-push-tags'],
+      final List<String> output = await runCapturingPrint(
+          commandRunner, <String>['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: <String>[]);
-
-      // Checking the remote should fail.
-      processRunner.mockProcessesForExecutable['git-remote'] = <io.Process>[
-        MockProcess(exitCode: 1),
-      ];
-
-      final List<String> output = await runCapturingPrint(
-          commandRunner, <String>[
-        'publish-plugin',
-        '--package',
-        'foo',
-        '--no-push-tags',
-        '--no-tag-release'
-      ]);
-
-      expect(
-          output,
-          containsAllInOrder(<Matcher>[
-            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<String> output = await runCapturingPrint(
-          commandRunner, <String>[
-        'publish-plugin',
-        '--package',
-        packageName,
-        '--no-push-tags',
-        '--no-tag-release'
-      ]);
-
-      expect(
-        output,
-        containsAllInOrder(
-          <Matcher>[
-            contains('Running `pub publish ` in /packages/a_package...'),
-            contains('Package published!'),
-          ],
-        ),
-      );
-    });
   });
 
   group('Publishes package', () {
@@ -206,13 +156,7 @@ void main() {
       ];
 
       final List<String> output = await runCapturingPrint(
-          commandRunner, <String>[
-        'publish-plugin',
-        '--package',
-        'foo',
-        '--no-push-tags',
-        '--no-tag-release'
-      ]);
+          commandRunner, <String>['publish-plugin', '--package', 'foo']);
 
       expect(
           output,
@@ -227,13 +171,8 @@ void main() {
 
       mockStdin.mockUserInputs.add(utf8.encode('user input'));
 
-      await runCapturingPrint(commandRunner, <String>[
-        'publish-plugin',
-        '--package',
-        'foo',
-        '--no-push-tags',
-        '--no-tag-release'
-      ]);
+      await runCapturingPrint(
+          commandRunner, <String>['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<String> output =
+          await runCapturingPrint(commandRunner, <String>[
+        'publish-plugin',
+        '--package',
+        packageName,
+      ]);
+
+      expect(
+        output,
+        containsAllInOrder(
+          <Matcher>[
+            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: <String>[]);
-
-      mockStdin.readLineOutput = 'help';
-
-      Error? commandError;
-      final List<String> output =
-          await runCapturingPrint(commandRunner, <String>[
-        'publish-plugin',
-        '--package',
-        'foo',
-      ], errorHandler: (Error e) {
-        commandError = e;
-      });
-
-      expect(commandError, isA<ToolExit>());
-      expect(output, contains('Tag push canceled.'));
-    });
-
     test('to upstream by default', () async {
       createFakePlugin('foo', packagesDir, examples: <String>[]);
 
@@ -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: <String>[]);
-
-      final List<String> output =
-          await runCapturingPrint(commandRunner, <String>[
-        'publish-plugin',
-        '--package',
-        'foo',
-        '--no-tag-release',
-      ]);
-
-      expect(
-          processRunner.recordedCalls
-              .map((ProcessCall call) => call.executable),
-          isNot(contains('git-push')));
-      expect(
-          output,
-          containsAllInOrder(<Matcher>[
-            contains('Running `pub publish ` in /packages/foo...'),
-            contains('Package published!'),
-            contains('Released [foo] successfully.'),
-          ]));
-    });
   });
 
   group('Auto release (all-changed flag)', () {

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.