From 2ed71d4cda70905c28b13cc9f04c5e55b802d734 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 24 Sep 2026 00:11:10 +0800 Subject: [PATCH 1/3] Add regression test: C++ consumers of std_msgs must not link its Python bindings Every interface package exports its Python C bindings library (lib__rosidl_generator_py) in _TARGETS, so plain C++ consumers link it. That library has unresolved CPython symbols, so the consumer aborts at startup on macOS ("symbol not found in flat namespace '_PyExc_RuntimeError'") and fails to link or load on Linux when the library links Python3::Module (RoboStack/ros-kilted#76). The test builds a C++ executable against ${std_msgs_TARGETS} and runs it (unix only), and checks that the Python bindings still work from Python. std_msgs is bumped to build 26 so CI rebuilds it and runs the test. This commit is expected to fail the new test on macOS. On Linux the library currently links libpython (Python3::Python), which hides the problem: the consumer runs, but loads libpython. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Maurice --- pkg_additional_info.yaml | 2 ++ tests/ros-rolling-std-msgs.yaml | 29 +++++++++++++++++++ .../cpp_consumer/CMakeLists.txt | 7 +++++ .../cpp_consumer/main.cpp | 11 +++++++ 4 files changed, 49 insertions(+) create mode 100644 tests/ros-rolling-std-msgs.yaml create mode 100644 tests/ros-rolling-std-msgs/cpp_consumer/CMakeLists.txt create mode 100644 tests/ros-rolling-std-msgs/cpp_consumer/main.cpp diff --git a/pkg_additional_info.yaml b/pkg_additional_info.yaml index d8f6da8c..a57d126d 100644 --- a/pkg_additional_info.yaml +++ b/pkg_additional_info.yaml @@ -236,3 +236,5 @@ ros2cli: build_number: 26 rosidl_cli: build_number: 26 +std_msgs: + build_number: 26 diff --git a/tests/ros-rolling-std-msgs.yaml b/tests/ros-rolling-std-msgs.yaml new file mode 100644 index 00000000..c4055f0d --- /dev/null +++ b/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 diff --git a/tests/ros-rolling-std-msgs/cpp_consumer/CMakeLists.txt b/tests/ros-rolling-std-msgs/cpp_consumer/CMakeLists.txt new file mode 100644 index 00000000..17d2b6eb --- /dev/null +++ b/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}) diff --git a/tests/ros-rolling-std-msgs/cpp_consumer/main.cpp b/tests/ros-rolling-std-msgs/cpp_consumer/main.cpp new file mode 100644 index 00000000..fc5aa7c7 --- /dev/null +++ b/tests/ros-rolling-std-msgs/cpp_consumer/main.cpp @@ -0,0 +1,11 @@ +#include + +#include + +int main() +{ + std_msgs::msg::String msg; + msg.data = "hello"; + std::cout << "std_msgs C++ consumer started fine: " << msg.data << std::endl; + return 0; +} From 7d891c316252bd678da13e1e525e14cca43052ad Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 24 Sep 2026 00:11:23 +0800 Subject: [PATCH 2/3] rosidl_generator_py: compile the Python bindings into the extension modules Replace the macOS-only patch with a cross-platform one that removes the shared lib__rosidl_generator_py library: - The generated C conversion code is compiled (as an OBJECT library) into each _s__rosidl_typesupport_* Python extension module, which links Python3::Module. Nothing is installed or exported, so _TARGETS only contains C/C++ libraries and no library has unresolved CPython symbols. - Conversion functions of nested types from other packages are looked up (and cached) from those packages' Python message classes (_CONVERT_FROM_PY / _CONVERT_TO_PY capsules, already used by rclpy) instead of linking the other package's library. rosidl_generator_py is bumped to build 26 so CI rebuilds it. So are rosidl_core_generators and rosidl_default_generators: std_msgs only depends on the generator through them, and without rebuilding them rattler-build does not know to build the generator before std_msgs. The regression test added in the previous commit now passes. Message packages built by the old generator link their dependencies' lib__rosidl_generator_py, so all interface packages must be rebuilt together (e.g. in the next full rebuild). Co-Authored-By: Claude Opus 5.5 Signed-off-by: Maurice --- .../ros-rolling-rosidl-generator-py.osx.patch | 69 ------- patch/ros-rolling-rosidl-generator-py.patch | 178 ++++++++++++++++++ pkg_additional_info.yaml | 6 + 3 files changed, 184 insertions(+), 69 deletions(-) delete mode 100644 patch/ros-rolling-rosidl-generator-py.osx.patch create mode 100644 patch/ros-rolling-rosidl-generator-py.patch diff --git a/patch/ros-rolling-rosidl-generator-py.osx.patch b/patch/ros-rolling-rosidl-generator-py.osx.patch deleted file mode 100644 index e0795ef7..00000000 --- a/patch/ros-rolling-rosidl-generator-py.osx.patch +++ /dev/null @@ -1,69 +0,0 @@ -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 de6a6c65..62093e4a 100644 ---- a/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake -+++ b/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake -@@ -152,7 +152,7 @@ set_property( - - set(_target_name_lib "${rosidl_generate_interfaces_TARGET}__rosidl_generator_py") - add_library(${_target_name_lib} SHARED ${_generated_c_files}) --target_link_libraries(${_target_name_lib} PRIVATE -+target_link_libraries(${_target_name_lib} PUBLIC - ${rosidl_generate_interfaces_TARGET}__rosidl_generator_c) - add_dependencies( - ${_target_name_lib} -@@ -160,11 +160,6 @@ add_dependencies( - ${rosidl_generate_interfaces_TARGET}__rosidl_typesupport_c - ) - --target_link_libraries( -- ${_target_name_lib} PRIVATE -- Python3::NumPy -- Python3::Python --) - target_include_directories(${_target_name_lib} - PRIVATE - ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_c -@@ -176,8 +171,15 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - set(_extension_compile_flags -Wall -Wextra) - endif() - -+if(APPLE) -+ target_link_libraries(${_target_name_lib} PRIVATE Python3::NumPy Python3::Module) -+ target_include_directories(${_target_name_lib} PRIVATE ${Python3_INCLUDE_DIRS} ${Python3_NumPy_INCLUDE_DIRS}) -+else() -+ target_link_libraries(${_target_name_lib} PUBLIC Python3::NumPy Python3::Python) -+endif() -+ - rosidl_get_typesupport_target(c_typesupport_target "${rosidl_generate_interfaces_TARGET}" "rosidl_typesupport_c") --target_link_libraries(${_target_name_lib} PRIVATE ${c_typesupport_target}) -+target_link_libraries(${_target_name_lib} PUBLIC ${c_typesupport_target}) - - foreach(_typesupport_impl ${_typesupport_impls}) - find_package(${_typesupport_impl} REQUIRED) -@@ -210,7 +212,7 @@ foreach(_typesupport_impl ${_typesupport_impls}) - RUNTIME_OUTPUT_DIRECTORY ${_output_path}) - - target_link_libraries( -- ${_target_name} PRIVATE -+ ${_target_name} PUBLIC - ${_target_name_lib} - ${rosidl_generate_interfaces_TARGET}__${_typesupport_impl} - ${c_typesupport_target} -@@ -226,7 +228,7 @@ foreach(_typesupport_impl ${_typesupport_impls}) - ) - - foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES}) -- target_link_libraries(${_target_name} PRIVATE ${${_pkg_name}__TARGETS}) -+ target_link_libraries(${_target_name} PUBLIC ${${_pkg_name}__TARGETS}) - endforeach() - - add_dependencies(${_target_name} -@@ -243,7 +245,7 @@ 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}}) -+ target_link_libraries(${_target_name_lib} PUBLIC ${${_pkg_name}_TARGETS${rosidl_generator_py_suffix}}) - endforeach() - - set_target_properties(${_target_name_lib} PROPERTIES COMPILE_OPTIONS "${_extension_compile_flags}") \ No newline at end of file diff --git a/patch/ros-rolling-rosidl-generator-py.patch b/patch/ros-rolling-rosidl-generator-py.patch new file mode 100644 index 00000000..81611256 --- /dev/null +++ b/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) */ diff --git a/pkg_additional_info.yaml b/pkg_additional_info.yaml index a57d126d..c7d07c0e 100644 --- a/pkg_additional_info.yaml +++ b/pkg_additional_info.yaml @@ -236,5 +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 From 82d0021e34467f1dc36fec94660f2aa2c177f27e Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 24 Sep 2026 00:53:46 +0800 Subject: [PATCH 3/3] [DO NOT MERGE] CI: drop cached std_msgs build 26 from earlier PR runs The earlier CI runs of this PR cached a std_msgs build 26 built with the old generator; with --skip-existing it would be reused instead of being rebuilt with the patched generator. Drop this commit before merging. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Maurice --- .github/workflows/testpr.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/testpr.yml b/.github/workflows/testpr.yml index 221a4959..eae18643 100644 --- a/.github/workflows/testpr.yml +++ b/.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