RoboStack / ros-humble

Recipes for ROS 2 Humble Hawksbill
81 stars 32 forks source link

add additional patches for gcc 13 and fix rosidl py generator patch on linux #169

Closed wolfv closed 1 month ago

wolfv commented 1 month ago

I had to make the linkage PRIVATE to work on Linux (and not complain later on).

And I had to add 2x cstdint header because gcc 13 requires it.

tony-p commented 1 month ago

making the linkage public was needed to solve https://github.com/RoboStack/ros-humble/issues/119

I have a fix for the later packages in this branch https://github.com/tony-p/ros-humble/tree/fix/rebuild-regressions

Haven't opened a PR yet as a gazebo plugins package build was failing and hadn't yet had the chance to look into it/solve it

wolfv commented 1 month ago

I had the issue that with PUBLIC linkage downstream repos complained because of a missing find_package(Python3 ...). Do you know how to solve that, perhaps?

I took the changes (inspiration) from upstream, btw: https://github.com/ros2/rosidl_python/blob/rolling/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake

They recently (2 months ago) modernized this CMake.

wolfv commented 1 month ago

Any help and insights appreciated btw!

traversaro commented 1 month ago

making the linkage public was needed to solve #119

I have a fix for the later packages in this branch https://github.com/tony-p/ros-humble/tree/fix/rebuild-regressions

Haven't opened a PR yet as a gazebo plugins package build was failing and hadn't yet had the chance to look into it/solve it

It may be helpful also if you have a MRE (such as a small self-contained CMake project showing the problem) of the problem tracked in https://github.com/RoboStack/ros-humble/issues/119, so that we can easily check if the problem is solved or not.

wolfv commented 1 month ago

Upstream also uses this function: https://github.com/ros2/rosidl_python/blob/0497ce85b9df224085dd578eb0454aa56d1713b2/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake#L178-L180

Not sure if that changes much...

tony-p commented 1 month ago

The fix to find the python packages was in cmake/rosidl_generator_py_generate_interfaces.cmake add

find_package(rosidl_generator_py REQUIRED)
ament_export_dependencies(rosidl_generator_py)

as rosidl_generator_py includes a cmake extras file which includes the necessary find_package(Python .... and in this way it gets passed to all the downstream dependencies

https://github.com/tony-p/ros-humble/blob/fix/rebuild-regressions/patch/ros-humble-rosidl-generator-py.patch

tony-p commented 1 month ago

making the linkage public was needed to solve #119 I have a fix for the later packages in this branch https://github.com/tony-p/ros-humble/tree/fix/rebuild-regressions Haven't opened a PR yet as a gazebo plugins package build was failing and hadn't yet had the chance to look into it/solve it

It may be helpful also if you have a MRE (such as a small self-contained CMake project showing the problem) of the problem tracked in #119, so that we can easily check if the problem is solved or not.

The public version did solve (the building) for me and no longer need to manually link in the python files. Next step was to actually run nodes to ensure it still ran happily

I can look into making an MRE .... just juggling a few things at the moment

tony-p commented 1 month ago

The work around is a bit dirty, but the ament dependency at least makes sure it is applied recursively. I'm no cmake expert so tried to do the same using more native CMake for only the python libraries but quickly gave up.

wolfv commented 1 month ago

btw. related I also thought about adding tests in this repository, that would be added to the individual recipes (similar to patches). That way we could do some simple checks (e.g. starting the demos, importing rclpy etc.).