[tool] More main-branch detection improvement (#7067)

Follow-up to #7038 to try to make it work on LUCI. Looking at the checkout steps of a LUCI run, it looks like we do a full `fetch` of `origin`, but likely only have a `master` branch locally. Rather than rely on a specific local branch name existing, this allows for checking `origin` (and just in case, since it's another common remote name, `upstream`). Hopefully fixes flutter/flutter#119330
flutter · Feb 1, 2023 · 3d81a00 · 3d81a00
1 parent 8fcff87
commit 3d81a00
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 5 deletions.
diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart
@@ -538,10 +538,28 @@ abstract class PackageCommand extends Command<void> {
   // 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;
+    // The target branch may not exist locally; try some common remote names for
+    // the branch as well.
+    final List<String> candidateBranchNames = <String>[
+      branchName,
+      'origin/$branchName',
+      'upstream/$branchName',
+    ];
+    for (final String branch in candidateBranchNames) {
+      final io.ProcessResult result = await (await gitDir).runCommand(
+          <String>['merge-base', '--is-ancestor', 'HEAD', branch],
+          throwOnError: false);
+      if (result.exitCode == 0) {
+        return true;
+      } else if (result.exitCode == 1) {
+        // 1 indicates that the branch was successfully checked, but it's not
+        // an ancestor.
+        return false;
+      }
+      // Any other return code is an error, such as `branch` not being a valid
+      // name in the repository, so try other name variants.
+    }
+    return false;
   }
 
   Future<String?> _getBranch() async {

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+1
+version: 0.13.4+2
 
 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
@@ -875,6 +875,46 @@ packages/b_package/lib/src/foo.dart
           ));
     });
 
+    test(
+        'only tests changed packages relative to the previous commit if '
+        'running on a specific hash from origin/main', () async {
+      processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
+        MockProcess(stdout: 'packages/plugin1/plugin1.dart'),
+      ];
+      processRunner.mockProcessesForExecutable['git-rev-parse'] = <Process>[
+        MockProcess(stdout: 'HEAD'),
+      ];
+      processRunner.mockProcessesForExecutable['git-merge-base'] = <Process>[
+        MockProcess(exitCode: 128), // Fail with a non-1 exit code for 'main'
+        MockProcess(), // Succeed for the variant.
+      ];
+      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 {