diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 34def6ecf676..3c4905ad7071 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.13.4+1 + +* Makes `--packages-for-branch` detect any commit on `main` as being `main`, + so that it works with pinned checkouts (e.g., on LUCI). + ## 0.13.4 * Adds the ability to validate minimum supported Dart/Flutter versions in diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index 0e83d03e9846..0e084c4c2d5c 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -316,17 +316,28 @@ abstract class PackageCommand extends Command { } else if (getBoolArg(_packagesForBranchArg)) { final String? branch = await _getBranch(); if (branch == null) { - printError('Unabled to determine branch; --$_packagesForBranchArg can ' + printError('Unable to determine branch; --$_packagesForBranchArg can ' 'only be used in a git repository.'); throw ToolExit(exitInvalidArguments); } else { // Configure the change finder the correct mode for the branch. - final bool lastCommitOnly = branch == 'main' || branch == 'master'; + // Log the mode to make it easier to audit logs to see that the + // intended diff was used (or why). + final bool lastCommitOnly; + if (branch == 'main' || branch == 'master') { + print('--$_packagesForBranchArg: running on default branch.'); + lastCommitOnly = true; + } else if (await _isCheckoutFromBranch('main')) { + print( + '--$_packagesForBranchArg: running on a commit from default branch.'); + lastCommitOnly = true; + } else { + print('--$_packagesForBranchArg: running on branch "$branch".'); + lastCommitOnly = false; + } if (lastCommitOnly) { - // Log the mode to make it easier to audit logs to see that the - // intended diff was used. - print('--$_packagesForBranchArg: running on default branch; ' - 'using parent commit as the diff base.'); + print( + '--$_packagesForBranchArg: using parent commit as the diff base.'); changedFileFinder = GitVersionFinder(await gitDir, 'HEAD~'); } else { changedFileFinder = await retrieveVersionFinder(); @@ -522,6 +533,17 @@ abstract class PackageCommand extends Command { return packages; } + // Returns true if the current checkout is on an ancestor of [branch]. + // + // This is used because CI may check out a specific hash rather than a branch, + // in which case branch-name detection won't work. + Future _isCheckoutFromBranch(String branchName) async { + final io.ProcessResult result = await (await gitDir).runCommand( + ['merge-base', '--is-ancestor', 'HEAD', branchName], + throwOnError: false); + return result.exitCode == 0; + } + Future _getBranch() async { final io.ProcessResult branchResult = await (await gitDir).runCommand( ['rev-parse', '--abbrev-ref', 'HEAD'], diff --git a/script/tool/pubspec.yaml b/script/tool/pubspec.yaml index a8df2a9cd23a..73429638a402 100644 --- a/script/tool/pubspec.yaml +++ b/script/tool/pubspec.yaml @@ -1,7 +1,7 @@ name: flutter_plugin_tools description: Productivity utils for flutter/plugins and flutter/packages repository: https://github.com/flutter/plugins/tree/main/script/tool -version: 0.13.4 +version: 0.13.4+1 dependencies: args: ^2.1.0 diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart index aa0a20253955..ed76408bb300 100644 --- a/script/tool/test/common/package_command_test.dart +++ b/script/tool/test/common/package_command_test.dart @@ -778,7 +778,8 @@ packages/b_package/lib/src/foo.dart MockProcess(stdout: 'a-branch'), ]; processRunner.mockProcessesForExecutable['git-merge-base'] = [ - MockProcess(stdout: 'abc123'), + MockProcess(exitCode: 1), // --is-ancestor check + MockProcess(stdout: 'abc123'), // finding merge base ]; final RepositoryPackage plugin1 = createFakePlugin('plugin1', packagesDir); @@ -791,6 +792,7 @@ packages/b_package/lib/src/foo.dart expect( output, containsAllInOrder([ + contains('--packages-for-branch: running on branch "a-branch"'), contains( 'Running for all packages that have diffs relative to "abc123"'), ])); @@ -822,8 +824,9 @@ packages/b_package/lib/src/foo.dart expect( output, containsAllInOrder([ - contains('--packages-for-branch: running on default branch; ' - 'using parent commit as the diff base'), + contains('--packages-for-branch: running on default branch.'), + contains( + '--packages-for-branch: using parent commit as the diff base'), contains( 'Running for all packages that have diffs relative to "HEAD~"'), ])); @@ -836,7 +839,45 @@ packages/b_package/lib/src/foo.dart )); }); - test('tests all packages on master', () async { + test( + 'only tests changed packages relative to the previous commit if ' + 'running on a specific hash from main', () async { + processRunner.mockProcessesForExecutable['git-diff'] = [ + MockProcess(stdout: 'packages/plugin1/plugin1.dart'), + ]; + processRunner.mockProcessesForExecutable['git-rev-parse'] = [ + MockProcess(stdout: 'HEAD'), + ]; + final RepositoryPackage plugin1 = + createFakePlugin('plugin1', packagesDir); + createFakePlugin('plugin2', packagesDir); + + final List output = await runCapturingPrint( + runner, ['sample', '--packages-for-branch']); + + expect(command.plugins, unorderedEquals([plugin1.path])); + expect( + output, + containsAllInOrder([ + contains( + '--packages-for-branch: running on a commit from default branch.'), + contains( + '--packages-for-branch: using parent commit as the diff base'), + contains( + 'Running for all packages that have diffs relative to "HEAD~"'), + ])); + // Ensure that it's diffing against the prior commit. + expect( + processRunner.recordedCalls, + contains( + const ProcessCall( + 'git-diff', ['--name-only', 'HEAD~', 'HEAD'], null), + )); + }); + + test( + 'only tests changed packages relative to the previous commit on master', + () async { processRunner.mockProcessesForExecutable['git-diff'] = [ MockProcess(stdout: 'packages/plugin1/plugin1.dart'), ]; @@ -854,8 +895,9 @@ packages/b_package/lib/src/foo.dart expect( output, containsAllInOrder([ - contains('--packages-for-branch: running on default branch; ' - 'using parent commit as the diff base'), + contains('--packages-for-branch: running on default branch.'), + contains( + '--packages-for-branch: using parent commit as the diff base'), contains( 'Running for all packages that have diffs relative to "HEAD~"'), ])); @@ -887,7 +929,7 @@ packages/b_package/lib/src/foo.dart expect( output, containsAllInOrder([ - contains('Unabled to determine branch'), + contains('Unable to determine branch'), ])); }); });