doc: Remove build instruction for running clang-tidy #32662

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:250602-doc-clang-tidy changing 1 files +0 −1
  1. hebasto commented at 12:50 pm on June 2, 2025: member
    One of the benefits of using a compilation database, which is available after the CMake build system generation step, is that it is not necessary to actually build the code in order to run clang-tidy.
  2. doc: Remove build instruction for running `clang-tidy`
    One of the benefits of using a compilation database, which is available
    after the CMake build system generation step, is that it is not
    necessary to actually build the code in order to run `clang-tidy`.
    4b1b36acb4
  3. hebasto added the label Docs on Jun 2, 2025
  4. DrahtBot commented at 12:50 pm on June 2, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32662.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK TheCharlatan, janb84

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

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • “denoised of errors” → “denoised, with errors removed” [“denoised of” is not idiomatic English]

    drahtbot_id_4_m

  5. TheCharlatan approved
  6. TheCharlatan commented at 1:43 pm on June 2, 2025: contributor
    ACK 4b1b36acb48fab133ca4a3241148fa9683915874
  7. janb84 commented at 1:44 pm on June 2, 2025: contributor

    ACK 4b1b36acb48fab133ca4a3241148fa9683915874

    Checked both outcomes, the “old” way and the new way, both outcomes are the equal.(the diff was harder to check due to running it in multithreaded mode, therefor the files where out of order)

  8. fanquake merged this on Jun 2, 2025
  9. fanquake closed this on Jun 2, 2025

  10. hebasto deleted the branch on Jun 2, 2025
  11. maflcko commented at 1:37 pm on June 3, 2025: member

    Shouldn’t this be excluded in the tidy CI task as well?

    Also, would be good to mention that this will now print errors about the missing auto-generated files.

  12. TheCharlatan referenced this in commit f236a50dc1 on Jun 14, 2025
  13. fanquake commented at 1:47 pm on June 19, 2025: member

    Also, would be good to mention that this will now print errors about the missing auto-generated files.

    Yea, I think we could actually revert this. It mostly works, but as you say, now produces errors due to missing files, i.e:

    0/root/ci_scratch/src/test/sighash_tests.cpp:13:10: error: 'test/data/sighash.json.h' file not found [clang-diagnostic-error]
    1   13 | #include <test/data/sighash.json.h>
    2      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
    3694 warnings and 1 error generated.
    

    and it’s going to produce even more after something like #31802. i.e:

    0clang-tidy-18 -p=../build /root/ci_scratch/build/src/test/ipc_test.capnp.proxy-types.c++
    1error: no input files [clang-diagnostic-error]
    2error: no such file or directory: '/root/ci_scratch/build/src/test/ipc_test.capnp.proxy-types.c++' [clang-diagnostic-error]
    3error: unable to handle compilation, expected exactly one compiler job in '' [clang-diagnostic-error]
    4Error while processing /root/ci_scratch/build/src/test/ipc_test.capnp.proxy-types.c++.
    5Found compiler error(s).
    

    If we don’t want to revert, then it should be documented, but that also seems a bit odd for dev tooling, if we are going to document something like: “You might see a bunch of errors, but this might be fine”.

  14. ryanofsky commented at 2:37 pm on June 20, 2025: contributor

    It does seem nice to be able to run clang-tidy on generated files, so maybe the build command could be added back with a comment like # Optional, only necessary to run clang-tidy on generated sources. Maybe more ideally there could be custom target like make --build build -t codegen that only runs code generation steps and doesn’t compile the sources.

    Alternately if we don’t think it’s useful to run tidy on generated files maybe we could suggest using jq to filter them like CI does:

    https://github.com/bitcoin/bitcoin/blob/154b98a7aaae248f5d40ca567d1ec51410d09bae/ci/test/03_test_script.sh#L165

    It could make sense to have the jq filtering command in a script that could be shared between CI and these instructions.

  15. fanquake commented at 11:09 am on June 23, 2025: member
    @hebasto Are you going to follow up here?

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: 2025-07-08 12:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me