cmake: python: Allow for explicit specification of a Python interpreter #56205
Conversation
jackrosenthal
commented
Mar 24, 2023
jackrosenthal
commented
find_package_handle_standard_args is declared in the builtin package FindPackageHandleStandardArgs, but since we don't explicitly include that here, we may not have the function available yet. Include what we use =) Signed-off-by: Jack Rosenthal <[email protected]>
Setting PYTHON_PREFER may not select the desired interpreter under all circumstances. find_package(Python3 ... EXACT) will try to find the Python interpreter of the desired version, but will not provide the correct interpreter if it's not in PATH, or if multiple interpreters exist of the same major+minor version in the PATH. Let's add an option for a user to explicitly specify PYTHON_EXECUTABLE, not just a preference. The build system will then try that executable, and only that executable, and if it doesn't meet the version requirement, it'll error out instead of keep searching. Signed-off-by: Jack Rosenthal <[email protected]>
| endif() | ||
| # If we're told exactly which Python executable to use, we should respect that, | ||
| # instead of using the default matching process below. | ||
| if(DEFINED PYTHON_EXECUTABLE) |
There was a problem hiding this comment.
back in the rabbit hole.
Zephyr has been struggling with inconsistent CMake behavior on the Python lookup as well as a request to use the same Python interpreter as west whenever west build is invoked.
Part of this work resulted in PYTHON_PREFER, both to ensure that Zephyr would keep looking for a better alternative, but also because PYTHON_EXECUTABLE as input was not supported until CMake >= 3.16:
New in version 3.16.
To solve special cases, it is possible to specify directly the artifacts by setting the following variables:
Python_EXECUTABLE
The path to the interpreter.
Ref: https://cmake.org/cmake/help/latest/module/FindPython.html#artifacts-specification
Now minimum CMake required by Zephyr is 3.20, I would therefore like to see this code be cleaned up instead of more patching.
Note. if you set PYTHON_PREFER to absolute path, such as PYTHON_PREFER=/use/this/python, then it should behave identical to PYTHON_EXECUTABLE (which couldn't be used as input back then).
Only difference would be the FATAL_ERROR, and that could be done without all the additional changes.
Thus, a request to see if PYTHON_PREFER can be deprecated in favor of PYTHON_EXECUTABLE, and thereby simplify the code.
Please take a look at the history, some PRs and issues:
PRs:
- cmake: now using find_package(python3) from CMake to located python3 #24308
- cmake: use WEST_PYTHON as a preferred python installation if defined. #34479
Issues:
- Cmake's Python path breaks after using west build --pristine #34368
- #24308 Broke python3 interpreter selection #24340
- cmake 3.17 dev warning from FindPythonInterp.cmake #23922
- Python detection macro in cmake fails to detect highest installed version #24252
- The wrong Python version can be detected on Windows #11103
… interpreter Setting PYTHON_PREFER may not select the desired interpreter under all circumstances. find_package(Python3 ... EXACT) will try to find the Python interpreter of the desired version, but will not provide the correct interpreter if it's not in PATH, or if multiple interpreters exist of the same major+minor version in the PATH. Let's add an option for a user to explicitly specify PYTHON_EXECUTABLE, not just a preference. The build system will then try that executable, and only that executable, and if it doesn't meet the version requirement, it'll error out instead of keep searching. FROMPULL request: zephyrproject-rtos/zephyr#56205 BUG=b:269281716 TEST=build with bazel Change-Id: I26dee721ebc16bd4e9cbd49cb48c170d7a8ab1eb Signed-off-by: Jack Rosenthal <[email protected]> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/4615502 Reviewed-by: Aaron Massey <[email protected]>
find_package_handle_standard_args is declared in the builtin package FindPackageHandleStandardArgs, but since we don't explicitly include that here, we may not have the function available yet. Include what we use =) zephyrproject-rtos/zephyr#56205 Change-Id: I43a49edd40cf7e3eacc04e42350857b812561b61 Signed-off-by: Jack Rosenthal <[email protected]> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/4616554 Reviewed-by: Aaron Massey <[email protected]> Commit-Queue: Aaron Massey <[email protected]>
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
As no further progress has been made on this PR, I have posted a cleanup here: #59811 |
