diff --git a/script/tool/README.md b/script/tool/README.md index 0de5529bea57..4265fb484906 100644 --- a/script/tool/README.md +++ b/script/tool/README.md @@ -117,6 +117,7 @@ code changes across many packages: cd dart run script/tool/bin/flutter_plugin_tools.dart update-release-info \ --version=minimal \ + --base-branch=upstream/main \ --changelog="Fixes violations of new analysis option some_new_option." ``` @@ -124,10 +125,16 @@ The `minimal` option for `--version` will skip unchanged packages, and treat each changed package as either `bugfix` or `next` depending on the files that have changed in that package, so it is often the best choice for a bulk change. -For cases where you know the change time, `minor` or `bugfix` will make the +For cases where you know the change type, `minor` or `bugfix` will make the corresponding version bump, or `next` will update only `CHANGELOG.md` without changing the version. +If you have a standard repository setup, `--base-branch=upstream/main` will +usually give the behavior you want, finding all packages changed relative to +the branch point from `upstream/main`. For more complex use cases where you want +a different diff point, you can pass a different `--base-branch`, or use +`--base-sha` to pick the exact diff point. + ### Publish a Release **Releases are automated for `flutter/packages`.** diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart index 3965ae0ace47..0956e3ca9ec3 100644 --- a/script/tool/lib/src/common/git_version_finder.dart +++ b/script/tool/lib/src/common/git_version_finder.dart @@ -11,7 +11,11 @@ import 'package:yaml/yaml.dart'; /// Finding diffs based on `baseGitDir` and `baseSha`. class GitVersionFinder { /// Constructor - GitVersionFinder(this.baseGitDir, String? baseSha) : _baseSha = baseSha; + GitVersionFinder(this.baseGitDir, {String? baseSha, String? baseBranch}) + : assert(baseSha == null || baseBranch == null, + 'At most one of baseSha and baseBranch can be provided'), + _baseSha = baseSha, + _baseBranch = baseBranch ?? 'FETCH_HEAD'; /// The top level directory of the git repo. /// @@ -21,6 +25,9 @@ class GitVersionFinder { /// The base sha used to get diff. String? _baseSha; + /// The base branche used to find a merge point if baseSha is not provided. + final String _baseBranch; + static bool _isPubspec(String file) { return file.trim().endsWith('pubspec.yaml'); } @@ -101,13 +108,13 @@ class GitVersionFinder { } io.ProcessResult baseShaFromMergeBase = await baseGitDir.runCommand( - ['merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'], + ['merge-base', '--fork-point', _baseBranch, 'HEAD'], throwOnError: false); final String stdout = (baseShaFromMergeBase.stdout as String? ?? '').trim(); final String stderr = (baseShaFromMergeBase.stderr as String? ?? '').trim(); if (stderr.isNotEmpty || stdout.isEmpty) { baseShaFromMergeBase = await baseGitDir - .runCommand(['merge-base', 'FETCH_HEAD', 'HEAD']); + .runCommand(['merge-base', _baseBranch, 'HEAD']); } baseSha = (baseShaFromMergeBase.stdout as String).trim(); _baseSha = baseSha; diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index 8a2bbfc40058..9b0006123c47 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -94,12 +94,17 @@ abstract class PackageCommand extends Command { help: 'The base sha used to determine git diff. \n' 'This is useful when $_runOnChangedPackagesArg is specified.\n' 'If not specified, merge-base is used as base sha.'); + argParser.addOption(_baseBranchArg, + help: 'The base branch whose merge base is used as the base SHA if ' + '--$_baseShaArg is not provided. \n' + 'If not specified, FETCH_HEAD is used as the base branch.'); argParser.addFlag(_logTimingArg, help: 'Logs timing information.\n\n' 'Currently only logs per-package timing for multi-package commands, ' 'but more information may be added in the future.'); } + static const String _baseBranchArg = 'base-branch'; static const String _baseShaArg = 'base-sha'; static const String _excludeArg = 'exclude'; static const String _logTimingArg = 'log-timing'; @@ -188,6 +193,11 @@ abstract class PackageCommand extends Command { return (argResults![key] as String?) ?? ''; } + /// Convenience accessor for String arguments. + String? getNullableStringArg(String key) { + return argResults![key] as String?; + } + /// Convenience accessor for List arguments. List getStringListArg(String key) { // Clone the list so that if a caller modifies the result it won't change @@ -338,7 +348,7 @@ abstract class PackageCommand extends Command { if (lastCommitOnly) { print( '--$_packagesForBranchArg: using parent commit as the diff base.'); - changedFileFinder = GitVersionFinder(await gitDir, 'HEAD~'); + changedFileFinder = GitVersionFinder(await gitDir, baseSha: 'HEAD~'); } else { changedFileFinder = await retrieveVersionFinder(); } @@ -361,7 +371,7 @@ abstract class PackageCommand extends Command { } } else if (getBoolArg(_runOnDirtyPackagesArg)) { final GitVersionFinder gitVersionFinder = - GitVersionFinder(await gitDir, 'HEAD'); + GitVersionFinder(await gitDir, baseSha: 'HEAD'); print('Running for all packages that have uncommitted changes\n'); // _changesRequireFullTest is deliberately not used here, as this flag is // intended for use in CI to re-test packages changed by @@ -473,10 +483,12 @@ abstract class PackageCommand extends Command { /// /// Throws tool exit if [gitDir] nor root directory is a git directory. Future retrieveVersionFinder() async { - final String baseSha = getStringArg(_baseShaArg); + final String? baseSha = getNullableStringArg(_baseShaArg); + final String? baseBranch = + baseSha == null ? getNullableStringArg(_baseBranchArg) : null; - final GitVersionFinder gitVersionFinder = - GitVersionFinder(await gitDir, baseSha); + final GitVersionFinder gitVersionFinder = GitVersionFinder(await gitDir, + baseSha: baseSha, baseBranch: baseBranch); return gitVersionFinder; } diff --git a/script/tool/test/common/git_version_finder_test.dart b/script/tool/test/common/git_version_finder_test.dart index 538b72a90021..b0021b64f841 100644 --- a/script/tool/test/common/git_version_finder_test.dart +++ b/script/tool/test/common/git_version_finder_test.dart @@ -38,7 +38,8 @@ void main() { }); test('No git diff should result no files changed', () async { - final GitVersionFinder finder = GitVersionFinder(gitDir, 'some base sha'); + final GitVersionFinder finder = + GitVersionFinder(gitDir, baseSha: 'some base sha'); final List changedFiles = await finder.getChangedFiles(); expect(changedFiles, isEmpty); @@ -49,7 +50,8 @@ void main() { file1/file1.cc file2/file2.cc '''; - final GitVersionFinder finder = GitVersionFinder(gitDir, 'some base sha'); + final GitVersionFinder finder = + GitVersionFinder(gitDir, baseSha: 'some base sha'); final List changedFiles = await finder.getChangedFiles(); expect(changedFiles, equals(['file1/file1.cc', 'file2/file2.cc'])); @@ -60,7 +62,8 @@ file2/file2.cc file1/pubspec.yaml file2/file2.cc '''; - final GitVersionFinder finder = GitVersionFinder(gitDir, 'some base sha'); + final GitVersionFinder finder = + GitVersionFinder(gitDir, baseSha: 'some base sha'); final List changedFiles = await finder.getChangedPubSpecs(); expect(changedFiles, equals(['file1/pubspec.yaml'])); @@ -73,8 +76,28 @@ file1/pubspec.yaml file2/file2.cc '''; - final GitVersionFinder finder = GitVersionFinder(gitDir, null); + final GitVersionFinder finder = GitVersionFinder(gitDir); await finder.getChangedFiles(); + verify(gitDir.runCommand( + ['merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'], + throwOnError: false)); + verify(gitDir.runCommand( + ['diff', '--name-only', mergeBaseResponse, 'HEAD'])); + }); + + test('uses correct base branch to find base sha if specified', () async { + mergeBaseResponse = 'shaqwiueroaaidf12312jnadf123nd'; + gitDiffResponse = ''' +file1/pubspec.yaml +file2/file2.cc +'''; + + final GitVersionFinder finder = + GitVersionFinder(gitDir, baseBranch: 'upstream/main'); + await finder.getChangedFiles(); + verify(gitDir.runCommand( + ['merge-base', '--fork-point', 'upstream/main', 'HEAD'], + throwOnError: false)); verify(gitDir.runCommand( ['diff', '--name-only', mergeBaseResponse, 'HEAD'])); }); @@ -85,7 +108,8 @@ file2/file2.cc file1/pubspec.yaml file2/file2.cc '''; - final GitVersionFinder finder = GitVersionFinder(gitDir, customBaseSha); + final GitVersionFinder finder = + GitVersionFinder(gitDir, baseSha: customBaseSha); await finder.getChangedFiles(); verify(gitDir .runCommand(['diff', '--name-only', customBaseSha, 'HEAD'])); @@ -97,7 +121,8 @@ file2/file2.cc file1/pubspec.yaml file2/file2.cc '''; - final GitVersionFinder finder = GitVersionFinder(gitDir, customBaseSha); + final GitVersionFinder finder = + GitVersionFinder(gitDir, baseSha: customBaseSha); await finder.getChangedFiles(includeUncommitted: true); // The call should not have HEAD as a final argument like the default diff. verify(gitDir.runCommand(['diff', '--name-only', customBaseSha]));