tidy: remove todo, set minimum CMake to 3.22 #29696

1. 
   fanquake commented at 5:59 PM on March 21, 2024: member

   See https://github.com/hebasto/bitcoin/pull/123 for the minimum version bump.

2. 
   DrahtBot commented at 5:59 PM on March 21, 2024: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	hebasto

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

3. 
   hebasto commented at 6:11 PM on March 21, 2024: member

   Concept ACK, obviously :)

4. in contrib/devtools/bitcoin-tidy/CMakeLists.txt:11 in e1d29dd602 outdated

      7 | @@ -8,7 +8,11 @@ set(CMAKE_CXX_STANDARD 17)
      8 |  set(CMAKE_CXX_STANDARD_REQUIRED True)
      9 |  set(CMAKE_CXX_EXTENSIONS False)
     10 |  
     11 | -# TODO: Figure out how to avoid the terminfo check
     12 | +set(CMAKE_DISABLE_FIND_PACKAGE_CURL ON)
   

   ---

   ---

   

   hebasto commented at 11:53 AM on March 22, 2024:

   What Clang version should I test?

   Asking because the find_package(CURL) command is not used in my clang-14 installation.

   ---

   

   fanquake commented at 12:25 PM on March 22, 2024:

   What Clang version should I test?

   I assume any recent version, or the same one that is used in the CI job that uses this tool (18).

   ---

   

   hebasto commented at 12:53 PM on March 22, 2024:

   Right. I has been added since clang-17.0

5. in contrib/devtools/bitcoin-tidy/CMakeLists.txt:14 in e1d29dd602 outdated

      7 | @@ -8,7 +8,11 @@ set(CMAKE_CXX_STANDARD 17)
      8 |  set(CMAKE_CXX_STANDARD_REQUIRED True)
      9 |  set(CMAKE_CXX_EXTENSIONS False)
     10 |  
     11 | -# TODO: Figure out how to avoid the terminfo check
     12 | +set(CMAKE_DISABLE_FIND_PACKAGE_CURL ON)
     13 | +set(CMAKE_DISABLE_FIND_PACKAGE_ZLIB ON)
     14 | +
     15 | +set(LLVM_ENABLE_TERMINFO OFF)
   

   ---

   ---

   

   hebasto commented at 11:54 AM on March 22, 2024:

   Why not using the same approach:

   set(CMAKE_DISABLE_FIND_PACKAGE_Terminfo ON)
   

   ?

   ---

   

   fanquake commented at 12:26 PM on March 22, 2024:

   Mostly because I assumed we'd want to use the LLVM options, happy to change either way.

6. tidy: set minimum CMake to 3.22

   Matches https://github.com/hebasto/bitcoin/pull/123.
   This also also dev/ci only code.

   24410e560a
7. fanquake force-pushed on Mar 22, 2024
8. hebasto approved
9. 
   hebasto commented at 1:04 PM on March 22, 2024: member

   ACK cf9857883d65730060e8d74cd4f1afe9dd69afaa.

   It seems, the zstd package might be ignored as well:

   set(CMAKE_DISABLE_FIND_PACKAGE_zstd ON)
   

   UPD: ... and LibEdit?

10. tidy: remove terminfo TODO

    At the same time, also disable searching for CURL, LibEdit, LibXml2,
    ZLIB and zstd none of which we use.

    5b690aeb15
11. fanquake force-pushed on Mar 22, 2024
12. 
    fanquake commented at 1:35 PM on March 22, 2024: member

    UPD: ... and LibEdit?

    Added LibXml2 while we are at it.

13. tidy: set CMAKE_CXX_STANDARD to 20 c3a4ea1971
14. 
    hebasto commented at 1:41 PM on March 22, 2024: member

    Maybe also disable unneeded C compiler check:

    --- a/contrib/devtools/bitcoin-tidy/CMakeLists.txt
    +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt
    @@ -1,6 +1,10 @@
     cmake_minimum_required(VERSION 3.9)
     
    -project(bitcoin-tidy VERSION 1.0.0 DESCRIPTION "clang-tidy checks for Bitcoin Core")
    +project(bitcoin-tidy
    +    VERSION 1.0.0
    +    DESCRIPTION "clang-tidy checks for Bitcoin Core"
    +    LANGUAGES CXX
    +)
     
     include(GNUInstallDirs)
     
    

    ?

15. tidy: remove C compiler check

    Also requires disabling FFI.

    11ee058ef5
16. 
    fanquake commented at 1:48 PM on March 22, 2024: member

    Maybe also disable unneeded C compiler check:

    That works, but only after also disabling FFI.

17. hebasto approved
18. 
    hebasto commented at 2:44 PM on March 22, 2024: member

    re-ACK 11ee058ef5794de5f1b8e89d62bfa69c64693fff.

19. fanquake requested review from TheCharlatan on Mar 22, 2024
20. fanquake merged this on Mar 25, 2024
21. fanquake closed this on Mar 25, 2024

    ---

22. fanquake deleted the branch on Mar 25, 2024
23. bitcoin locked this on Mar 25, 2025

Contributors

fanquakeDrahtBothebasto

Review Requested
TheCharlatan