flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
161.86k stars 26.57k forks source link

Clang tidy output in the engine has regressed, making debugging difficult #145926

Closed matanlurey closed 1 month ago

matanlurey commented 1 month ago

From @dkwingsmt on an internal chat:

For some reason our engine CI no longer prints the actual error of the clang tidy check, but a bunch of useless logs, such as in https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8752198650857229249/+/u/test:_test:_lint_host_debug/stdout?format=raw Is it a known issue?

It is now! Definitely a regression, we need to fix this.

matanlurey commented 1 month ago

When I run this locally, it appears kosher:

% drt ./tools/clang_tidy/bin/main.dart --lint-regex="common\/graphics.*"
┌──────────────────────────┐
│ Engine Clang Tidy Linter │
└──────────────────────────┘
The following errors have been reported by the Engine Clang Tidy Linter.  For
more information on addressing these issues please see:
https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter

🔶 linting flutter/common/graphics/gl_context_switch.cc
🔶 linting flutter/common/graphics/persistent_cache.cc
🔶 linting flutter/common/graphics/texture.cc
[0:02] Jobs:  33% done,   1/3   completed,  2 in progress,   0 pending,   0 failed.    
[0:02] Still running: [clang-tidy on /Users/matanl/Developer/engine/src/flutter/common/graphics/persistent_cache.cc, clang-tidy on /Users/matanl/Developer/engine/src/flutter/common/graphics/texture.cc]
[0:04] Jobs:  66% done,   2/3   completed,  1 in progress,   0 pending,   0 failed.    
[0:04] Still running: [clang-tidy on /Users/matanl/Developer/engine/src/flutter/common/graphics/persistent_cache.cc]
[0:08] Jobs: 100% done,   3/3   completed,  0 in progress,   0 pending,   0 failed.    

No lint problems found.
matanlurey commented 1 month ago

I intentionally made a lint issue in a file and ran ./ci/clang_tidy.sh:

% ./ci/clang_tidy.sh 
12:59:42 Running clang_tidy
┌──────────────────────────┐
│ Engine Clang Tidy Linter │
└──────────────────────────┘
The following errors have been reported by the Engine Clang Tidy Linter.  For
more information on addressing these issues please see:
https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter

🔶 linting flutter/shell/platform/embedder/embedder_external_view_embedder.cc
🔶 linting flutter/shell/platform/embedder/embedder_external_view_embedder.cc
[0:09] Jobs:  50% done,   0/2   completed,  1 in progress,   0 pending,   1 failed.    
❌ Failures for clang-tidy on /Users/matanl/Developer/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc:
/Users/matanl/Developer/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc:309:39: error: the const qualified parameter 'target_provider' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param,-warnings-as-errors]
  309 |           FlutterBackingStoreConfig)> target_provider) {
      |                                       ^
      |                                      &
Suppressed 2921 warnings (2891 in non-user code, 30 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
[0:09] Jobs: 100% done,   0/2   completed,  0 in progress,   0 pending,   2 failed.    
❌ Failures for clang-tidy on /Users/matanl/Developer/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc:
/Users/matanl/Developer/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc:309:39: error: the const qualified parameter 'target_provider' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param,-warnings-as-errors]
  309 |           FlutterBackingStoreConfig)> target_provider) {
      |                                       ^
      |                                      &
Suppressed 2921 warnings (2891 in non-user code, 30 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error

Lint problems found.

Again, it looks fine.

matanlurey commented 1 month ago

Ah, here is the problem, the CI configuration turned on --verbose probably for debugging and was never turned off (in @zanderso's https://github.com/flutter/engine/pull/51001). I will turn this off.

github-actions[bot] commented 2 weeks ago

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 -v and a minimal reproduction of the issue.