search

erenon / bazel_clang_tidy

Run clang-tidy on Bazel C++ targets directly, efficiently, with caching enabled
MIT License
96 stars 51 forks source link

bazel_clang_tidy in a GitHub CI workflow #54

Closed eguiraud closed 6 months ago

eguiraud commented 7 months ago

Hello,

thank you for making this! I have a github CI/actions/workflow thing basically runs bazel build --config=clang-tidy -- //..., with the clang-tidy config being set up as per your README.

The problem: even if clang-tidy emits warnings, the step is green and successful: the above bazel build command returns exit code 0 even in case of warnings.

How would you set up a GitHub CI workflow that runs bazel_clang_tidy and errors out in case of clang-tidy warnings? Do you have an example somewhere maybe?

Thank you very much!

erenon commented 7 months ago

hi, do you have warnings or errors? I suspect clang exits with non-zero only if there are errors (not for warnings). Try 

WarningsAsErrors: "*"

in your config.

eguiraud commented 7 months ago

Hi, thank you for the quick reply. I have warnings. Promoting all warnings to errors seems problematic (files that were green before, now show errors):

$ clang-tidy ./util/file_io/file_io.cc
37543 warnings generated.
Suppressed 37543 warnings (37540 in non-user code, 3 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
$ # all good
$ clang-tidy --warnings-as-errors='*' ./util/file_io/file_io.cc
37543 warnings generated.
error: redefining builtin macro [clang-diagnostic-builtin-macro-redefined,-warnings-as-errors]
Suppressed 37540 warnings (37540 in non-user code).
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
$ # non-zero exit code on a file that generated no warnings before

Is this how people use bazel_clang_tidy in GitHub workflows?

erenon commented 7 months ago

Hi, yes, warnings as errors is recommended, similarly as for the compiler. See example: https://github.com/erenon/bazel_clang_tidy/blob/master/.clang-tidy

eguiraud commented 7 months ago

Alright, so I need to understand why setting --warnings-as-errors='*' introduces that redefining builtin macro error, but this has likely nothing to do with bazel_clang_tidy. This issue can be closed. Thanks for the help!