cmake: Simplify python finding #65995
Conversation
zephyrbot
requested review from
57300,
jeremybettis,
nashif,
nordicjm and
tejlmand
teburd
force-pushed
the
cmake_python_find
branch
2 times, most recently
from
c8b0059 to
9d58b7f
Compare
teburd
changed the title
| find_package(Deprecated COMPONENTS PYTHON_PREFER) | ||
|
|
||
| if(NOT DEFINED Python3_EXECUTABLE AND DEFINED WEST_PYTHON) | ||
| set(Python3_EXECUTABLE "${WEST_PYTHON}") |
There was a problem hiding this comment.
But you are changing this functionality which is described here: https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/modules/west.cmake#L11
There was a problem hiding this comment.
I think this is exactly the problem, as WEST_PYTHON doesn't seem to use the path to the symlinked python interpreter in my virtualenv?
Like if I were to keep the existing find python code, it finds not the virtualenv python but the actual installed python on my system, and this isn't correct!
Shouldn't this simply be finding the python that is my path not trying to do some odd "yeah but use the one west was called with" nonsense?
There was a problem hiding this comment.
Then you've installed west at a system level, you can't have it both ways, either install west and all zephyr dependencies at a system level, or install them at a virtual env level, not one at system and rest in virtual env.
There was a problem hiding this comment.
Both west and python are installed in the virtualenv, and no it doesn't choose the virtualenv python it uses the system python.
It does this because the static link is looked past in west.cmake I believe? Like with nix there's two levels of python environments basically... the actual interpreter, the interpret + python packages, then my virtualenv.
Without this patch I get...
(env) tburdick@zefire ~/z/z/doc (main)> make html-fast
make html DT_TURBO_MODE=1
make[1]: Entering directory '/home/tburdick/z/zephyr/doc'
cmake \
-GNinja \
-B_build \
-S. \
-DDOC_TAG=development \
-DSPHINXOPTS="-j auto -W --keep-going -T" \
-DSPHINXOPTS_EXTRA="" \
-DLATEXMKOPTS="-halt-on-error -no-shell-escape" \
-DDT_TURBO_MODE=1
Loading Zephyr module(s) (Zephyr base (cached)): doc
-- Found Python3: /nix/store/d9h22m8c7irvn866jmc6ilkk8j460bxg-python3-3.10.13-env/bin/python (found suitable version "3.10.13", minimum required is "3.8") found components: Interpreter
-- Cache files will be written to: /home/tburdick/.cache/zephyr
-- Zephyr base: /home/tburdick/z/zephyr
-- Could NOT find LATEX (missing: LATEX_COMPILER PDFLATEX)
CMake Warning at CMakeLists.txt:39 (message):
LaTeX components not found. PDF build will not be available.
-- Configuring done
-- Generating done
-- Build files have been written to: /home/tburdick/z/zephyr/doc/_build
cmake --build _build --target html
[0/2] Generating Devicetree bindings documentation...
Traceback (most recent call last):
File "/home/tburdick/z/zephyr/doc/_scripts/gen_devicetree_rest.py", line 21, in <module>
from devicetree import edtlib
File "/home/tburdick/z/zephyr/scripts/dts/python-devicetree/src/devicetree/edtlib.py", line 79, in <module>
import yaml
ModuleNotFoundError: No module named 'yaml'
FAILED: CMakeFiles/devicetree /home/tburdick/z/zephyr/doc/_build/CMakeFiles/devicetree
cd /home/tburdick/z/zephyr/doc/_build && /nix/store/h8q4r7f1j906rlffj96dlbbmwc9zbirk-cmake-3.25.3/bin/cmake -E env PYTHONPATH=/home/tburdick/z/zephyr/scripts/dts/python-devicetree/src: ZEPHYR_BASE=/home/tburdick/z/zephyr /nix/store/d9h22m8c7irvn866jmc6ilkk8j460bxg-python3-3.10.13-env/bin/python /home/tburdick/z/zephyr/doc/_scripts/gen_devicetree_rest.py --vendor-prefixes /home/tburdick/z/zephyr/dts/bindings/vendor-prefixes.txt --dts-root /home/tburdick/z/zephyr --turbo-mode /home/tburdick/z/zephyr/doc/_build/src/build/dts/api
ninja: build stopped: subcommand failed.
make[1]: *** [Makefile:21: html] Error 1
make[1]: Leaving directory '/home/tburdick/z/zephyr/doc'
make: *** [Makefile:18: html-fast] Error 2
(env) tburdick@zefire ~/z/z/doc (main) [2]> which python
/home/tburdick/z/env/bin/python
(env) tburdick@zefire ~/z/z/doc (main)> which west
/home/tburdick/z/env/bin/westWith this patch cmake correctly chooses my virtualenv python as the executable.
Hard to understand why any of this cmake to root out a different python interpreter is even needed, if I have python in my path it should be choosing the first python in my path not go spelunking into the static links looking for something.
There was a problem hiding this comment.
Without this patch I get...
@teburd could you move this data to a new bug?
EDIT:
Hard to understand why any of this cmake to root out a different python interpreter is even needed,
Isn't the answer in your commit message? "Now that the minimum version required is 3.20,..."
There was a problem hiding this comment.
I'd be happy with an explanation as to why west.cmake tries to find the real python elf. I couldn't find one in the git history or file comments.
Python is also broken on archlinux for me and has been for a long time, which is why I ended up using nix in the first place. Because I was tired of seeing some python dep suddenly breaking when python was updated on there.
Current situation on my arch machine without this patch..
(py3) ~/z/z/doc ❯❯❯ make html-fast
make html DT_TURBO_MODE=1
make[1]: Entering directory '/home/tburdick/zephyrtrees/zephyr/doc'
cmake \
-GNinja \
-B_build \
-S. \
-DDOC_TAG=development \
-DSPHINXOPTS="-j auto -W --keep-going -T" \
-DSPHINXOPTS_EXTRA="" \
-DLATEXMKOPTS="-halt-on-error -no-shell-escape" \
-DDT_TURBO_MODE=1
Loading Zephyr module(s) (Zephyr base (cached)): doc
-- Cache files will be written to: /home/tburdick/.cache/zephyr
-- Found west (found suitable version "1.2.0", minimum required is "1.0.0")
-- Zephyr base: /home/tburdick/zephyrtrees/zephyr
CMake Warning at CMakeLists.txt:39 (message):
LaTeX components not found. PDF build will not be available.
-- Configuring done (0.7s)
-- Generating done (0.0s)
-- Build files have been written to: /home/tburdick/zephyrtrees/zephyr/doc/_build
cmake --build _build --target html
[0/2] Generating Devicetree bindings documentation...
[1/2] Running Sphinx HTML build...
Running Sphinx v7.2.6
Traceback (most recent call last):
File "/usr/lib/python3.11/site-packages/sphinx/registry.py", line 447, in load_extension
mod = import_module(extname)
^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/importlib/__init__.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
File "<frozen importlib._bootstrap>", line 1140, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'breathe'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/lib/python3.11/site-packages/sphinx/cmd/build.py", line 293, in build_main
app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/site-packages/sphinx/application.py", line 233, in __init__
self.setup_extension(extension)
File "/usr/lib/python3.11/site-packages/sphinx/application.py", line 406, in setup_extension
self.registry.load_extension(self, extname)
File "/usr/lib/python3.11/site-packages/sphinx/registry.py", line 450, in load_extension
raise ExtensionError(__('Could not import extension %s') % extname,
sphinx.errors.ExtensionError: Could not import extension breathe (exception: No module named 'breathe')
Extension error:
Could not import extension breathe (exception: No module named 'breathe')
FAILED: CMakeFiles/html /home/tburdick/zephyrtrees/zephyr/doc/_build/CMakeFiles/html
cd /home/tburdick/zephyrtrees/zephyr/doc/_build && /usr/bin/cmake -E env DOXYGEN_EXECUTABLE=/usr/bin/doxygen DOT_EXECUTABLE=/usr/bin/dot /usr/bin/sphinx-build -b html -c /home/tburdick/zephyrtrees/zephyr/doc -d /home/tburdick/zephyrtrees/zephyr/doc/_build/doctrees -w /home/tburdick/zephyrtrees/zephyr/doc/_build/html.log -t development -j auto -W --keep-going -T /home/tburdick/zephyrtrees/zephyr/doc/_build/src /home/tburdick/zephyrtrees/zephyr/doc/_build/html
ninja: build stopped: subcommand failed.
make[1]: *** [Makefile:21: html] Error 1
make[1]: Leaving directory '/home/tburdick/zephyrtrees/zephyr/doc'
make: *** [Makefile:18: html-fast] Error 2
(py3) ~/z/z/doc ❯❯❯ python ✘ 2
Python 3.11.6 (main, Nov 14 2023, 09:36:21) [GCC 13.2.1 20230801] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import breathe
>>>
KeyboardInterrupt
>>>
(py3) ~/z/z/doc ❯❯❯ which python
/home/tburdick/py3/bin/python
(py3) ~/z/z/doc ❯❯❯ which pip
/home/tburdick/py3/bin/pip
(py3) ~/z/z/doc ❯❯❯ uname -a
Linux saturn 6.4.11-arch2-1 #1 SMP PREEMPT_DYNAMIC Sat, 19 Aug 2023 15:38:34 +0000 x86_64 GNU/LinuxWith this patch...
(py3) ~/z/z/doc ❯❯❯ make html-fast
make html DT_TURBO_MODE=1
make[1]: Entering directory '/home/tburdick/zephyrtrees/zephyr/doc'
cmake \
-GNinja \
-B_build \
-S. \
-DDOC_TAG=development \
-DSPHINXOPTS="-j auto -W --keep-going -T" \
-DSPHINXOPTS_EXTRA="" \
-DLATEXMKOPTS="-halt-on-error -no-shell-escape" \
-DDT_TURBO_MODE=1
Loading Zephyr module(s) (Zephyr base (cached)): doc
-- Found Python: /home/tburdick/py3/bin/python3.11 (found suitable version "3.11.6", required range is "3.8...3.12") found components: Interpreter
-- Cache files will be written to: /home/tburdick/.cache/zephyr
-- Found west (found suitable version "1.2.0", minimum required is "1.0.0")
-- Zephyr base: /home/tburdick/zephyrtrees/zephyr
CMake Warning at CMakeLists.txt:39 (message):
LaTeX components not found. PDF build will not be available.
-- Configuring done (0.7s)
-- Generating done (0.0s)
-- Build files have been written to: /home/tburdick/zephyrtrees/zephyr/doc/_build
cmake --build _build --target html
[0/2] Generating Devicetree bindings documentation...
[1/2] Running Sphinx HTML build...
Running Sphinx v6.2.1
making output directory... done
Building Kconfig database...... done
Preparing Doxyfile...
Checking if Doxygen needs to be run...
Running Doxygen...
Syncing Doxygen output...
building [mo]: targets for 0 po files that are out of date
writing output...
building [html]: targets for 1 source files that are out of date
updating environment: [new config] 1603 added, 0 changed, 0 removed
reading sources... [ 34%] boards/shields/mikroe_adc_click/doc/index .. boards/xtensa/m5stickc_plus/doc/index So no its not just nix actually
There was a problem hiding this comment.
Python is also broken on archlinux for me and has been for a long time, which is why I ended up using nix in the first place. Because I was tired of seeing some python dep suddenly break when python was updated on there.
This would only be the case if you did something like ignore this warning:
｢1 ~｣$ pip install blah
error: externally-managed-environment
× This environment is externally managed
╰─> To install Python packages system-wide, try 'pacman -S
python-xyz', where xyz is the package you are trying to
install.
If you wish to install a non-Arch-packaged Python package,
create a virtual environment using 'python -m venv path/to/venv'.
Then use path/to/venv/bin/python and path/to/venv/bin/pip.
If you wish to install a non-Arch packaged Python application,
it may be easiest to use 'pipx install xyz', which will manage a
virtual environment for you. Make sure you have python-pipx
installed via pacman.
note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.
If you ignore it, you do so at your own risk, which may or may not work for you (personally for me, it's been working fine on the same VM for years)
There was a problem hiding this comment.
I never saw a warning like the above, but I guess there's no argument convincing enough here for you.
As I said, I'd be happy with an explanation as to why west.cmake code in particular does what it does. The git history and file have no clues for me.
There was a problem hiding this comment.
I'd be happy with an explanation as to why west.cmake tries to find the real python elf. I couldn't find one in the git history or file comments.
Did you look here: 9284e60
or here: #34368
Generally speaking, the CMake find python handling has always been broken when having more than just a single python interpreter installed.
If not using venv and having multiple Pythons installed, but only the west package / other Zephyr required pip-packages in one of them can give strange results if using the plain CMake FindPython3 mechanism.
That said, CMake's FindPython3 has improved significantly over the years, and is quite good nowadays.
Also we have a requirement / expectation that if west build is used for building Zephyr, then we want to use that exact same Python interpreter that was used for invoking west.
A bit of some history to check up on, to see what we have tried to handle:
Please take a look at the history, some PRs and issues (note, the list is not exhaustive):
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
There was a problem hiding this comment.
Noticing this:
(env) tburdick@zefire ~/z/z/doc (main)> make html-fast
is this only a problem when building the docs or also in general ?
Cause in this particular case then west is not used to invoke the build and hence the WEST_PYTHON is not set.
Did you try to set Python3_EXECUTABLE pointing directly to the interpreter you want ?
(not because that's what you should generally do, but in order to help further investigations)
|
Labeled as a bug because to me, this is fixing a bug with the way cmake is trying to find python. |
If you build using
|
There was a problem hiding this comment.
The current system seems to always find the wrong interpreter for me, and cmake (since 3.18) has a sensible FindPython module with the potential to use a version range.
Having Zephyr to pick the Python interpreter expected by the user has always been an uphill battle.
- Some users expect the Python3 in path
- Some users expect the latest Python3 when multiple are installed
- Some expects the same interpreter the
west builduses (even if it is not in path) - venv is another story
- Windows users
- etc.
So please open an issue on you expectations and why you'd expect that Python to be picked up together with info on which python is actually picked up.
Then we can start a discussion and try to understand why things are not behaving as expected.
A brief history of Python struggle:
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
cmake/modules/python.cmake
Outdated
| endif() | ||
|
|
||
| find_package(Python3 ${PYTHON_MINIMUM_REQUIRED} REQUIRED) | ||
| find_package(Python 3.8...3.12 COMPONENTS Interpreter) |
There was a problem hiding this comment.
as minimum, please verify that this is not reintroduced: #24340
as well as verifying the different scenarios described in this PR are correctly handled: #24364
Just because this PR works in your environment doesn;t mean it work across various environments, and we need to be a bit careful here.
But would be wonderful if later CMake version have improved in this regard.
There was a problem hiding this comment.
just verified myself, with CMake 3.26, then the issue reported in #24340 still exists.
I have Python 3.8 in path but have both 3.8 and 3.11 installed.
Zephyr picks up 3.8 (as expected because it meets the requirements) but TF-M (using vanilla FindPython3) picks up 3.11, which may lead to other surprises.
See also: #66219
and: https://discord.com/channels/720317445772017664/733037890514321419/1181702822245437510
|
Hi @tejlmand thank you for linking various issues describing the python.cmake behavior, that is very helpful indeed. It's pretty clear that finding python (you would think this would be an incredibly simple task of picking the first one in a users path?) is somehow an incredibly bug ridden and painful task. I believe I found the culprit, at least for why on NixOS unexpected things happen, and have described in details in the linked issue here. I'm still investigating why this fails on archlinux as it seems to be an entirely different issue as why it fails on nix. On archlinux it appears to try and use sphinx from my python-sphinx archlinux package and not the one in the virtualenv. Entirely unexpected and highly confusing. |
teburd
force-pushed
the
cmake_python_find
branch
from
9d58b7f to
3210957
Compare
|
@tejlmand I don't have a windows machine with the problem mentioned, but perhaps the strategy and windows registry options are of use here to avoid the issue on windows with multiple interpreters. |
teburd
force-pushed
the
cmake_python_find
branch
from
3210957 to
060b7a5
Compare
|
@tejlmand setting Python3_EXECUTABLE doesn't play well seemingly with twister, setting as an env var doesn't seem to be accounted for, so I'm currently carrying around this patch on every branch I'm working on. Since all the weird python finding logic seems tied to quirks on a particular platform, what do you think about using find_package here for linux users where it works and using the goofy python finding logic on windows where its seemingly broken? |
teburd
force-pushed
the
cmake_python_find
branch
from
060b7a5 to
a0f81a5
Compare
|
Just found this by chance, can't resist sharing it: https://yeray.dev/python/direnv-asdf-python-virtualenv
Irrelevant context: https://discuss.python.org/t/official-zephyr-project-getting-desperate-with-2-months-old-pep-541-name-claim/24431 |
This is I guess part of my point here, cmake's own python module works just fine. Our custom finder does not. If you look at the cmake python module there is a lot of work done to handle this already.
|
And if/when it does not in some edge cases, maybe a nice Zephyr error message when Python is not found could recommend users upgrade CMake first and then point them at "cmake.org/issues" (among other hints). CMake is a very active and well maintained project that cares a great deal about backwards compatibility. I understand Python is absolutely critical to Zephyr and that CMake was falling short. But CMake has made progress so maybe it's time to move on now and stop duplicating (parts of) CMake in Zephyr? At least for Linux users who seem to have fewer edge cases and could be used as earlier CMake adopters. |
|
I'm kind of curious if this works for anyone that is using venv? I'm running nixos, and I setup a venv to hold the python packages needed for Zephyr. Normally, to use the venv, it needs to pull the python interpreter in the path, which is the venv wrapper. But we seem to be resolving the symlinks that venv sets up, and then the venv packages aren't in the path. So, please leave this open, and know that there is at least one other user that has to cherry pick this change to be able to build things. |
lets open a blocker bug for this. Not being able have a working a development environment without local hack is extreme, and I do not see how satisfying the needs of one external package trumps general user needs. |
The current system seems to always find the wrong interpret for me, and cmake (since 3.18) has a sensible FindPython module with the potential to use a version range. So take advantage of both of those as our current minimum cmake version is now 3.20. This only works on non-windows platforms however as on windows platforms multiple python installs may still result in erroneous python interpreters being found, though the reasoning is a bit vague to me as this would happen only when PATH has multiple interpreters with the one in the path being selected over all others, or with a set of python interpreters in the register being looked at (which can be disabled). Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
teburd
force-pushed
the
cmake_python_find
branch
from
a0f81a5 to
52f72c4
Compare
|
Updated to only affect non-windows platforms where presumably cmake's find python will do the right thing. |
I don't see this having been done |
I can copy what I've written in this PR to an issue, I'll mark it as a blocker for me as suggest by Anas |
I had done this actually #66236 but it had been so long ago I had forgotten, will link the duplicate #70258 now I guess. I think the updated issue better explains things anyways with less verbage. |
yes, for your particular use-case, but there are many other valid use-cases where the CMake's own python module does not work: so the use-cases above must continue to work. |
@nashif This is not a blocker. It happens only with
There is nothing extreme in having to specify |
|
@teburd still needed? |
The current system seems to always find the wrong interpreter for me, and cmake (since 3.18) has a sensible FindPython module with the potential to use a version range.
So take advantage of both of those as our current minimum cmake version is now 3.20.