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.

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

Python detection macro in cmake fails to detect highest installed version #24252

Closed
nashif opened this issue Apr 9, 2020 · 4 comments
Closed
Assignees
Labels
area: Build System bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@nashif
Copy link
Member

nashif commented Apr 9, 2020

Describe the bug

Python detection fails to detect highest installed version, instead it find a compatibility and older version installed.

To Reproduce

Including boilerplate (Zephyr base): /home/nashif/Work/zephyrproject/zephyr/cmake/app/boilerplate.cmake
-- Application: /home/nashif/Work/zephyrproject/zephyr/samples/hello_world
-- Zephyr version: 2.2.99 (/home/nashif/Work/zephyrproject/zephyr)
-- Found PythonInterp: /usr/bin/python3.6 (found suitable version "3.6.10", minimum required is "3.6")
-- Board: nsim_em
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'west.version'
CMake Error at /home/nashif/Work/zephyrproject/zephyr/cmake/host-tools.cmake:26 (message):
  Unable to import west.version from '/usr/bin/python3.6'
Call Stack (most recent call first):
  /home/nashif/Work/zephyrproject/zephyr/cmake/app/boilerplate.cmake:483 (include)
  /home/nashif/Work/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:24 (include)
  /home/nashif/Work/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:35 (include_boilerplate)
  CMakeLists.txt:5 (find_package)

However, I have 3.7.x

i9:zephyr(master): python3 --version
Python 3.7.6

Expected behavior

cmake should find the highest version installed.

Impact

Failure to build

Additional context

Looking at cmake/python.cmake

# The 'FindPythonInterp' that is distributed with CMake 3.8 has a bug
# that we need to work around until we upgrade to 3.13. Until then we
# maintain a patched copy in our repo. Bug:
# https://github.com/zephyrproject-rtos/zephyr/issues/11103
set(PythonInterp_FIND_VERSION 3.6)
set(PythonInterp_FIND_VERSION_COUNT 2)
set(PythonInterp_FIND_VERSION_MAJOR 3)
set(PythonInterp_FIND_VERSION_MINOR 6)
set(PythonInterp_FIND_VERSION_EXACT 0)
set(PythonInterp_FIND_REQUIRED 1)
include(${ZEPHYR_BASE}/cmake/backports/FindPythonInterp.cmake)

That text seems to be outdated and needs to be updated. we already use 3.13, is all of this still needed?

If I make this change:

diff --git a/cmake/python.cmake b/cmake/python.cmake
index 156bbc921c..cdd1025d2a 100644
--- a/cmake/python.cmake
+++ b/cmake/python.cmake
@@ -16,7 +16,7 @@ endif()
 set(PythonInterp_FIND_VERSION 3.6)
 set(PythonInterp_FIND_VERSION_COUNT 2)
 set(PythonInterp_FIND_VERSION_MAJOR 3)
-set(PythonInterp_FIND_VERSION_MINOR 6)
-set(PythonInterp_FIND_VERSION_EXACT 0)
+#set(PythonInterp_FIND_VERSION_MINOR 6)
+#set(PythonInterp_FIND_VERSION_EXACT 0)
 set(PythonInterp_FIND_REQUIRED 1)
 include(${ZEPHYR_BASE}/cmake/backports/FindPythonInterp.cmake)

I get the expected behavior:

-- west build: generating a build system
Including boilerplate (Zephyr base): /home/nashif/Work/zephyrproject/zephyr/cmake/app/boilerplate.cmake
-- Application: /home/nashif/Work/zephyrproject/zephyr/samples/hello_world
-- Zephyr version: 2.2.99 (/home/nashif/Work/zephyrproject/zephyr)
-- Found PythonInterp: /usr/bin/python3 (found suitable version "3.7.6", minimum required is "3.6")
-- Board: nsim_em
-- Found west: /home/nashif/.local/bin/west (found suitable version "0.7.2", minimum required is "0.7.1")
-- Found toolchain: zephyr (/home/nashif/Work/SDK/0.11.2)
-- Found BOARD.dts: /home/nashif/Work/zephyrproject/zephyr/boards/arc/nsim/nsim_em.dts
-- Generated zephyr.dts: /home/nashif/Work/zephyrproject/zephyr/build/zephyr/zephyr.dts
@nashif nashif added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug area: Build System labels Apr 9, 2020
nashif added a commit to nashif/zephyr that referenced this issue Apr 11, 2020
Macro for python detection was taking old version installed for
compatability (needed by the zephyr SDK for gdb). Make the macro detect
latest version installed instead.

Fixes zephyrproject-rtos#24252.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
@mbolivar-nordic
Copy link
Contributor

That text seems to be outdated and needs to be updated. we already use 3.13, is all of this still needed?

I think no. This backport is not necessary. And in fact FindPythonInterp is deprecated since CMake 3.12:

https://cmake.org/cmake/help/v3.12/module/FindPythonInterp.html

We should be using this instead:

https://cmake.org/cmake/help/v3.13/module/FindPython3.html#module:FindPython3

@tejlmand
Copy link
Collaborator

tejlmand commented Apr 11, 2020

@nashif already looking at this for another reason, see this: #23981 (comment)

We might be able to completely remove Zephyr local copy for finding ZephyrInterp

@tejlmand
Copy link
Collaborator

Seems this bug has been re-introduced #11103 by this commit a5eb70f 😞

-- Zephyr version: 2.2.99 (C:/projects/github/ncs/zephyr)
CMake Error at C:/Program Files/CMake/share/cmake-3.14/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find PythonInterp: Found unsuitable version "2.7.9", but required
  is at least "3.6" (found C:/Python27/python.exe)
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.14/Modules/FindPackageHandleStandardArgs.cmake:376 (_FPHSA_FAILURE_MESSAGE)
  C:/projects/github/ncs/zephyr/cmake/backports/FindPythonInterp.cmake:174 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  C:/projects/github/ncs/zephyr/cmake/python.cmake:22 (include)
  C:/projects/github/ncs/zephyr/cmake/app/boilerplate.cmake:115 (include)
  C:/projects/github/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:24 (include)
  C:/projects/github/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:35 (include_boilerplate)
  CMakeLists.txt:5 (find_package)

going to ensure #11103 is fixed again, and check the following issues are still fixed when switching to CMake built-in Python search.
#12066
#20303

@tejlmand
Copy link
Collaborator

tejlmand commented Apr 13, 2020

Expected behavior

cmake should find the highest version installed.

@nashif actually, the current design is made so it should choose the version first in path, which was even an issue raised by you, #11857
PR: #11907.

In my opinion, CMake should choose:

  1. The python 3.x in path that meets the minimum required Python3 version
    a. If multiple Python 3.x is in path, choose the highest, for example
    - /usr/bin/python3
    - /usr/bin/python3.6
    - /usr/bin/python3.7
    - /usr/bin/python3.8
    Should choose 3.8 (Note, this is not applicable on windows, where python.exe is the name for all versions)
  1. In no Python 3.x in path, then select highest installed version that is detected

tejlmand added a commit to tejlmand/zephyr that referenced this issue Apr 13, 2020
Fixes: zephyrproject-rtos#23922, zephyrproject-rtos#24252, zephyrproject-rtos#11103

This commit switches to use the find_package(Python3) introduced with
CMake 3.12.

This removes the need for a Zephyr backport of Python detection module.

The search order for Python3 is following CMake search order and can be
controlled through CMake flags (See CMake documentation).

Default it will use the Python version found in PATH.
If multiple Python3 versions are found in PATH, the newest version will
be selected (Can be controlled using `Python3_FIND_STRATEGY`)

Using find_package(Python3) also ensures Python2.7 will never be
selected, issue zephyrproject-rtos#11103, which was re-introduced in Zephyr.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
nashif pushed a commit that referenced this issue Apr 13, 2020
Fixes: #23922, #24252, #11103

This commit switches to use the find_package(Python3) introduced with
CMake 3.12.

This removes the need for a Zephyr backport of Python detection module.

The search order for Python3 is following CMake search order and can be
controlled through CMake flags (See CMake documentation).

Default it will use the Python version found in PATH.
If multiple Python3 versions are found in PATH, the newest version will
be selected (Can be controlled using `Python3_FIND_STRATEGY`)

Using find_package(Python3) also ensures Python2.7 will never be
selected, issue #11103, which was re-introduced in Zephyr.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
avisconti pushed a commit to avisconti/zephyr that referenced this issue Apr 15, 2020
Fixes: zephyrproject-rtos#23922, zephyrproject-rtos#24252, zephyrproject-rtos#11103

This commit switches to use the find_package(Python3) introduced with
CMake 3.12.

This removes the need for a Zephyr backport of Python detection module.

The search order for Python3 is following CMake search order and can be
controlled through CMake flags (See CMake documentation).

Default it will use the Python version found in PATH.
If multiple Python3 versions are found in PATH, the newest version will
be selected (Can be controlled using `Python3_FIND_STRATEGY`)

Using find_package(Python3) also ensures Python2.7 will never be
selected, issue zephyrproject-rtos#11103, which was re-introduced in Zephyr.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
hakehuang pushed a commit to hakehuang/zephyr that referenced this issue Jun 20, 2020
Fixes: zephyrproject-rtos#23922, zephyrproject-rtos#24252, zephyrproject-rtos#11103

This commit switches to use the find_package(Python3) introduced with
CMake 3.12.

This removes the need for a Zephyr backport of Python detection module.

The search order for Python3 is following CMake search order and can be
controlled through CMake flags (See CMake documentation).

Default it will use the Python version found in PATH.
If multiple Python3 versions are found in PATH, the newest version will
be selected (Can be controlled using `Python3_FIND_STRATEGY`)

Using find_package(Python3) also ensures Python2.7 will never be
selected, issue zephyrproject-rtos#11103, which was re-introduced in Zephyr.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants