-
Notifications
You must be signed in to change notification settings - Fork 626
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
Remove python_cmake_module from ros2.repos. #1524
base: rolling
Are you sure you want to change the base?
Conversation
Just wanted to say that if you have
This leads to a build failure on some ROS related packages even with this PR applied. https://gist.github.com/yashi/0661b04106661e75b1749d3f057331e7 BTW, my intention isn't using Python 3.12. I wan't to use 3.11, which is the system default. But CMake finds the latest development module. |
Yeah, there is just nothing we can do about that. That is a bug in the distribution. |
With all of the other changes that have gone in, the core no longer requires it. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
146d424
to
940b0e8
Compare
This PR aims to remove
python_cmake_module
from the core ROS 2 repositories.The basic idea behind this is that
python_cmake_module
is an unnecessary indirection tofind_package(Python3)
, which is a standard CMake feature. So we may as well remove it and save that additional layer of indirection.While we are doing this, we also notice that we can support venvs in ROS 2 much better by taking advantage of some newer features of
find_package(Python3)
. In particular, we can set CMP0094, which setsPython3_FIND_STRATEGY
toLOCATION
. In short, this means thatfind_package(Python3)
will stop searching after it finds the first python3 binary it can.However,
find_package(Python3)
, at least on Linux, will attempt to find the most specific version it can by default. That means that if bothpython3.11
andpython3
are on the PATH, it will by default choosepython3.11
. We can also control that behavior by setting Python3_FIND_UNVERSIONED_NAMES toFIRST
.The end result of all of this is that venvs on Linux should work properly, as long as the venv is activated. On Windows, due to different logic, this always worked properly. If all else fails, the user can always pass
--cmake-args -DPython3_EXECUTABLE=/path/to/python3
, which will forcibly set the path to the python executable.Given the complexity of Python packaging, I don't expect it will fix all related problems, but it should improve many of our venv issues, like ros2/ros2cli#879 , ament/ament_cmake#502 , #1519 , #1094 , and #1430 .
Also note that this is not backportable to Humble, since it requires CMake 3.20 and Humble has to support CMake versions older than that. There we are using a simpler version of this, in ros2/geometry2#650 and ament/ament_cmake#507 . This might be backportable to Iron, but we'll have to see.
Note that this is a draft because we need to get all of the dependent PRs in first, and I still have to write up the documentation and CI code for this.