diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index f51c011ec03..94a808afc17 100644 --- 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 { 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: [_pluginsLegacyAliasArg], ); @@ -67,6 +71,9 @@ abstract class PackageCommand extends Command { 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 { 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 { 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 packages = Set.from(getStringListArg(_packagesArg)); final GitVersionFinder? changedFileFinder; @@ -458,6 +475,30 @@ abstract class PackageCommand extends Command { 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 _everyTopLevelPackage() async* { for (final Directory dir in [ packagesDir, if (thirdPartyPackagesDir.existsSync()) thirdPartyPackagesDir, @@ -466,33 +507,14 @@ abstract class PackageCommand extends Command { 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({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 possibleMatches = { - 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); } } } @@ -500,6 +522,29 @@ abstract class PackageCommand extends Command { } } + Set _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 { + packageName, + path.relative(package.path, + from: parentDir.parent.path), // fully specified + if (allowGroup) path.basename(parentDir.path), // group name + }; + } else { + return {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 index eedea90e737..b678e36b7ab 100644 --- 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(['uncommitted changes']); } - if (!await _publish(package)) { - return PackageResult.fail(['publish failed']); + final bool tagOnly = getBoolArg(_tagForAutoPublishFlag); + if (!tagOnly) { + if (!await _publish(package)) { + return PackageResult.fail(['publish failed']); + } } - if (!await _tagRelease(package)) { - return PackageResult.fail(['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(['missing tag']); + } + } else { + if (!await _tagRelease(package, tag)) { + return PackageResult.fail(['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 -v, and push it to the remote. // // Return `true` if successful, `false` otherwise. - Future _tagRelease(RepositoryPackage package) async { - final String tag = _getTag(package); + Future _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> _getCurrentTags() async { + // git tag --points-at HEAD + final io.ProcessResult tagsResult = await (await gitDir).runCommand( + ['tag', '--points-at', 'HEAD'], + throwOnError: false, + ); + if (tagsResult.exitCode != 0) { + return []; + } + + return (tagsResult.stdout as String) + .split('\n') + .map((String line) => line.trim()) + .where((String line) => line.isNotEmpty); + } + Future _checkGitStatus(RepositoryPackage package) async { final io.ProcessResult statusResult = await (await gitDir).runCommand( [ diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart index a71fc2a8eac..f80db4181d3 100644 --- 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(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, ['sample', '--base-sha=main', '--packages=plugin1']); + await runCapturingPrint(runner, ['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, + ['sample', '--packages=plugin1', '--exact-match-only']); + + expect(command.plugins, unorderedEquals([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 index 3af74a1607b..1f3c0958b92 100644 --- a/script/tool/test/publish_command_test.dart +++ b/script/tool/test/publish_command_test.dart @@ -349,6 +349,33 @@ void main() { ), ); }); + + test('skips publish with --tag-for-auto-publish', () async { + const String packageName = 'a_package'; + createFakePackage(packageName, packagesDir); + + final List output = + await runCapturingPrint(commandRunner, [ + 'publish', + '--packages=$packageName', + '--tag-for-auto-publish', + ]); + + // There should be no variant of any command containing "publish". + expect( + processRunner.recordedCalls + .map((ProcessCall call) => call.toString()), + isNot(contains(contains('publish')))); + // The output should indicate that it was tagged, not published. + expect( + output, + containsAllInOrder( + [ + contains('Tagged a_package successfully!'), + ], + ), + ); + }); }); group('Tags release', () { @@ -390,6 +417,18 @@ void main() { isNot(contains( const ProcessCall('git-tag', ['foo-v0.0.1'], null)))); }); + + test('when passed --tag-for-auto-publish', () async { + createFakePlugin('foo', packagesDir, examples: []); + await runCapturingPrint(commandRunner, [ + 'publish', + '--packages=foo', + '--tag-for-auto-publish', + ]); + + expect(processRunner.recordedCalls, + contains(const ProcessCall('git-tag', ['foo-v0.0.1'], null))); + }); }); group('Pushes tags', () { @@ -439,6 +478,21 @@ void main() { ])); }); + test('when passed --tag-for-auto-publish', () async { + createFakePlugin('foo', packagesDir, examples: []); + await runCapturingPrint(commandRunner, [ + 'publish', + '--packages=foo', + '--skip-confirmation', + '--tag-for-auto-publish', + ]); + + expect( + processRunner.recordedCalls, + contains(const ProcessCall( + 'git-push', ['upstream', 'foo-v0.0.1'], null))); + }); + test('to upstream by default, dry run', () async { final RepositoryPackage plugin = createFakePlugin('foo', packagesDir, examples: []); @@ -488,6 +542,62 @@ void main() { }); }); + group('--already-tagged', () { + test('passes when HEAD has the expected tag', () async { + createFakePlugin('foo', packagesDir, examples: []); + + processRunner.mockProcessesForExecutable['git-tag'] = [ + FakeProcessInfo(MockProcess()), // Skip the initializeRun call. + FakeProcessInfo(MockProcess(stdout: 'foo-v0.0.1\n'), + ['--points-at', 'HEAD']) + ]; + + await runCapturingPrint(commandRunner, + ['publish', '--packages=foo', '--already-tagged']); + }); + + test('fails if HEAD does not have the expected tag', () async { + createFakePlugin('foo', packagesDir, examples: []); + + Error? commandError; + final List output = await runCapturingPrint(commandRunner, + ['publish', '--packages=foo', '--already-tagged'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('The current checkout is not already tagged "foo-v0.0.1"'), + contains('missing tag'), + ])); + }); + + test('does not create or push tags', () async { + createFakePlugin('foo', packagesDir, examples: []); + + processRunner.mockProcessesForExecutable['git-tag'] = [ + FakeProcessInfo(MockProcess()), // Skip the initializeRun call. + FakeProcessInfo(MockProcess(stdout: 'foo-v0.0.1\n'), + ['--points-at', 'HEAD']) + ]; + + await runCapturingPrint(commandRunner, + ['publish', '--packages=foo', '--already-tagged']); + + expect( + processRunner.recordedCalls, + isNot(contains( + const ProcessCall('git-tag', ['foo-v0.0.1'], null)))); + expect( + processRunner.recordedCalls + .map((ProcessCall call) => call.executable), + isNot(contains('git-push'))); + }); + }); + group('Auto release (all-changed flag)', () { test('can release newly created plugins', () async { mockHttpResponses['plugin1'] = {