cmake: Improve robustness and usability #31233

1. 
   hebasto commented at 1:45 pm on November 6, 2024: member

   This PR:

   1. Switches to a modern CMake approach by using the Python3::Interpreter imported target, which is more robust than using variables.

   2. Disables tests instead of ignoring them.

   For example:

   • building without Python available:

    0$ cmake -B build
    1$ cmake --build build -j 16
    2$ # ctest --test-dir build -j 16 -R "^util_"
    3Internal ctest changing into directory: /bitcoin/build
    4Test project /bitcoin/build
    5    Start 113: util_tests
    6    Start 114: util_threadnames_tests
    7    Start 112: util_string_tests
    8    Start   1: util_test_runner
    91/5 Test   [#1](/bitcoin-bitcoin/1/): util_test_runner .................***Not Run (Disabled)   0.00 sec
   10    Start   2: util_rpcauth_test
   112/5 Test   [#2](/bitcoin-bitcoin/2/): util_rpcauth_test ................***Not Run (Disabled)   0.00 sec
   123/5 Test [#112](/bitcoin-bitcoin/112/): util_string_tests ................   Passed    0.01 sec
   134/5 Test [#114](/bitcoin-bitcoin/114/): util_threadnames_tests ...........   Passed    0.01 sec
   145/5 Test [#113](/bitcoin-bitcoin/113/): util_tests .......................   Passed    0.13 sec
   15
   16100% tests passed, 0 tests failed out of 3
   17
   18Total Test time (real) =   0.13 sec
   19
   20The following tests did not run:
   21	  1 - util_test_runner (Disabled)
   22	  2 - util_rpcauth_test (Disabled)
   

   • building without bitcoin-tx:

    0$ cmake -B build -DBUILD_TX=OFF
    1$ cmake --build build -j 16
    2$ ctest --test-dir build -j 16 -R "^util_"
    3Internal ctest changing into directory: /bitcoin/build
    4Test project /bitcoin/build
    5    Start   1: util_test_runner
    61/5 Test   [#1](/bitcoin-bitcoin/1/): util_test_runner .................***Not Run (Disabled)   0.00 sec
    7    Start   2: util_rpcauth_test
    8    Start 112: util_string_tests
    9    Start 113: util_tests
   10    Start 114: util_threadnames_tests
   112/5 Test [#112](/bitcoin-bitcoin/112/): util_string_tests ................   Passed    0.01 sec
   123/5 Test [#114](/bitcoin-bitcoin/114/): util_threadnames_tests ...........   Passed    0.01 sec
   134/5 Test   [#2](/bitcoin-bitcoin/2/): util_rpcauth_test ................   Passed    0.06 sec
   145/5 Test [#113](/bitcoin-bitcoin/113/): util_tests .......................   Passed    0.13 sec
   15
   16100% tests passed, 0 tests failed out of 4
   17
   18Total Test time (real) =   0.13 sec
   19
   20The following tests did not run:
   21	  1 - util_test_runner (Disabled)
   

2. cmake, refactor: Switch to `Python3::Interpreter` imported target

   Imported targets are more robust than using variables.

   4d5bc29eee
3. cmake, test: Disable tests instead of ignoring them 4b6a842c28
4. hebasto added the label Build system on Nov 6, 2024
5. hebasto added the label Tests on Nov 6, 2024
6. 
   DrahtBot commented at 1:45 pm on November 6, 2024: 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/31233.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	lucasbalieiro, BrandonOdiwuor

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #30997 (build: Switch to Qt 6 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.

7. 
   hebasto commented at 3:35 pm on November 6, 2024: member

   My Guix build:

    0aarch64
    1b4bd41285ec597b50489a3e3bf8ffde5e557d514ad5860fa3cfdd04da2144839  guix-build-4b6a842c2801/output/aarch64-linux-gnu/SHA256SUMS.part
    23222cf16fe93c3df73efc360150b384bf968f18ce929875bf384c9833e6411e7  guix-build-4b6a842c2801/output/aarch64-linux-gnu/bitcoin-4b6a842c2801-aarch64-linux-gnu-debug.tar.gz
    368cb1589be0fe45c8d9bd2dfa96b2610d49fcec5621b678a952db857493fdae9  guix-build-4b6a842c2801/output/aarch64-linux-gnu/bitcoin-4b6a842c2801-aarch64-linux-gnu.tar.gz
    49e289ec70fff76b6f6079ea852bb192bb97976618460404d46570992e6416964  guix-build-4b6a842c2801/output/arm-linux-gnueabihf/SHA256SUMS.part
    55c26ca52d1b93ac1356ee002fcdc772262dd8d7611da0b4295a286d512702e24  guix-build-4b6a842c2801/output/arm-linux-gnueabihf/bitcoin-4b6a842c2801-arm-linux-gnueabihf-debug.tar.gz
    66b2c340d82b641c58b956e27b8b034a72e791435cbc6c9de6551d6d359495cc7  guix-build-4b6a842c2801/output/arm-linux-gnueabihf/bitcoin-4b6a842c2801-arm-linux-gnueabihf.tar.gz
    7351619f4e964f8d2b7c8e22d1ddd6b1d0db0cf27c3e1be36c489a9962388a8b2  guix-build-4b6a842c2801/output/arm64-apple-darwin/SHA256SUMS.part
    8ea99f4078880f5233dd0e46666611839d52a9ffaf35fa4d85471f09e320da519  guix-build-4b6a842c2801/output/arm64-apple-darwin/bitcoin-4b6a842c2801-arm64-apple-darwin-unsigned.tar.gz
    98b78963b310762d2e0068bc8ba0b643ba4cae110d2942d659387fea807297718  guix-build-4b6a842c2801/output/arm64-apple-darwin/bitcoin-4b6a842c2801-arm64-apple-darwin-unsigned.zip
   1092ac17a362ec939c33498179371224d60daeb0338ea482edffb7ce0cbea551d0  guix-build-4b6a842c2801/output/arm64-apple-darwin/bitcoin-4b6a842c2801-arm64-apple-darwin.tar.gz
   11fece3efc27e98b0e163e79adf8e965cbff67037a6e73c217fb58bc40d19f7f1d  guix-build-4b6a842c2801/output/dist-archive/bitcoin-4b6a842c2801.tar.gz
   12f9383706e867737e69cc6853487d88dcf92c5a1b1e350e27e69115a0853e8d4f  guix-build-4b6a842c2801/output/powerpc64-linux-gnu/SHA256SUMS.part
   13c3a3a578d64a2721e1be9b92dd42d787c270a0fe5a16c9c4821abf833f2020e4  guix-build-4b6a842c2801/output/powerpc64-linux-gnu/bitcoin-4b6a842c2801-powerpc64-linux-gnu-debug.tar.gz
   14fa2df5e6902c846657aa57d75d900010e2cc3f00ad946785fb6afb961b72b132  guix-build-4b6a842c2801/output/powerpc64-linux-gnu/bitcoin-4b6a842c2801-powerpc64-linux-gnu.tar.gz
   15df2f59f8d71094fa4d2aa5eab39dce5136f004813746c5773bb793460d94cb45  guix-build-4b6a842c2801/output/riscv64-linux-gnu/SHA256SUMS.part
   1692f62e9650540ac8cdb44b7cd24998e038dfcac69d9db17bfe6d09cd8df02c71  guix-build-4b6a842c2801/output/riscv64-linux-gnu/bitcoin-4b6a842c2801-riscv64-linux-gnu-debug.tar.gz
   17ba982ed663b82f143b82807c873697eeea90aa99812d4663cd36d80dd66d2664  guix-build-4b6a842c2801/output/riscv64-linux-gnu/bitcoin-4b6a842c2801-riscv64-linux-gnu.tar.gz
   18cf98209feab437849d8bf48839ae303310cd4b89fe4117556ff303a3f119471c  guix-build-4b6a842c2801/output/x86_64-apple-darwin/SHA256SUMS.part
   19923859ff858ae7c521aea4a211752623133d52a38b475e335b66241aa97f5193  guix-build-4b6a842c2801/output/x86_64-apple-darwin/bitcoin-4b6a842c2801-x86_64-apple-darwin-unsigned.tar.gz
   206398753ab5dc7a7976a7a109be9b5ed71259b4c9fc68d8312e936009f5c9ace8  guix-build-4b6a842c2801/output/x86_64-apple-darwin/bitcoin-4b6a842c2801-x86_64-apple-darwin-unsigned.zip
   2113c6f9050ee33e1324b3cf65f97d5ab2540644953a1c7ce94c053e6395c9508a  guix-build-4b6a842c2801/output/x86_64-apple-darwin/bitcoin-4b6a842c2801-x86_64-apple-darwin.tar.gz
   22fee8c5bf60d56acfd6cdaed763ac841e132eea3cfc839b0bc8d824bc5d554cb6  guix-build-4b6a842c2801/output/x86_64-linux-gnu/SHA256SUMS.part
   23f92482e905e2949c1f34d60d4a895b2fafe09825ed00e5aa5401ae30d0976757  guix-build-4b6a842c2801/output/x86_64-linux-gnu/bitcoin-4b6a842c2801-x86_64-linux-gnu-debug.tar.gz
   24d958174725702e4be7162c2b8ad93afd600ad10e9e82598e8584914bbea9fe5d  guix-build-4b6a842c2801/output/x86_64-linux-gnu/bitcoin-4b6a842c2801-x86_64-linux-gnu.tar.gz
   2549a7b22039c44c4ed1c6324811d9043944665320560bc12dcbc7d3ae1321807a  guix-build-4b6a842c2801/output/x86_64-w64-mingw32/SHA256SUMS.part
   2633c90eb5a0393e35418dcb1ddf333f90df35804c6a2ae1352aacdf5908fc1e3f  guix-build-4b6a842c2801/output/x86_64-w64-mingw32/bitcoin-4b6a842c2801-win64-debug.zip
   274a24314797582e87ffdb7c36d248cd248682ceec62e7f419b39beb464ce6200a  guix-build-4b6a842c2801/output/x86_64-w64-mingw32/bitcoin-4b6a842c2801-win64-setup-unsigned.exe
   287f1c314c88d858976f7c0c5f0fb49540d758ef585c41b8dcd884fd5e981dd941  guix-build-4b6a842c2801/output/x86_64-w64-mingw32/bitcoin-4b6a842c2801-win64-unsigned.tar.gz
   294e7c8a0c6770b18072d7dd641cd4d1904e03728196e5c3c2a9a84a82fc27cdf1  guix-build-4b6a842c2801/output/x86_64-w64-mingw32/bitcoin-4b6a842c2801-win64.zip
   

8. lucasbalieiro approved
9. 
   lucasbalieiro commented at 3:48 am on November 19, 2024: none

   Hi, @hebasto!

   tACK Commit 4b6a842

   I have successfully run the build modifications on my machine:

   Distributor: Ubuntu
   Version: 22.04.5 LTS
   Release: 22.04
   Codename: Jammy

   The build completed without any warnings or errors. I also ran the tests both with and without Python, and the console responses were as expected. Please see the attached images for further details.

   1. Without Python
      

   2. With Python
      

   3. With -DBUILD_TX=OFF
      

   For the “no Python” simulation, I commented out the following line in CMakeLists.txt:

   0#find_package(Python3 3.10 COMPONENTS Interpreter)
   

   Minor Recommendations (Really minor):

   1. Verbosity for Missing Python:
      Consider adding more verbosity when Python is not found. For example, in tests.cmake:

      0if(NOT TARGET Python3::Interpreter)
      1  message(WARNING "Python3 interpreter not found. Some tests will be disabled.")
      2endif()
      

      This could help clarify the situation when Python is missing. Example output:

      

   2. Improving Readability with Variables:
      For better readability, it might be beneficial to replace TARGET_EXISTS checks with variables. For example:

      0set(PYTHON_AVAILABLE $<TARGET_EXISTS:Python3::Interpreter>)
      1set(UTIL_AND_TX_AVAILABLE $<AND:$<TARGET_EXISTS:bitcoin-util>,$<TARGET_EXISTS:bitcoin-tx>>)
      

      This would enhance clarity and make the conditions easier to follow.

   ---

   This is my first PR review, and I want to ensure I’m doing it correctly. If there’s anything I missed or could improve in my testing approach, feel free to let me know.
   Thanks for the contribution!

10. BrandonOdiwuor approved
11. 
    BrandonOdiwuor commented at 12:28 pm on November 19, 2024: contributor

    tACK 4b6a842c28010a00e121fd36cc0b4e1fa658d249 on Ubuntu 24.04

    Was able to successfully test that util_test_runner and util_rpcauth_test are correctly disabled when Python3 interpreter is not found (screenshots below)

    Overally this is an improvement, using the Python3::Interpreter target defined directly by find_package(…) is cleaner than relying on the ${PYTHON_COMMAND} variable we define

    without Python3

    With Python3

Contributors
hebasto DrahtBot lucasbalieiro BrandonOdiwuor

Labels
Build system Tests