ci: use clang-16 in tidy task #27404

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:clang_16_tidy_task changing 6 files +9 −14
  1. fanquake commented at 11:11 AM on April 3, 2023: member

    Follow up to #27311 (comment), as IWYU now has a clang_16 branch.

    This also removes some workarounds for (now fixed) clang-tidy issues, and simplifies the IWYU install steps.

  2. DrahtBot commented at 11:11 AM on April 3, 2023: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK hebasto, MarcoFalke, josibake

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #27340 (ci: Use Cirrus CI dockerfile env by MarcoFalke)
    • #25797 (build: Add CMake-based build system by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label Tests on Apr 3, 2023
  4. fanquake force-pushed on Apr 3, 2023
  5. hebasto commented at 11:12 AM on April 3, 2023: member

    Concept ACK.

  6. fanquake commented at 12:45 PM on April 3, 2023: member

    Note that even though the CI here is "green", the tidy task has failed:

    In file included from common/url.cpp:5:
    In file included from ./common/url.h:8:
    In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/string:40:
    In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/char_traits.h:39:
    In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/postypes.h:40:
    In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/cwchar:44:
    /usr/include/wchar.h:35:10: fatal error: 'stddef.h' file not found
    #include <stddef.h>
             ^~~~~~~~~~
    
  7. fanquake force-pushed on Apr 3, 2023
  8. DrahtBot cross-referenced this on Apr 3, 2023 from issue ci: Use Cirrus CI dockerfile env by maflcko
  9. DrahtBot cross-referenced this on Apr 4, 2023 from issue build: Add CMake-based build system by hebasto
  10. hebasto commented at 1:15 PM on April 4, 2023: member

    FWIW, on a local Ubuntu 23.04 installation, I have no issues with running the IWYU tool manually.

  11. hebasto commented at 4:54 PM on April 4, 2023: member
    /usr/include/wchar.h:35:10: fatal error: 'stddef.h' file not found
    #include <stddef.h>
             ^~~~~~~~~~
    

    This error can be fixed with the following diff:

    --- a/ci/test/00_setup_env_native_tidy.sh
    +++ b/ci/test/00_setup_env_native_tidy.sh
    @@ -15,5 +15,5 @@ export RUN_FUNCTIONAL_TESTS=false
     export RUN_FUZZ_TESTS=false
     export RUN_TIDY=true
     export GOAL="install"
    -export BITCOIN_CONFIG="CC=clang-16 CXX=clang++-16 --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'"
    +export BITCOIN_CONFIG="CC=clang-16 CXX=clang++-16 --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-16/lib/clang/16/include'"
     export CCACHE_SIZE=200M
    

    See: https://cirrus-ci.com/task/5852229609979904

    Some related discussion see in https://github.com/include-what-you-use/include-what-you-use/issues/679.


    Note that even though the CI here is "green", the tidy task has failed

    #26763 is somewhat related.

  12. maflcko commented at 5:10 PM on April 4, 2023: member

    This error can be fixed with the following diff:

    I wonder if another workaround would be to use libc++, but I haven't checked if the iwyu output is better or worse with libc++.

  13. maflcko commented at 10:02 AM on April 5, 2023: member
  14. ci: use clang-16 in tidy task a56c96507a
  15. fanquake force-pushed on Apr 5, 2023
  16. fanquake commented at 10:48 AM on April 5, 2023: member

    -I/usr/lib/llvm-16/lib/clang/16/include

    Included this. I think we could merge this as-is, but should also figure out why it's needed. It feels like there is another issue here.

    Also #25466 (review)

    Removed the related suppressions.

  17. fanquake cross-referenced this on Apr 5, 2023 from issue ci: add unused-using-decls to clang-tidy by fanquake
  18. fanquake marked this as ready for review on Apr 5, 2023
  19. hebasto approved
  20. hebasto commented at 11:05 AM on April 5, 2023: member

    ACK a56c96507a9e943bbcd7e126bc827de9495f0ebd

  21. maflcko commented at 11:23 AM on April 5, 2023: member

    lgtm ACK a56c96507a9e943bbcd7e126bc827de9495f0ebd

  22. josibake commented at 11:57 AM on April 5, 2023: member

    ACK https://github.com/bitcoin/bitcoin/pull/27404/commits/a56c96507a9e943bbcd7e126bc827de9495f0ebd

    Included this. I think we could merge this as-is, but should also figure out why it's needed. It feels like there is another issue here.

    did a little digging and found this: https://github.com/include-what-you-use/include-what-you-use/issues/679#issuecomment-482798379 , which seems to still be an issue based on the discussion in the linked solution. fwiw, the fix you have here seems to be one of the preferred ways of dealing with this

  23. fanquake merged this on Apr 5, 2023
  24. fanquake closed this on Apr 5, 2023

  25. fanquake deleted the branch on Apr 5, 2023
  26. sidhujag referenced this in commit d72911b6c4 on Apr 5, 2023
  27. bitcoin locked this on Apr 4, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:53 UTC