Skip to content

cmake: make missing Python interpreter behaviour more explicit #33278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

cmake: make missing Python interpreter behaviour more explicit #33278

wants to merge 5 commits into from
+13 −6

Conversation

stickies-v
Copy link
Contributor

@stickies-v stickies-v commented Sep 2, 2025 •
edited
Loading

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

  1. functional test suite: runtime failure if python missing (as e.g. described in CI: Cmake warnings should be errors #31476)
  2. maintenance targets: targets skipped if python missing
  3. 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.

@stickies-v
cmake: explicitly warn if maintenance targets are skipped
ac83a51
@stickies-v
cmake: skip darwin deploy target if python not found
92fb8c0 
Rather than failing, skip the deploy target (which was not explicitly
requested) and explicitly warn the user.
@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 2, 2025 •
edited
Loading

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.

@stickies-v stickies-v mentioned this pull request Sep 2, 2025
Open
@hebasto
Copy link
Member

hebasto commented Sep 2, 2025

cc @purpleKarrot

@DrahtBot DrahtBot mentioned this pull request Sep 2, 2025
Open
purpleKarrot
purpleKarrot reviewed Sep 3, 2025
View reviewed changes
@stickies-v
cmake: add BUILD_FUNCTIONAL_TESTS option
7db9ebd 
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.
@stickies-v
cmake: fatal error for functional tests without python
98b8477 
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`).
@stickies-v
cmake: remove generic Python warning
bad9092 
Affected targets or usage already has explicit warnings or errors.
@stickies-v stickies-v force-pushed the 2025-09/cmake-fatal-python branch from a0b6492 to bad9092 Compare September 3, 2025 12:56
@stickies-v
Copy link
Contributor Author

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

@davidgumberg
Copy link
Contributor

ACK bad9092

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@purpleKarrot purpleKarrot purpleKarrot left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stickies-v @DrahtBot @hebasto @davidgumberg @purpleKarrot