WITH_PYTHON build option
#31669
This was suggested in #31476 (comment):
An alternative would be to require python to be disabled explicitly. Otherwise, it seems odd that every setting in cmake has a static default that can only be overridden explicitly, except for some, which are silently downgraded?
This PR updates the build system to fail by default on systems where the minimum required Python version is unavailable. It introduces an option that allows users to explicitly disable the Python requirement.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31669.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept NACK | fanquake |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
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.
Not sure. Why does this need another build option? Given that this is just for working around silent CMake warnings in the CI, couldn’t this just be fixed by using explicit *_TEST options that otherwise fail if Python is missing?
It also doesn’t solve #31476 generically, because every time a similar case comes up, we’ll have to add even more code and/or options.
NACK - based on the reasoning above.
Not sure. Why does this need another build option? Given that this is just for working around silent CMake warnings in the CI, couldn’t this just be fixed by using explicit
*_TESToptions that otherwise fail if Python is missing?
What “explicit *_TEST options” do you mean?
By default, the build system generation will fail on systems where the
minimum required Python version is unavailable. This option allows
users to explicitly disable the Python requirement.
571+ set(Python3_FIND_UNVERSIONED_NAMES FIRST CACHE STRING "")
572+ mark_as_advanced(Python3_FIND_FRAMEWORK Python3_FIND_UNVERSIONED_NAMES)
573+ find_package(Python3 3.10 REQUIRED COMPONENTS Interpreter)
574 set(PYTHON_COMMAND ${Python3_EXECUTABLE})
575 else()
576 list(APPEND configure_warnings
shouldn’t this be an error now?
Why? It precisely mirrors the scenario “I don’t have python and I don’t care about the tests” and simply serves as a reminder to the user of the consequences:
0$ cmake -B build -DWITH_PYTHON=OFF
1...
2Use ccache for compiling .............. ON
3
4
5 ******
6
7CMake Warning at CMakeLists.txt:680 (message):
8 Utils and rpcauth tests are disabled.
9
10 Use "-DWITH_PYTHON=ON" to enable them.
11
12
13 ******
14
15-- Configuring done (12.8s)
16-- Generating done (0.0s)
17-- Build files have been written to: /home/hebasto/dev/bitcoin/build
An error happens when WITH_PYTHON=ON is set, which is the default, but Python has not been found.
I see. I guess I wanted to say the opposite: Can’t this be removed, because it seems a bit odd to print a warning that something is disabled, when the users explicitly asked it to be disabled? There is also no warning when a user sets BUILD_TESTS=OFF.
Another alternative would be to just require python3, without a way to disable it. Thus, the option WITH_PYTHON can be removed.
I am just wondering: Was there ever a single user (compiling from source) to ask for the python dependency to be optional, or can such a user practically exist? Normally, when compiling from source, someone should at a minimum run all tests. Also, python is so widely packaged, that installing it to compile Bitcoin Core should not be an obstacle?
Labels
Build system