RoboStack / ros-humble

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

update ros idl generate-py patch #168

Closed wolfv closed 1 month ago

wolfv commented 1 month ago

The initial problem for me was that when performing a full rebuild, it complains later about not finding Python.h (when generating or depending on some messages, I think). Some CMake-export file does not properly expose the Python dependency in a later stage.

The following patch worked a little bit better when doing a local rebuild, but the Python demos actually crash with a segfault on macOS... I haven't investigated why but I remember that such a crash was previously a problem when linking Python, right?

Just for reference I copied a bit of CMake from the current main branch: https://github.com/ros2/rosidl_python/blob/rolling/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake

I can try to find some more minimal way of debugging this over the next days.

wolfv commented 1 month ago

I can try to use the entirety of this commit: https://github.com/ros2/rosidl_python/pull/140

Although I am not sure how applicable that is to ros-humble ...

Tobias-Fischer commented 1 month ago

I think this causes the issues on OSX: https://github.com/RoboStack/ros-humble/blob/2ac1e702ac4c96b3c62faafb4f59baad611467f9/patch/ros-humble-rosidl-generator-py.patch#L76C2-L80C83

We should only link to Python3::Python on non-osx platforms (not sure about numpy)

wolfv commented 1 month ago

I simplified the changes to just use

target_include_directories(${_target_name_lib} PUBLIC ${Python3_INCLUDE_DIRS} ${Python3_NumPy_INCLUDE_DIRS})

on osx ... I think that helped! At least it seems to work :)

Tobias-Fischer commented 1 month ago

Thanks a lot @wolfv. I'm happy with the change - could you please take a quick look too, @traversaro?

wolfv commented 1 month ago

I'll try this on Linux today, too, just to be sure.