CMake: react to python version changes #3299
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new FindPython-based variant of the CMake scripts caches information about the chosen Python version that can become stale. For example, suppose I configure a simple pybind11-based project as follows ``` cmake -S . -B build -GNinja -DPython_ROOT=<path to python 3.8> ``` which will generate `my_extension.cpython-38-x86_64-linux-gnu.so`. A subsequent change to the python version like ``` cmake -S . -B build -GNinja -DPython_ROOT=<path to python 3.9> ``` does not update all necessary build system information. In particular, the compiled file is still called `my_extension.cpython-38-x86_64-linux-gnu.so`. This commit fixes the problem by detecting changes in `Python_EXECUTABLE` and re-running Python as needed. Note that the previous way of detecting Python does not seem to be affected, it always specifies the right suffix.
for more information, see https://pre-commit.ci
|
Amazing! (the pre-commit fixes :D) |
henryiii
reviewed
Sep 24, 2021
tools/pybind11NewTools.cmake
Outdated
Comment on lines
85
to
89
| if(NOT ${_Python}_EXECUTABLE STREQUAL ${_Python}_EXECUTABLE_LAST) | ||
| # Detect changes to the Python version/binary in subsequent CMake runs, and refresh config if needed | ||
| unset(PYTHON_IS_DEBUG CACHE) | ||
| unset(PYTHON_MODULE_EXTENSION CACHE) | ||
| set(${_Python}_EXECUTABLE_LAST |
There was a problem hiding this comment.
Suggested change
| if(NOT ${_Python}_EXECUTABLE STREQUAL ${_Python}_EXECUTABLE_LAST) | |
| # Detect changes to the Python version/binary in subsequent CMake runs, and refresh config if needed | |
| unset(PYTHON_IS_DEBUG CACHE) | |
| unset(PYTHON_MODULE_EXTENSION CACHE) | |
| set(${_Python}_EXECUTABLE_LAST | |
| if(NOT ${_Python}_EXECUTABLE STREQUAL PYTHON_EXECUTABLE_LAST) | |
| # Detect changes to the Python version/binary in subsequent CMake runs, and refresh config if needed | |
| unset(PYTHON_IS_DEBUG CACHE) | |
| unset(PYTHON_MODULE_EXTENSION CACHE) | |
| set(PYTHON_EXECUTABLE_LAST |
If you change from Python2 to Python3, then this should still be regenerated, since these variables are not tied to ${_Python}. (_PYTHON_EXECUTABLE_LAST would also be fine as a name)
Skylion007
approved these changes
Sep 24, 2021
|
(Thanks for the feedback, I plan to merge this once CI passes.) |
henryiii
approved these changes
Sep 24, 2021
wjakob
force-pushed
the
cmake-py-stale
branch
from
fd45b47 to
d8cca11
Compare
|
Thanks, I wondered how this actually managed to pass my (local) testing. Now it makes sense, I'm again removing the |
|
This is great. May I suggest to use a prefixed name, e.g. PYBIND11_PYTHON_EXECUTABLE_LAST, instead? This makes it easier to identify in bigger trees and avoids collisions. |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The new FindPython-based variant of the CMake scripts caches information
about the chosen Python version that can become stale. For example,
suppose I configure a simple pybind11-based project as follows
which will generate
my_extension.cpython-38-x86_64-linux-gnu.so.A subsequent change to the python version like
does not update all necessary build system information. In particular,
the compiled file is still called
my_extension.cpython-38-x86_64-linux-gnu.so.This commit fixes the problem by detecting changes in
Python_EXECUTABLEand re-running Python as needed.Note that the previous way of detecting Python does not seem to be
affected, it always specifies the right suffix.
Suggested changelog entry: