Skip to content

Remove python_cmake_module from ros2.repos.#1524

Merged
clalancette merged 1 commit intorollingfrom
clalancette/remove-python-cmake-module
Oct 3, 2024
Merged

Remove python_cmake_module from ros2.repos.#1524
clalancette merged 1 commit intorollingfrom
clalancette/remove-python-cmake-module

Conversation

@clalancette
Copy link
Contributor

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 to find_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 sets Python3_FIND_STRATEGY to LOCATION. In short, this means that find_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 both python3.11 and python3 are on the PATH, it will by default choose python3.11. We can also control that behavior by setting Python3_FIND_UNVERSIONED_NAMES to FIRST.

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.

This was referenced Feb 15, 2024
@yashi
Copy link

yashi commented Feb 21, 2024

Just wanted to say that if you have libpython3.12-dev installed but your python3 is 3.11.8, the following two find different versions. This is the current situation on Debian Sid right now.

  • find_package(Python3 REQUIRED COMPONENTS Development)
  • find_package(Python3 REQUIRED COMPONENTS Interpreter)

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.

@clalancette
Copy link
Contributor Author

Just wanted to say that if you have libpython3.12-dev installed but your python3 is 3.11.8, the following two find different versions. This is the current situation on Debian Sid right now.

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>
@clalancette clalancette force-pushed the clalancette/remove-python-cmake-module branch from 146d424 to 940b0e8 Compare June 5, 2024 17:27
@clalancette
Copy link
Contributor Author

All right, I've now updated this series. Here's is CI again on all of this, including Windows Debug:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status
  • Windows Debug Build Status

@clalancette
Copy link
Contributor Author

clalancette commented Jun 26, 2024

I've updated the series again, and added in some more fixes for Windows Debug. Here's another Windows Debug run:

  • Windows Debug Build Status

@clalancette
Copy link
Contributor Author

clalancette commented Sep 21, 2024

Updated yet again, here is another CI run:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status
  • Windows Debug Build Status

@clalancette clalancette marked this pull request as ready for review October 3, 2024 18:20
@clalancette clalancette requested a review from codebot as a code owner October 3, 2024 18:20
@clalancette clalancette merged commit e1dbaf8 into rolling Oct 3, 2024
@clalancette clalancette deleted the clalancette/remove-python-cmake-module branch October 3, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants