Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit 3d81a00

Browse files
[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
1 parent 8fcff87 commit 3d81a00

File tree

3 files changed

+63
-5
lines changed

3 files changed

+63
-5
lines changed

script/tool/lib/src/common/package_command.dart

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -538,10 +538,28 @@ abstract class PackageCommand extends Command<void> {
538538
// This is used because CI may check out a specific hash rather than a branch,
539539
// in which case branch-name detection won't work.
540540
Future<bool> _isCheckoutFromBranch(String branchName) async {
541-
final io.ProcessResult result = await (await gitDir).runCommand(
542-
<String>['merge-base', '--is-ancestor', 'HEAD', branchName],
543-
throwOnError: false);
544-
return result.exitCode == 0;
541+
// The target branch may not exist locally; try some common remote names for
542+
// the branch as well.
543+
final List<String> candidateBranchNames = <String>[
544+
branchName,
545+
'origin/$branchName',
546+
'upstream/$branchName',
547+
];
548+
for (final String branch in candidateBranchNames) {
549+
final io.ProcessResult result = await (await gitDir).runCommand(
550+
<String>['merge-base', '--is-ancestor', 'HEAD', branch],
551+
throwOnError: false);
552+
if (result.exitCode == 0) {
553+
return true;
554+
} else if (result.exitCode == 1) {
555+
// 1 indicates that the branch was successfully checked, but it's not
556+
// an ancestor.
557+
return false;
558+
}
559+
// Any other return code is an error, such as `branch` not being a valid
560+
// name in the repository, so try other name variants.
561+
}
562+
return false;
545563
}
546564

547565
Future<String?> _getBranch() async {

script/tool/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: flutter_plugin_tools
22
description: Productivity utils for flutter/plugins and flutter/packages
33
repository: https://github.com/flutter/plugins/tree/main/script/tool
4-
version: 0.13.4+1
4+
version: 0.13.4+2
55

66
dependencies:
77
args: ^2.1.0

script/tool/test/common/package_command_test.dart

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,46 @@ packages/b_package/lib/src/foo.dart
875875
));
876876
});
877877

878+
test(
879+
'only tests changed packages relative to the previous commit if '
880+
'running on a specific hash from origin/main', () async {
881+
processRunner.mockProcessesForExecutable['git-diff'] = <Process>[
882+
MockProcess(stdout: 'packages/plugin1/plugin1.dart'),
883+
];
884+
processRunner.mockProcessesForExecutable['git-rev-parse'] = <Process>[
885+
MockProcess(stdout: 'HEAD'),
886+
];
887+
processRunner.mockProcessesForExecutable['git-merge-base'] = <Process>[
888+
MockProcess(exitCode: 128), // Fail with a non-1 exit code for 'main'
889+
MockProcess(), // Succeed for the variant.
890+
];
891+
final RepositoryPackage plugin1 =
892+
createFakePlugin('plugin1', packagesDir);
893+
createFakePlugin('plugin2', packagesDir);
894+
895+
final List<String> output = await runCapturingPrint(
896+
runner, <String>['sample', '--packages-for-branch']);
897+
898+
expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
899+
expect(
900+
output,
901+
containsAllInOrder(<Matcher>[
902+
contains(
903+
'--packages-for-branch: running on a commit from default branch.'),
904+
contains(
905+
'--packages-for-branch: using parent commit as the diff base'),
906+
contains(
907+
'Running for all packages that have diffs relative to "HEAD~"'),
908+
]));
909+
// Ensure that it's diffing against the prior commit.
910+
expect(
911+
processRunner.recordedCalls,
912+
contains(
913+
const ProcessCall(
914+
'git-diff', <String>['--name-only', 'HEAD~', 'HEAD'], null),
915+
));
916+
});
917+
878918
test(
879919
'only tests changed packages relative to the previous commit on master',
880920
() async {

0 commit comments

Comments
 (0)