flutter · auto-submit · Jan 30, 2023 · Jan 27, 2023 · Jan 27, 2023 · Jan 30, 2023
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 0.13.4+1
+
+* Makes `--packages-for-branch` detect any commin 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
@@ -316,17 +316,28 @@ abstract class PackageCommand extends Command<void> {
     } 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,18 @@ abstract class PackageCommand extends Command<void> {
     return packages;
   }
 
+  // Retruns true if the environment indicates that the current treeish is from
+  // [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<bool> _isCheckoutFromBranch(String branchName) async {
+    final io.ProcessResult result = await (await gitDir).runCommand(
+        <String>['merge-base', '--is-ancestor', 'HEAD', branchName],
+        throwOnError: false);
+    return result.exitCode == 0;
+  }
+
   Future<String?> _getBranch() async {
     final io.ProcessResult branchResult = await (await gitDir).runCommand(
         <String>['rev-parse', '--abbrev-ref', 'HEAD'],

diff --git 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
@@ -778,7 +778,8 @@ packages/b_package/lib/src/foo.dart
         MockProcess(stdout: 'a-branch'),
       ];
       processRunner.mockProcessesForExecutable['git-merge-base'] = <Process>[
-        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(<Matcher>[
+            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(<Matcher>[
-            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'] = <Process>[
+        MockProcess(stdout: 'packages/plugin1/plugin1.dart'),
+      ];
+      processRunner.mockProcessesForExecutable['git-rev-parse'] = <Process>[
+        MockProcess(stdout: 'HEAD'),
+      ];
+      final RepositoryPackage plugin1 =
+          createFakePlugin('plugin1', packagesDir);
+      createFakePlugin('plugin2', packagesDir);
+
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['sample', '--packages-for-branch']);
+
+      expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
+      expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            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', <String>['--name-only', 'HEAD~', 'HEAD'], null),
+          ));
+    });
+
+    test(
+        'only tests changed packages relative to the previous commit on master',
+        () async {
       processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
         MockProcess(stdout: 'packages/plugin1/plugin1.dart'),
       ];
@@ -854,8 +895,9 @@ packages/b_package/lib/src/foo.dart
       expect(
           output,
           containsAllInOrder(<Matcher>[
-            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(<Matcher>[
-            contains('Unabled to determine branch'),
+            contains('Unable to determine branch'),
           ]));
     });
   });