Repository navigation
find_commit.dart hardcodes master branch #115476
Description
When looking up git commits, find_commit.dart uses a git log invocation here:
flutter/dev/tools/bin/find_commit.dart
Lines 68 to 75 in a2233ea
| return git(secondaryRepoDirectory, <String>[ | |
| 'log', | |
| '--format=%H', | |
| '--until=${anchor.timestamp.toIso8601String()}', | |
| '--max-count=1', | |
| secondaryBranch, | |
| '--', | |
| ]); |
This is called from here:
flutter/dev/tools/bin/find_commit.dart
Lines 99 to 105 in a2233ea
| print(findCommit( | |
| primaryRepoDirectory: primaryRepo, | |
| primaryBranch: git(primaryRepo, <String>['rev-parse', '--abbrev-ref', 'HEAD']).trim(), | |
| primaryTrunk: 'master', | |
| secondaryRepoDirectory: secondaryRepo, | |
| secondaryBranch: 'master', | |
| ).trim()); |
Since we hardcode master as both the primary and secondary repo branches, it will fail for repos that have only a main branch and no master, such as flutter/cocoon.
See example failure here:
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8797370543676737825/+/u/Customer_testing/customer_testing/test_stderr
Related, but we should probably fail whichever tools are using find_commit.dart if the lookup fails. In the log above it looks like even though the lookup fails, we continue on and run the tests anyway:
git -C ../../bin/cache/pkg/tests checkout
dart --enable-asserts run_tests.dartNote the lack of SHA in the checkout command, which I presume is meant to be there.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsFixed
Activity
cbracken commentedon Nov 16, 2022
/cc @Hangyujin who spotted this in her patch's presubmits.
ricardoamador commentedon Nov 16, 2022
@HansMuller Can you reassign this please?
HansMuller commentedon Nov 18, 2022
@ricardoamador - not my area
@godofredoc - this issue is preventing us from landing an approved and time-critical PR
jmagman commentedon Nov 18, 2022
The find_commit logic is old, and hasn't cocoon been missing a master branch for awhile?
Are you sure the real failure isn't the new analysis issues coming from
flutter_cocoon?https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8797370543676737825/+/u/Customer_testing/customer_testing/test_stdout
When I run it locally on this PR I also see these, which definitely look related to #113894 (the PR where this is failing).
https://github.com/flutter/cocoon/blob/7f0c5689961be85c2d621df52d88c31a29732ee9/dashboard/lib/navigation_drawer.dart#L10
HansMuller commentedon Nov 18, 2022
Right and thanks for tracking down the root of the problem! The actual problem is a simple one. The original error logs, https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8797303295761259729/+/u/Customer_testing/customer_testing/test_stdout, completely obscure the problem:
XilaiZhang commentedon Feb 13, 2023
Regarding the
git logerror in find_commit.dart, we tried fixing it with #119648 . The fix worked in most of the scenarios, but would fail in certain situations (e.g. check runs of #119384) due to possible luci git behavior. The luci git behavior was discussed in https://chat.google.com/room/AAAAc_4rqiI/_k6gi9syHTY and the conclusion was that, we cannot rely on the output of git commands in luci to find out the current branch.It seems like the root cause of the failure was identified and solved in flutter/cocoon#2313 now?
3 remaining items
XilaiZhang commentedon Nov 27, 2023
Re-reading this thread. It seems like the proposed fix landed in #119648 but it is impossible to handle all scenarios (due to systematic error of luci git)?
Migrate customer_testing to sharded tests. (#138659)
Migrate customer_testing to sharded tests. (flutter#138659)
flutter-triage-bot commentedon Apr 1, 2024
This issue is assigned to @XilaiZhang but has had no recent status updates. Please consider unassigning this issue if it is not going to be addressed in the near future. This allows people to have a clearer picture of what work is actually planned. Thanks!
flutter-triage-bot commentedon May 23, 2024
This issue was assigned to @XilaiZhang but has had no status updates in a long time. To remove any ambiguity about whether the issue is being worked on, the assignee was removed.
github-actions commentedon Sep 6, 2024
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.