-
Notifications
You must be signed in to change notification settings - Fork 38.7k
cmake: use python from venv if available #31411
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.
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
Conversation
First check if $VIRTUAL_ENV is set. If it is, use the python binary from this venv. Fall back to system python otherwise. This helps run various targets which use python, when system python is not being used.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31411. ReviewsSee the guideline for information on the review process. ConflictsReviewers, 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. |
|
As far as I know, all venv providers set |
A quick note. This PR seems incompatible with #31233. |
|
Thanks hebasto, I had not seen this PR. Will compare/review. It has come to my attention that having these checks complete in a dev environment is not that useful anyway, so perhaps "making things easier for me to run locally" is not a great motivation for this change in any case... |
This one? |
|
Yes, and I highly recommend it! |
I've read https://github.com/astral-sh/uv/blob/main/README.md#python-management and used the following script for simplicity: cmake_minimum_required(VERSION 3.22)
project(test_py LANGUAGES NONE)
find_package(Python3 COMPONENTS Interpreter)
message("Python3_EXECUTABLE=${Python3_EXECUTABLE}")I have no problems with finding What did I miss? |
|
I had the same thought. Shouldn't a venv "just work" out of the box, because they set the |
Apparently not. Here's another case where CMake just picks some other Python: # python --version
Python 3.13.1
# python3 --version
Python 3.13.1
# which python
/Users/xxx/.pyenv/shims/python
# cmake -B build
<snip>
-- Found Python3: /opt/homebrew/Frameworks/Python.framework/Versions/3.13/bin/python3.13 (found suitable version "3.13.0", minimum required is "3.10") found components: Interpreter |
I cannot reproduce this on my Sequoia 15.1.1 (Intel): CMake picks the Python from Any specific steps are required to reproduce your issue? |
I'm not using a |
|
Thanks @hebasto for the double-check. It has helped me work out what was going wrong for me (I think)... Looks like I had not bumped my python version in the venv since #30527 so I was still running 3.9 in my venv. This caused the This was then combined with me manually exporting So this seems to have been my error indeed. I can confirm that using a python 3.10 venv fixes the issue for me. I think I can probably close this now, and perhaps move to an issue while we figure out what cmake is doing with @fanquake's setup? As this PR does not apparently fix that case, so seems largely useless now. NB I would note that it could be nice to give a warning when a venv is detected but not used (as it doesn't meet requirements), but seems like a nice-to-have, rather than something essential. |
My apologies. Adding |
Right, so CMake just picks a different version here for some reason? Is this a bug? Do we need to document this, so it's clear to users/developers that sometimes CMake will just ignore the shell/in-use Python version? |
The behaviour is extensively documented:
|
Sure, but none of that means anything to a (new) developer, who doesn't care about the intricacies of the build system. It's just unintuituve that CMake would pick some version of Python, other than the one their shell/sytem is (globally) configured to use. We even suggest using |
|
Does any of this matter in practise for running the tests? The tests should be passing for any supported python version, as long as it is above the minimum, which is what cmake already checks for. Also, the tests can be run with a python version explicitly, like Though, even for writing tests in python, the version should mostly be irrelevant now that python3 has matured for the commonly used batteries. Historically, one could accidentally write python3.6 code, when the minimum supported was 3.5, however I don't think this is relevant today when new python features aren't applicable to the tests in this repo anyway. In any case, if people think that forcing |
If CMake picks a different Python to the one you expect it to use, which is not the one you've |
|
OK the issue here is that most venv providers export the From the cmake docs:
As Therefore the only remaining issue here IMO is how to handle I don't think it can be solved in a cmake context without custom cmake find package logic, which I think we should avoid. Therefore my preference would be to recommend that folks move to |
Fixed in #31421. |
Great, will close here then. |
dead908 cmake: Improve compatibility with Python version managers (Hennadii Stepanov) Pull request description: This PR resolves the issue [highlighted](#31411 (comment)) in #31411: > Here's another case where CMake just picks some other Python... The fix leverages two [hints](https://cmake.org/cmake/help/latest/module/FindPython3.html#hints) for the CMake `FindPython3` module: 1. `Python3_FIND_FRAMEWORK` is set to `LAST`. This ensures that Unix-style package components are preferred over frameworks on macOS. As a side effect, the `FindPython3` module reports a shim or symlink (e.g., from `pyenv`) rather than the underlying framework's binary. The module's output aligns with the result of the `which` command. 2. `Python3_FIND_UNVERSIONED_NAMES` is set to `FIRST`. This supports scenarios where tools like `pyenv`—which use shims—have multiple Python versions installed. Here are examples of output on my macOS 15.1.1 (Intel) with installed Homebrew's [Python 3.13.0](https://formulae.brew.sh/formula/python@3.13): - without any Python version manager: ``` % which -a python3 /usr/local/bin/python3 /usr/bin/python3 % cmake -B build <snip> -- Found Python3: /usr/local/bin/python3 (found suitable version "3.13.0", minimum required is "3.10") found components: Interpreter <snip> ``` - using `pyenv`: ``` % pyenv versions system * 3.10.14 (set by /Users/hebasto/dev/bitcoin/.python-version) 3.12.8 3.13.1 % which -a python3 /Users/hebasto/.pyenv/shims/python3 /usr/local/bin/python3 /usr/bin/python3 % cmake -B build <snip> -- Found Python3: /Users/hebasto/.pyenv/shims/python3 (found suitable version "3.10.14", minimum required is "3.10") found components: Interpreter <snip> ``` Both variables, `Python3_FIND_FRAMEWORK` and `Python3_FIND_UNVERSIONED_NAMES`, can still be overridden by the user via the command line if needed. ACKs for top commit: theuni: No opinion on the python selection changes themselves, but code-review ACK dead908 willcl-ark: ACK dead908 Tree-SHA512: 69f8541223e5b6c35c892b4ba2a2dcfc24b41a10cf20accc75d3008b16434db8a9240c99c886c3a4566ba24269c5b0e0d856357891811f0a77b39f4afbee3634
When running targets that depend on a python, such as:
... it can be tricky to configure cmake to use a python venv. This change checks for the
$VIRTUAL_ENVenv var and uses python from there using the following logic: