Make it possible to override PYTHON_VERSION when invoking cmake #1442
Conversation
mchukhrii
force-pushed
the
master
branch
9 times, most recently
from
21e1e17 to
89f31ba
Compare
| set(PYTHON_EXECUTABLE ${Python_EXECUTABLE}) | ||
| if (NOT NO_PYVERBS AND Python_VERSION_MAJOR EQUAL 3) | ||
| FIND_PACKAGE(PythonInterp REQUIRED) | ||
| FIND_PACKAGE(PythonLibs ${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR} EXACT) |
There was a problem hiding this comment.
This is quite different now, previously we only required the development packages if cmake was enabled, now it needs it always
There was a problem hiding this comment.
And this discards the preference to use python3, that doesn't seem right. If PYTHON_VERSION is not specified it should continue to do the automatic detection logic.
There was a problem hiding this comment.
find_package will not fail if PythonLibs is not found.
https://cmake.org/cmake/help/v3.11/command/find_package.html
but PYTHONLIBS_FOUND wont be set so i check for that
There was a problem hiding this comment.
PythonInterp already prefers 3 over 2. No need to make this explicit
| elseif (${CMAKE_VERSION} VERSION_GREATER "3.12") | ||
| set(Python_EXECUTABLE ${PYTHON_EXECUTABLE}) | ||
| FIND_PACKAGE(Python 3 REQUIRED COMPONENTS Interpreter OPTIONAL_COMPONENTS Development) | ||
| set(PYTHON_EXECUTABLE ${Python_EXECUTABLE}) |
There was a problem hiding this comment.
Both legs if this if should be setting this, right?
There was a problem hiding this comment.
PythonInterp sets PYTHON_EXECUTABLE
https://cmake.org/cmake/help/v3.11/module/FindPythonInterp.html
| FIND_PACKAGE(PythonLibs ${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR} EXACT) | ||
| elseif (${CMAKE_VERSION} VERSION_GREATER "3.12") | ||
| set(Python_EXECUTABLE ${PYTHON_EXECUTABLE}) | ||
| FIND_PACKAGE(Python 3 REQUIRED COMPONENTS Interpreter OPTIONAL_COMPONENTS Development) |
There was a problem hiding this comment.
What fails cmake if cython is enabled and we don't find Development?
There was a problem hiding this comment.
If find_package python find developer libs Python_Development_FOUND variable is set so i check for that
https://cmake.org/cmake/help/v3.13/module/FindPython.html#result-variables
|
Updated commit. Now should always prefer python3 |
CMakeLists.txt
Outdated
| if (NOT NO_PYVERBS AND Python_VERSION_MAJOR EQUAL 3) | ||
| FIND_PACKAGE(PythonInterp 3 REQUIRED) | ||
| FIND_PACKAGE(PythonLibs ${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR} EXACT) | ||
| elseif (${CMAKE_VERSION} VERSION_GREATER "3.12") |
There was a problem hiding this comment.
You're missing to handle 3.12:
VERSION_GREATER_EQUAL
Make it possible to override PYTHON_VERSION when invoking cmake. Signed-off-by: Maksym Chukhrii <mchukhrii@nvidia.com>
Currently cmake script will override PYTHON_EXECUTABLE if specified in command line.
This patch should fix it. Now if PYTHON_EXECUTABLE is not empty it will be used and python version discovery will not run.