cmake: make missing Python interpreter behaviour more explicit

stickies-v commented at 11:45 am on September 2, 2025: contributor

We require Python 3.10 for multiple targets, and currently raise a general warning if it is missing, leading to:

functional test suite: runtime failure if python missing (as e.g. described in #31476)
maintenance targets: targets skipped if python missing
macos deploy target: target created, but build fails if python missing

This PR:

adds a BUILD_FUNCTIONAL_TESTS option (default ON) and raises FATAL_ERROR if Python is missing and we’re building the functional tests
raises explicit warnings (in-line, not at the end of the configure summary) for maintenance and macos deploy targets, and skips the macos deploy target instead of letting the build fail
removes the general missing python warning at the end of the configure summary
removes a dependency on our custom configure_warnings system (but requires further work to be completely removed)

Behaviour changes:

cmake -B build now throws a FATAL_ERROR if minimum Python could not be found (overridden with cmake -B build -DBUILD_FUNCTIONAL_TESTS=0)
deploy target no longer created if minimum Python could not be found
warnings are raised on a usage basis, and no longer generically mentioned at the end of configure

Alternative to #31669 and partially to #33144 and #32865, partially addresses #31476.

cmake: explicitly warn if maintenance targets are skipped ac83a51c32

cmake: skip darwin deploy target if python not found

Rather than failing, skip the deploy target (which was not explicitly
requested) and explicitly warn the user.

92fb8c02db

DrahtBot added the label Build system on Sep 2, 2025

DrahtBot commented at 11:45 am on September 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/33278.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	davidgumberg

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:

#33247 (build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings by 151henry151)

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.

hebasto commented at 2:49 pm on September 2, 2025: member

cc @purpleKarrot

in CMakeLists.txt:103 in 5ed626fa18 outdated

 99@@ -100,6 +100,7 @@ option(BUILD_GUI "Build bitcoin-qt executable." OFF)
100 option(BUILD_CLI "Build bitcoin-cli executable." ON)
101 
102 option(BUILD_TESTS "Build test_bitcoin and other unit test executables." ON)
103+option(BUILD_FUNCTIONAL_TESTS "Build the functional tests." ${BUILD_TESTS})

purpleKarrot commented at 6:45 am on September 3, 2025:

I think this should be a dependent option. Also, we should start prefixing all options to allow the project to be used as a subproject:

0cmake_dependent_option(BTCCORE_ENABLE_FUNCTIONAL_TESTS "Enable the functional tests" ON BUILD_TESTS OFF)

stickies-v commented at 7:48 am on September 3, 2025:

I think this should be a dependent option.

Why? Imo it’s not unreasonable to want to build with only one of unit tests and functional tests, they don’t depend on each other.

Edit: BUILD_TESTS is actually more BUILD_UNIT_TESTS, but that change is out of scope for this PR imo.

Also, we should start prefixing all options to allow the project to be used as a subproject:

That sounds like a good idea, but pragmatically I think it makes sense to first open a separate issue, to discuss and get consensus on the goal and approach (e.g. prefix name), so I’m going to hold off on that here.

purpleKarrot commented at 8:11 am on September 3, 2025:

Imo it’s not unreasonable to want to build with only one of unit tests and functional tests, they don’t depend on each other.

They don’t? ${BUILD_TESTS} as the default value of the BUILD_FUNCTIONAL_TESTS option implies a dependency. If they are independent, maybe give them independent default values?

stickies-v commented at 12:59 pm on September 3, 2025:

If they are independent, maybe give them independent default values?

Done, thanks. I used the default BUILD_TESTS value because I suspect in most cases if people don’t want to build the unit tests, they also don’t want to build the functional tests, but perhaps if we do want that logic it’s best to have a BUILD_TESTS that acts like an umbrella setting, with cmake_dependent_options BUILD_UNIT_TESTS and BUILD_FUNCTIONAL_TESTS. That seems out of scope for this PR, so I’ve just changed the default to ON, reducing behaviour change in this PR (updated description).

cmake: add BUILD_FUNCTIONAL_TESTS option

Adds new option (default on) to control the building
of functional tests. In a future commit, allows us to FATAL_ERROR if
no required Python interpreter can be found.

7db9ebdf73

cmake: fatal error for functional tests without python

The functional tests have a hard Python requirement. Building them
without being able to run them is unintuitive and error-prone for
automated systems like CI, so instead raise a FATAL_ERROR if
Python requirement is not satisfied.

Note: on platforms without the minimum (currently 3.10) Python version
installed, this will lead to the default configuration (`cmake -B
build`) failing, instead requiring tests to be explicitly disabled
(`cmake -B build -DBUILD_FUNCTIONAL_TESTS=0`).

98b84771ec

cmake: remove generic Python warning

Affected targets or usage already has explicit warnings or errors.

bad9092725

stickies-v force-pushed on Sep 3, 2025

stickies-v commented at 1:00 pm on September 3, 2025: contributor

Force-pushed to default BUILD_FUNCTIONAL_TESTS to ON instead of to BUILD_TESTS, reducing behaviour change and addressing @purpleKarrot’s feedback.

davidgumberg commented at 9:42 pm on September 4, 2025: contributor

ACK https://github.com/bitcoin/bitcoin/pull/33278/commits/bad909272545defe9f7d8f27cfa3ff79859aa828

We should not be building functional tests without the minimum python version satisfied, it seems fine to leave it as a warning for maintenance and deploy targets, since these will error later when attempting to build the target.

in test/CMakeLists.txt:33 in bad9092725

29@@ -30,6 +30,10 @@ function(create_test_config)
30   configure_file(config.ini.in config.ini USE_SOURCE_PERMISSIONS @ONLY)
31 endfunction()
32 
33+if(NOT TARGET Python3::Interpreter)

BrandonOdiwuor commented at 7:16 pm on September 9, 2025:

Concept ACK

I am however unable to trigger the fatal error on Ubuntu 24.04.3 LTS with Python 3.9.23

Adding a few debug logs show that Python3::Interpreter target is set even if the minimum version is not met

stickies-v commented at 9:36 pm on September 9, 2025:

That’s interesting. The https://cmake.org/cmake/help/v3.14/module/FindPython.html docs for Python::Interpreter state “Target defined if component Interpreter is found.”, so I’m not sure why that’s not failing for you.

Since the behaviour of relying on Python3::Interpreter is not changed in this PR, I suspect you won’t get the “Minimum required Python not found.” warning (at the very end of the configure step) that’s removed in this PR, since that relies on the same mechanism? Correct?

I’ve implemented an alternative approach in https://github.com/stickies-v/bitcoin/commits/2025-09/cmake-fatal-python-found/ - does that throw the expected warning for you?

cc @hebasto since you recently updated this in bb9157db5d3920d971e365e416de4e23866c715a - is it important that we keep relying on checking the existence of the Python3::Interpreter target?

cmake: make missing Python interpreter behaviour more explicit #33278

Code Coverage & Benchmarks

Reviews

Conflicts