Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cmake: python: Allow for explicit specification of a Python interpreter #56205

Closed
wants to merge 2 commits into from
Closed

cmake: python: Allow for explicit specification of a Python interpreter #56205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7ded78
cmake: Fix FindDtc usage of find_package_handle_standard_args
jackrosenthal Mar 24, 2023
9ead6fd
cmake: python: Allow for explicit specification of a Python interpreter
jackrosenthal Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmake/modules/FindDtc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
# 'DTC_VERSION_STRING'
# The version of devicetree compiler, dtc.

include(FindPackageHandleStandardArgs)

find_program(
DTC
dtc
Expand Down
74 changes: 45 additions & 29 deletions cmake/modules/python.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,51 @@ endif()

set(PYTHON_MINIMUM_REQUIRED 3.8)

# We are using foreach here, instead of find_program(PYTHON_EXECUTABLE_SYSTEM_DEFAULT "python" "python3")
# cause just using find_program directly could result in a python2.7 as python, and not finding a valid python3.
foreach(PYTHON_PREFER ${PYTHON_PREFER} ${WEST_PYTHON} "python" "python3")
find_program(PYTHON_PREFER_EXECUTABLE ${PYTHON_PREFER})
if(PYTHON_PREFER_EXECUTABLE)
execute_process (COMMAND "${PYTHON_PREFER_EXECUTABLE}" -c
"import sys; sys.stdout.write('.'.join([str(x) for x in sys.version_info[:2]]))"
RESULT_VARIABLE result
OUTPUT_VARIABLE version
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE)
function(python_get_version interpreter)
execute_process(COMMAND "${interpreter}" -c
"import sys; sys.stdout.write('.'.join([str(x) for x in sys.version_info[:2]]))"
RESULT_VARIABLE result
OUTPUT_VARIABLE version
ERROR_QUIET
OUTPUT_STRIP_TRAILING_WHITESPACE)
set(PYTHON_VERSION "${version}" PARENT_SCOPE)
endfunction()

if(version VERSION_LESS PYTHON_MINIMUM_REQUIRED)
set(PYTHON_PREFER_EXECUTABLE "PYTHON_PREFER_EXECUTABLE-NOTFOUND")
else()
set(PYTHON_MINIMUM_REQUIRED ${version})
set(PYTHON_EXACT EXACT)
# Python3_ROOT_DIR ensures that location will be preferred by FindPython3.
# On Linux, this has no impact as it will usually be /usr/bin
# but on Windows it solve issues when both 32 and 64 bit versions are
# installed, as version is not enough and FindPython3 might pick the
# version not on %PATH%. Setting Python3_ROOT_DIR ensures we are using
# the version we just tested.
get_filename_component(PYTHON_PATH ${PYTHON_PREFER_EXECUTABLE} DIRECTORY)
set(Python3_ROOT_DIR ${PYTHON_PATH})
break()
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)
Copy link
Collaborator

@tejlmand tejlmand Mar 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Issues:

python_get_version("${PYTHON_EXECUTABLE}")
if(PYTHON_VERSION VERSION_LESS PYTHON_MINIMUM_REQUIRED)
message(FATAL_ERROR
"Python executable ${PYTHON_EXECUTABLE} has version ${PYTHON_VERSION}, "
"however, the minimum required version is ${PYTHON_MINIMUM_REQUIRED}.")
endif()
endforeach()
set(Python3_EXECUTABLE "${PYTHON_EXECUTABLE}")
else()
# We are using foreach here, instead of find_program(PYTHON_EXECUTABLE_SYSTEM_DEFAULT "python" "python3")
# cause just using find_program directly could result in a python2.7 as python, and not finding a valid python3.
foreach(PYTHON_PREFER ${PYTHON_PREFER} ${WEST_PYTHON} "python" "python3")
find_program(PYTHON_PREFER_EXECUTABLE ${PYTHON_PREFER})
if(PYTHON_PREFER_EXECUTABLE)
python_get_version("${PYTHON_PREFER_EXECUTABLE}")
if(PYTHON_VERSION VERSION_LESS PYTHON_MINIMUM_REQUIRED)
set(PYTHON_PREFER_EXECUTABLE "PYTHON_PREFER_EXECUTABLE-NOTFOUND")
else()
set(PYTHON_MINIMUM_REQUIRED "${PYTHON_VERSION}")
set(PYTHON_EXACT EXACT)
# Python3_ROOT_DIR ensures that location will be preferred by FindPython3.
# On Linux, this has no impact as it will usually be /usr/bin
# but on Windows it solve issues when both 32 and 64 bit versions are
# installed, as version is not enough and FindPython3 might pick the
# version not on %PATH%. Setting Python3_ROOT_DIR ensures we are using
# the version we just tested.
get_filename_component(PYTHON_PATH ${PYTHON_PREFER_EXECUTABLE} DIRECTORY)
set(Python3_ROOT_DIR ${PYTHON_PATH})
break()
endif()
endif()
endforeach()

find_package(Python3 ${PYTHON_MINIMUM_REQUIRED} REQUIRED ${PYTHON_EXACT})
set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
find_package(Python3 ${PYTHON_MINIMUM_REQUIRED} REQUIRED ${PYTHON_EXACT})
set(PYTHON_EXECUTABLE "${Python3_EXECUTABLE}")
endif()