Stop linking the Python bindings library into C++ consumers of interface packages (fixes dyld _PyExc_RuntimeError / ros-kilted#76)

.github/workflows/testpr.yml

@@ -126,6 +126,7 @@ jobs:
         # You can uncomment/modify the line below in case the cache for some packages becomes corrupted and needs to be regenerated.
         # Make sure you don't merge these changes, though!
         # rm -rf ${{ matrix.folder_cache }}/ros-rolling-moveit-core* ${{ matrix.folder_cache }}/ros2-moveit-core* 2>/dev/null || true
+        rm -rf ${{ matrix.folder_cache }}/ros-rolling-std-msgs* ${{ matrix.folder_cache }}/ros2-std-msgs* 2>/dev/null || true
         mkdir -p ${{ matrix.folder_cache }}
         pixi run rattler-index fs ${{ matrix.folder_cache }}/.. --force
 

patch/ros-rolling-rosidl-generator-py.patch

@@ -0,0 +1,178 @@
+diff --git a/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake b/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake
+index 2fe245b..f46b674 100644
+--- a/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake
++++ b/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake
+@@ -152,8 +152,14 @@ set_property(
+   ${_generated_extension_files} ${_generated_py_files} ${_generated_c_files}
+   PROPERTY GENERATED 1)
+
++# The generated C conversion functions are compiled into each Python extension
++# module below, rather than into a shared library: they use CPython symbols
++# that only a Python interpreter provides. Conversion functions of types from
++# other packages are looked up from those packages' Python message classes at
++# runtime, so no library needs to be exported or linked across packages.
+ set(_target_name_lib "${rosidl_generate_interfaces_TARGET}__rosidl_generator_py")
+-add_library(${_target_name_lib} SHARED ${_generated_c_files})
++add_library(${_target_name_lib} OBJECT ${_generated_c_files})
++set_target_properties(${_target_name_lib} PROPERTIES POSITION_INDEPENDENT_CODE ON)
+ target_link_libraries(${_target_name_lib} PRIVATE
+   ${rosidl_generate_interfaces_TARGET}__rosidl_generator_c)
+ add_dependencies(
+@@ -165,7 +171,7 @@ add_dependencies(
+ target_link_libraries(
+   ${_target_name_lib} PRIVATE
+   Python3::NumPy
+-  Python3::Python
++  Python3::Module
+ )
+ target_include_directories(${_target_name_lib}
+   PRIVATE
+@@ -242,26 +248,8 @@ foreach(_typesupport_impl ${_typesupport_impls})
+   endif()
+ endforeach()
+
+-
+-# Depend on rosidl_generator_py generated targets from our dependencies
+-foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES})
+-  target_link_libraries(${_target_name_lib} PRIVATE ${${_pkg_name}_TARGETS${rosidl_generator_py_suffix}})
+-endforeach()
+-
+ set_target_properties(${_target_name_lib} PROPERTIES COMPILE_OPTIONS "${_extension_compile_flags}")
+
+-if(NOT rosidl_generate_interfaces_SKIP_INSTALL)
+-  install(TARGETS ${_target_name_lib}
+-    EXPORT export_${_target_name_lib}
+-    ARCHIVE DESTINATION lib
+-    LIBRARY DESTINATION lib
+-    RUNTIME DESTINATION bin)
+-
+-  # Export this target so downstream interface packages can depend on it
+-  rosidl_export_typesupport_targets("${rosidl_generator_py_suffix}" "${_target_name_lib}")
+-  ament_export_targets(export_${_target_name_lib})
+-endif()
+-
+ if(BUILD_TESTING AND rosidl_generate_interfaces_ADD_LINTER_TESTS)
+   if(
+     NOT _generated_py_files STREQUAL "" OR
+diff --git a/rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em b/rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em
+index d234915..6d1bef1 100644
+--- a/rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em
++++ b/rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em
+@@ -48,9 +48,7 @@ static void @('__'.join(message.structure.namespaced_type.namespaces + [module_n
+   @(msg_typename)__destroy(ros_message);
+ }
+
+-ROSIDL_GENERATOR_C_IMPORT
+ bool @('__'.join(message.structure.namespaced_type.namespaces + [module_name]))__convert_from_py(PyObject * _pymsg, void * ros_message);
+-ROSIDL_GENERATOR_C_IMPORT
+ PyObject * @('__'.join(message.structure.namespaced_type.namespaces + [module_name]))__convert_to_py(void * raw_ros_message);
+
+
+diff --git a/rosidl_generator_py/resource/_msg_support.c.em b/rosidl_generator_py/resource/_msg_support.c.em
+index 62941b2..76c89bd 100644
+--- a/rosidl_generator_py/resource/_msg_support.c.em
++++ b/rosidl_generator_py/resource/_msg_support.c.em
+@@ -158,15 +158,80 @@ type_ = member.type
+ if isinstance(type_, AbstractNestedType):
+     type_ = type_.value_type
+ }@
+-@[  if isinstance(type_, NamespacedType)]@
+-@[    if type_.namespaces[0] != package_name]@
+-ROSIDL_GENERATOR_C_IMPORT
+-@[    end if]@
++@[  if isinstance(type_, NamespacedType) and type_.namespaces[0] == package_name]@
+ bool @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)]))__convert_from_py(PyObject * _pymsg, void * _ros_message);
+-@[    if type_.namespaces[0] != package_name]@
+-ROSIDL_GENERATOR_C_IMPORT
+-@[    end if]@
+ PyObject * @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)]))__convert_to_py(void * raw_ros_message);
++@[  elif isinstance(type_, NamespacedType) and ('converter', *type_.namespaced_name()) not in include_directives]@
++@{
++include_directives.add(('converter', *type_.namespaced_name()))
++converter_prefix = '__'.join(type_.namespaces + [convert_camel_case_to_lower_case_underscore(type_.name)])
++# the Python module is named after the interface file, e.g. _add_two_ints for
++# the AddTwoInts_Request service message
++interface_name = type_.name if type_.namespaces[-1] == 'msg' else type_.name.split('_', 1)[0]
++converter_module = '.'.join(type_.namespaces) + '._' + convert_camel_case_to_lower_case_underscore(interface_name)
++}@
++@[    if 'converter_lookup' not in include_directives]@
++@{include_directives.add('converter_lookup')}@
++// Get a conversion function of a message type from another package from its
++// Python class, which that package's extension module provides it to.
++static void * _rosidl_generator_py__get_converter(
++  const char * module_name, const char * class_name, const char * capsule_name)
++{
++  PyObject * module = PyImport_ImportModule(module_name);
++  if (!module) {
++    return NULL;
++  }
++  PyObject * pyclass = PyObject_GetAttrString(module, class_name);
++  Py_DECREF(module);
++  if (!pyclass) {
++    return NULL;
++  }
++  PyObject * capsule = PyObject_GetAttrString(pyclass, capsule_name);
++  if (capsule == Py_None) {
++    Py_DECREF(capsule);
++    PyObject * result = PyObject_CallMethod(pyclass, "__import_type_support__", NULL);
++    if (!result) {
++      Py_DECREF(pyclass);
++      return NULL;
++    }
++    Py_DECREF(result);
++    capsule = PyObject_GetAttrString(pyclass, capsule_name);
++  }
++  Py_DECREF(pyclass);
++  if (!capsule) {
++    return NULL;
++  }
++  void * converter = PyCapsule_GetPointer(capsule, NULL);
++  Py_DECREF(capsule);
++  return converter;
++}
++
++@[    end if]@
++static bool @(converter_prefix)__convert_from_py(PyObject * _pymsg, void * _ros_message)
++{
++  static bool (* convert)(PyObject *, void *) = NULL;
++  if (!convert) {
++    convert = (bool (*)(PyObject *, void *))_rosidl_generator_py__get_converter(
++      "@(converter_module)", "@(type_.name)", "_CONVERT_FROM_PY");
++    if (!convert) {
++      return false;
++    }
++  }
++  return convert(_pymsg, _ros_message);
++}
++
++static PyObject * @(converter_prefix)__convert_to_py(void * raw_ros_message)
++{
++  static PyObject * (* convert)(void *) = NULL;
++  if (!convert) {
++    convert = (PyObject * (*)(void *))_rosidl_generator_py__get_converter(
++      "@(converter_module)", "@(type_.name)", "_CONVERT_TO_PY");
++    if (!convert) {
++      return NULL;
++    }
++  }
++  return convert(raw_ros_message);
++}
+ @[  end if]@
+ @[end for]@
+
+@@ -175,7 +240,6 @@ module_name = '_' + convert_camel_case_to_lower_case_underscore(interface_path.s
+ class_module = '%s.%s' % ('.'.join(message.structure.namespaced_type.namespaces), module_name)
+ namespaced_type = message.structure.namespaced_type.name
+ }@
+-ROSIDL_GENERATOR_C_EXPORT
+ bool @('__'.join(message.structure.namespaced_type.namespaces + [convert_camel_case_to_lower_case_underscore(message.structure.namespaced_type.name)]))__convert_from_py(PyObject * _pymsg, void * _ros_message)
+ {
+   // check that the passed message is of the expected Python class
+@@ -573,7 +637,6 @@ nested_type = '__'.join(type_.namespaced_name())
+   return true;
+ }
+
+-ROSIDL_GENERATOR_C_EXPORT
+ PyObject * @('__'.join(message.structure.namespaced_type.namespaces + [convert_camel_case_to_lower_case_underscore(message.structure.namespaced_type.name)]))__convert_to_py(void * raw_ros_message)
+ {
+   /* NOTE(esteve): Call constructor of @(message.structure.namespaced_type.name) */

pkg_additional_info.yaml

@@ -236,3 +236,11 @@ ros2cli:
   build_number: 26
 rosidl_cli:
   build_number: 26
+rosidl_core_generators:
+  build_number: 26
+rosidl_default_generators:
+  build_number: 26
+rosidl_generator_py:
+  build_number: 26
+std_msgs:
+  build_number: 26

tests/ros-rolling-std-msgs.yaml

@@ -0,0 +1,29 @@
+tests:
+  # Regression test: a plain C++ consumer of std_msgs must build and run.
+  # If std_msgs_TARGETS exports the Python C bindings library
+  # (libstd_msgs__rosidl_generator_py), the consumer links it and it has
+  # unresolved CPython symbols: the executable aborts on macOS ("symbol not
+  # found in flat namespace '_PyExc_RuntimeError'") and fails to link or load
+  # on Linux when it links Python3::Module
+  # (https://github.com/RoboStack/ros-kilted/issues/76).
+  # LDFLAGS is cleared so -Wl,--as-needed / -Wl,-dead_strip_dylibs can't hide it.
+  - if: unix
+    then:
+      script:
+        - env -u LDFLAGS cmake -S cpp_consumer -B build -G Ninja -DCMAKE_PREFIX_PATH="$PREFIX"
+        - cmake --build build
+        - ./build/std_msgs_cpp_consumer
+      requirements:
+        build:
+          - ${{ compiler('cxx') }}
+          - cmake
+          - ninja
+      files:
+        recipe:
+          - cpp_consumer/
+  # The Python C bindings library still works when loaded by Python.
+  - script:
+      - python -c "from rclpy.serialization import deserialize_message, serialize_message; from std_msgs.msg import Header; m = deserialize_message(serialize_message(Header(frame_id='map')), Header); assert m.frame_id == 'map'"
+    requirements:
+      run:
+        - ros-rolling-rclpy

tests/ros-rolling-std-msgs/cpp_consumer/CMakeLists.txt

@@ -0,0 +1,7 @@
+cmake_minimum_required(VERSION 3.20)
+project(std_msgs_cpp_consumer CXX)
+
+find_package(std_msgs REQUIRED)
+
+add_executable(std_msgs_cpp_consumer main.cpp)
+target_link_libraries(std_msgs_cpp_consumer ${std_msgs_TARGETS})

tests/ros-rolling-std-msgs/cpp_consumer/main.cpp

@@ -0,0 +1,11 @@
+#include <iostream>
+
+#include <std_msgs/msg/string.hpp>
+
+int main()
+{
+  std_msgs::msg::String msg;
+  msg.data = "hello";
+  std::cout << "std_msgs C++ consumer started fine: " << msg.data << std::endl;
+  return 0;
+}