[tool] Add features to support GCB auto-publish flow (flutter#6218)

stuartmorgan-g · web-flow · commit 6a4e2ffbc4d6 · 2024-03-05T22:34:58.000Z
Adds the flowing to the tool: - A new `--exact-match-only` flag to be used with `--packages` to prevent group matching (i.e., a selection like `--packages=path_provider --exact-match-only` would only run on `packages/path_provider/path_provider`, not `packages/path_provider/*`). - Two new `publish` command flags: - `--tag-for-auto-publish`, to do all the steps that `publish` currently does except for the real `pub publish`, so it would dry-run the publish and then create and push the tag if successful. - `--already-tagged`, to skip the step of adding and pushing a tag, and replace it with a check that `HEAD` already has the expected tag. This set of additions supports a workflow where the current `release` step is changed to use `--tag-for-auto-publish`, and then the separate auto-publish system would publish each package with `... publish --already-tagged --packages=<some package> --exact-match-only`. See flutter/packages#5005 (comment) for previous discussion/context. Part of flutter#126827
diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart
@@ -50,7 +50,11 @@ abstract class PackageCommand extends Command<void> {
     argParser.addMultiOption(
       _packagesArg,
       help:
-          'Specifies which packages the command should run on (before sharding).\n',
+          'Specifies which packages the command should run on (before sharding).\n'
+          'If a package name is the name of a plugin group, it will include '
+          'the entire group; to avoid this, use group/package as the name '
+          '(e.g., shared_preferences/shared_preferences), or pass '
+          '--$_exactMatchOnlyArg',
       valueHelp: 'package1,package2,...',
       aliases: <String>[_pluginsLegacyAliasArg],
     );
@@ -67,6 +71,9 @@ abstract class PackageCommand extends Command<void> {
       valueHelp: 'n',
       defaultsTo: '1',
     );
+    argParser.addFlag(_exactMatchOnlyArg,
+        help: 'Disables package group matching in package selection.',
+        negatable: false);
     argParser.addMultiOption(
       _excludeArg,
       abbr: 'e',
@@ -136,6 +143,7 @@ abstract class PackageCommand extends Command<void> {
   static const String _pluginsLegacyAliasArg = 'plugins';
   static const String _runOnChangedPackagesArg = 'run-on-changed-packages';
   static const String _runOnDirtyPackagesArg = 'run-on-dirty-packages';
+  static const String _exactMatchOnlyArg = 'exact-match-only';
   static const String _excludeArg = 'exclude';
   static const String _filterPackagesArg = 'filter-packages-to';
   // Diff base selection.
@@ -361,6 +369,15 @@ abstract class PackageCommand extends Command<void> {
       throw ToolExit(exitInvalidArguments);
     }
 
+    // Whether to require that a package name exactly match to be included,
+    // rather than allowing package groups for federated plugins. Any cases
+    // where the set of packages is determined programatically based on repo
+    // state should use exact matching.
+    final bool allowGroupMatching = !(getBoolArg(_exactMatchOnlyArg) ||
+        argResults!.wasParsed(_runOnChangedPackagesArg) ||
+        argResults!.wasParsed(_runOnDirtyPackagesArg) ||
+        argResults!.wasParsed(_packagesForBranchArg));
+
     Set<String> packages = Set<String>.from(getStringListArg(_packagesArg));
 
     final GitVersionFinder? changedFileFinder;
@@ -458,6 +475,30 @@ abstract class PackageCommand extends Command<void> {
           excludeAllButPackageNames.intersection(possibleNames).isEmpty;
     }
 
+    await for (final RepositoryPackage package in _everyTopLevelPackage()) {
+      if (packages.isEmpty ||
+          packages
+              .intersection(_possiblePackageIdentifiers(package,
+                  allowGroup: allowGroupMatching))
+              .isNotEmpty) {
+        // Exclusion is always human input, so groups should always be allowed
+        // unless they have been specifically forbidden.
+        final bool excluded = isExcluded(_possiblePackageIdentifiers(package,
+            allowGroup: !getBoolArg(_exactMatchOnlyArg)));
+        yield PackageEnumerationEntry(package, excluded: excluded);
+      }
+    }
+  }
+
+  /// Returns every top-level package in the repository, according to repository
+  /// conventions.
+  ///
+  /// In particular, it returns:
+  /// - Every package that is a direct child of one of the know "packages"
+  ///   directories.
+  /// - Every package that is a direct child of a non-package subdirectory of
+  ///   one of those directories (to cover federated plugin groups).
+  Stream<RepositoryPackage> _everyTopLevelPackage() async* {
     for (final Directory dir in <Directory>[
       packagesDir,
       if (thirdPartyPackagesDir.existsSync()) thirdPartyPackagesDir,
@@ -466,40 +507,44 @@ abstract class PackageCommand extends Command<void> {
           in dir.list(followLinks: false)) {
         // A top-level Dart package is a standard package.
         if (isPackage(entity)) {
-          if (packages.isEmpty || packages.contains(p.basename(entity.path))) {
-            yield PackageEnumerationEntry(
-                RepositoryPackage(entity as Directory),
-                excluded: isExcluded(<String>{entity.basename}));
-          }
+          yield RepositoryPackage(entity as Directory);
         } else if (entity is Directory) {
           // Look for Dart packages under this top-level directory; this is the
           // standard structure for federated plugins.
           await for (final FileSystemEntity subdir
               in entity.list(followLinks: false)) {
             if (isPackage(subdir)) {
-              // There are three ways for a federated plugin to match:
-              // - package name (path_provider_android)
-              // - fully specified name (path_provider/path_provider_android)
-              // - group name (path_provider), which matches all packages in
-              //   the group
-              final Set<String> possibleMatches = <String>{
-                path.basename(subdir.path), // package name
-                path.basename(entity.path), // group name
-                path.relative(subdir.path, from: dir.path), // fully specified
-              };
-              if (packages.isEmpty ||
-                  packages.intersection(possibleMatches).isNotEmpty) {
-                yield PackageEnumerationEntry(
-                    RepositoryPackage(subdir as Directory),
-                    excluded: isExcluded(possibleMatches));
-              }
+              yield RepositoryPackage(subdir as Directory);
             }
           }
         }
       }
     }
   }
 
+  Set<String> _possiblePackageIdentifiers(
+    RepositoryPackage package, {
+    required bool allowGroup,
+  }) {
+    final String packageName = path.basename(package.path);
+    if (package.isFederated) {
+      // There are three ways for a federated plugin to be identified:
+      // - package name (path_provider_android).
+      // - fully specified name (path_provider/path_provider_android).
+      // - group name (path_provider), which includes all packages in
+      //   the group.
+      final io.Directory parentDir = package.directory.parent;
+      return <String>{
+        packageName,
+        path.relative(package.path,
+            from: parentDir.parent.path), // fully specified
+        if (allowGroup) path.basename(parentDir.path), // group name
+      };
+    } else {
+      return <String>{packageName};
+    }
+  }
+
   /// Returns all Dart package folders (typically, base package + example) of
   /// the packages involved in this command execution.
   ///
diff --git a/script/tool/lib/src/publish_command.dart b/script/tool/lib/src/publish_command.dart
@@ -58,6 +58,11 @@ class PublishCommand extends PackageLoopingCommand {
   })  : _pubVersionFinder =
             PubVersionFinder(httpClient: httpClient ?? http.Client()),
         _stdin = stdinput ?? io.stdin {
+    argParser.addFlag(_alreadyTaggedFlag,
+        help:
+            'Instead of tagging, validates that the current checkout is already tagged with the expected version.\n'
+            'This is primarily intended for use in CI publish steps triggered by tagging.',
+        negatable: false);
     argParser.addMultiOption(_pubFlagsOption,
         help:
             'A list of options that will be forwarded on to pub. Separate multiple flags with commas.');
@@ -83,13 +88,20 @@ class PublishCommand extends PackageLoopingCommand {
     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');
+    argParser.addFlag(_tagForAutoPublishFlag,
+        help:
+            'Runs the dry-run publish, and tags if it succeeds, but does not actually publish.\n'
+            'This is intended for use with a separate publish step that is based on tag push events.',
+        negatable: false);
   }
 
+  static const String _alreadyTaggedFlag = 'already-tagged';
   static const String _pubFlagsOption = 'pub-publish-flags';
   static const String _remoteOption = 'remote';
   static const String _allChangedFlag = 'all-changed';
   static const String _dryRunFlag = 'dry-run';
   static const String _skipConfirmationFlag = 'skip-confirmation';
+  static const String _tagForAutoPublishFlag = 'tag-for-auto-publish';
 
   static const String _pubCredentialName = 'PUB_CREDENTIALS';
 
@@ -193,15 +205,27 @@ class PublishCommand extends PackageLoopingCommand {
       return PackageResult.fail(<String>['uncommitted changes']);
     }
 
-    if (!await _publish(package)) {
-      return PackageResult.fail(<String>['publish failed']);
+    final bool tagOnly = getBoolArg(_tagForAutoPublishFlag);
+    if (!tagOnly) {
+      if (!await _publish(package)) {
+        return PackageResult.fail(<String>['publish failed']);
+      }
     }
 
-    if (!await _tagRelease(package)) {
-      return PackageResult.fail(<String>['tagging failed']);
+    final String tag = _getTag(package);
+    if (getBoolArg(_alreadyTaggedFlag)) {
+      if (!(await _getCurrentTags()).contains(tag)) {
+        printError('The current checkout is not already tagged "$tag"');
+        return PackageResult.fail(<String>['missing tag']);
+      }
+    } else {
+      if (!await _tagRelease(package, tag)) {
+        return PackageResult.fail(<String>['tagging failed']);
+      }
     }
 
-    print('\nPublished ${package.directory.basename} successfully!');
+    final String action = tagOnly ? 'Tagged' : 'Published';
+    print('\n$action ${package.directory.basename} successfully!');
     return PackageResult.success();
   }
 
@@ -277,8 +301,7 @@ Safe to ignore if the package is deleted in this commit.
   // Tag the release with <package-name>-v<version>, and push it to the remote.
   //
   // Return `true` if successful, `false` otherwise.
-  Future<bool> _tagRelease(RepositoryPackage package) async {
-    final String tag = _getTag(package);
+  Future<bool> _tagRelease(RepositoryPackage package, String tag) async {
     print('Tagging release $tag...');
     if (!getBoolArg(_dryRunFlag)) {
       final io.ProcessResult result = await (await gitDir).runCommand(
@@ -301,6 +324,22 @@ Safe to ignore if the package is deleted in this commit.
     return success;
   }
 
+  Future<Iterable<String>> _getCurrentTags() async {
+    // git tag --points-at HEAD
+    final io.ProcessResult tagsResult = await (await gitDir).runCommand(
+      <String>['tag', '--points-at', 'HEAD'],
+      throwOnError: false,
+    );
+    if (tagsResult.exitCode != 0) {
+      return <String>[];
+    }
+
+    return (tagsResult.stdout as String)
+        .split('\n')
+        .map((String line) => line.trim())
+        .where((String line) => line.isNotEmpty);
+  }
+
   Future<bool> _checkGitStatus(RepositoryPackage package) async {
     final io.ProcessResult statusResult = await (await gitDir).runCommand(
       <String>[
diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart
@@ -222,11 +222,6 @@ void main() {
     test(
         'explicitly specifying the plugin (group) name of a federated plugin '
         'should include all plugins in the group', () async {
-      processRunner.mockProcessesForExecutable['git-diff'] = <FakeProcessInfo>[
-        FakeProcessInfo(MockProcess(stdout: '''
-packages/plugin1/plugin1/plugin1.dart
-''')),
-      ];
       final Directory pluginGroup = packagesDir.childDirectory('plugin1');
       final RepositoryPackage appFacingPackage =
           createFakePlugin('plugin1', pluginGroup);
@@ -235,8 +230,7 @@ packages/plugin1/plugin1/plugin1.dart
       final RepositoryPackage implementationPackage =
           createFakePlugin('plugin1_web', pluginGroup);
 
-      await runCapturingPrint(
-          runner, <String>['sample', '--base-sha=main', '--packages=plugin1']);
+      await runCapturingPrint(runner, <String>['sample', '--packages=plugin1']);
 
       expect(
           command.plugins,
@@ -247,6 +241,21 @@ packages/plugin1/plugin1/plugin1.dart
           ]));
     });
 
+    test(
+        'specifying the app-facing package of a federated plugin with '
+        '--exact-match-only should only include only that package', () async {
+      final Directory pluginGroup = packagesDir.childDirectory('plugin1');
+      final RepositoryPackage appFacingPackage =
+          createFakePlugin('plugin1', pluginGroup);
+      createFakePlugin('plugin1_platform_interface', pluginGroup);
+      createFakePlugin('plugin1_web', pluginGroup);
+
+      await runCapturingPrint(runner,
+          <String>['sample', '--packages=plugin1', '--exact-match-only']);
+
+      expect(command.plugins, unorderedEquals(<String>[appFacingPackage.path]));
+    });
+
     test(
         'specifying the app-facing package of a federated plugin using its '
         'fully qualified name should include only that package', () async {
diff --git a/script/tool/test/publish_command_test.dart b/script/tool/test/publish_command_test.dart
