find_commit.dart hardcodes master branch

[cbracken]

When looking up git commits, find_commit.dart uses a git log invocation here: https://github.com/flutter/flutter/blob/a2233eabed2283c473dfe5c05a21fd555428541e/dev/tools/bin/find_commit.dart#L68-L75

This is called from here: https://github.com/flutter/flutter/blob/a2233eabed2283c473dfe5c05a21fd555428541e/dev/tools/bin/find_commit.dart#L99-L105

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.dart

Note the lack of SHA in the checkout command, which I presume is meant to be there.

[cbracken]

/cc @hangyujin who spotted this in her patch's presubmits.

[ricardoamador]

@HansMuller Can you reassign this please?

[HansMuller]

@ricardoamador - not my area

@godofredoc - this issue is preventing us from landing an approved and time-critical PR

[jmagman]

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?

|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • lib/build_dashboard_page.dart:436:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • lib/build_dashboard_page.dart:436:23 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • lib/index_page.dart:60:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • lib/index_page.dart:60:23 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:30:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:30:17 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:48:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:48:23 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:71:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:71:17 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:87:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:87:17 • missing_required_argument

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 https://github.com/flutter/flutter/pull/113894 (the PR where this is failing).

   info • The import of 'navigation_drawer.dart' is unnecessary because all of the used elements are also provided by the import of
          'package:flutter/material.dart' • lib/build_dashboard_page.dart:12:8 • unnecessary_import
  error • The name 'NavigationDrawer' is defined in the libraries 'package:flutter/src/material/navigation_drawer.dart (via package:flutter/material.dart)'
         and 'package:flutter_dashboard/navigation_drawer.dart' • lib/build_dashboard_page.dart:437:23 • ambiguous_import
   info • The import of 'navigation_drawer.dart' is unnecessary because all of the used elements are also provided by the import of
          'package:flutter/material.dart' • lib/index_page.dart:9:8 • unnecessary_import
  error • The name 'NavigationDrawer' is defined in the libraries 'package:flutter/src/material/navigation_drawer.dart (via package:flutter/material.dart)'
         and 'package:flutter_dashboard/navigation_drawer.dart' • lib/index_page.dart:61:23 • ambiguous_import
   info • The import of 'package:flutter_dashboard/navigation_drawer.dart' is unnecessary because all of the used elements are also provided by the import
          of 'package:flutter/material.dart' • test/navigation_drawer_test.dart:7:8 • unnecessary_import
  error • 'NavigationDrawer' isn't a function • test/navigation_drawer_test.dart:31:17 • invocation_of_non_function
  error • Invalid constant value • test/navigation_drawer_test.dart:31:17 • invalid_constant
  error • The name 'NavigationDrawer' is defined in the libraries 'package:flutter/src/material/navigation_drawer.dart (via package:flutter/material.dart)'
         and 'package:flutter_dashboard/navigation_drawer.dart' • test/navigation_drawer_test.dart:31:17 • ambiguous_import
  error • The name 'NavigationDrawer' is defined in the libraries 'package:flutter/src/material/navigation_drawer.dart (via package:flutter/material.dart)'
         and 'package:flutter_dashboard/navigation_drawer.dart' • test/navigation_drawer_test.dart:49:23 • ambiguous_import
  error • 'NavigationDrawer' isn't a function • test/navigation_drawer_test.dart:72:17 • invocation_of_non_function
  error • Invalid constant value • test/navigation_drawer_test.dart:72:17 • invalid_constant
  error • The name 'NavigationDrawer' is defined in the libraries 'package:flutter/src/material/navigation_drawer.dart (via package:flutter/material.dart)'
         and 'package:flutter_dashboard/navigation_drawer.dart' • test/navigation_drawer_test.dart:72:17 • ambiguous_import
  error • 'NavigationDrawer' isn't a function • test/navigation_drawer_test.dart:88:17 • invocation_of_non_function
  error • Invalid constant value • test/navigation_drawer_test.dart:88:17 • invalid_constant
  error • The name 'NavigationDrawer' is defined in the libraries 'package:flutter/src/material/navigation_drawer.dart (via package:flutter/material.dart)'
         and 'package:flutter_dashboard/navigation_drawer.dart' • test/navigation_drawer_test.dart:88:17 • ambiguous_import

https://github.com/flutter/cocoon/blob/7f0c5689961be85c2d621df52d88c31a29732ee9/dashboard/lib/navigation_drawer.dart#L10

[HansMuller]

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:

|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • lib/build_dashboard_page.dart:436:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • lib/build_dashboard_page.dart:436:23 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • lib/index_page.dart:60:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • lib/index_page.dart:60:23 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:30:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:30:17 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:48:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:48:23 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:71:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:71:17 • missing_required_argument
|   error • A value of type 'Null' can't be assigned to a parameter of type 'List<Widget>' in a const constructor • test/navigation_drawer_test.dart:87:17 • const_constructor_param_type_mismatch
|   error • The named parameter 'children' is required, but there's no corresponding argument • test/navigation_drawer_test.dart:87:17 • missing_required_argument

[XilaiZhang]

Regarding the git log error in find_commit.dart, we tried fixing it with https://github.com/flutter/flutter/pull/119648 . The fix worked in most of the scenarios, but would fail in certain situations (e.g. check runs of https://github.com/flutter/flutter/pull/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 https://github.com/flutter/cocoon/pull/2313 now?

[flutter-triage-bot[bot]]

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!

[ricardoamador]

@XilaiZhang is this still happening?

[XilaiZhang]

Re-reading this thread. It seems like the proposed fix landed in https://github.com/flutter/flutter/pull/119648 but it is impossible to handle all scenarios (due to systematic error of luci git)?

[flutter-triage-bot[bot]]

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[bot]]

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.