rosidl_generator_py_generate_interfaces: link Python3::Module instead of Python3::Python

[traversaro]

Description

When linking a Python loadable module (or a shared library that will be linked by a Python loadable module), CMake suggest to link Python3::Module instead of Python3::Python, see https://cmake.org/cmake/help/latest/module/FindPython.html. In particular, Python3::Python is described as:

Python library for Python embedding.

while Python3::Module is described as:

Python library for Python module.

Why is this necessary? There is not a big difference on any platform, except on macOS when using a Python interpreter that statically link libpython. In that case, linking to Python3::Module ensure that there are no segfaults when the corresponding Python module is loaded in the Python interpreter.

See some related issues on this:

• Link Python bindings to Python::Module and not Python::Python to prevent Python bindings segfault on macOS robotology/icub-main#987
• Can't link in /librcl_interfaces__rosidl_generator_py.so due to it not linking in libpython RoboStack/ros-humble#119
• Statically link the python executable to libpython and disable the shared library astral-sh/python-build-standalone#540 (comment)
• segmentation fault python macos AprilRobotics/apriltag#352
• Building 23.02 with conda results in CLI tools not working PixarAnimationStudios/OpenUSD#2371
• Provide python executable compiled with enable-shared  conda-forge/python-feedstock#595
• Remove SHARED from pybind11_add_module rclpy#1305
• Remove SHARED from pybind11_add_module rosbag2#1929

Fixes # (issue)

Is this user-facing behavior change?

No, it is not a user-facing behaviour change.

Did you use Generative AI?

No.

Additional Information

This is one of the long standing patches that we carry on in RoboStack, I am not sure why we never upstreamed it.

[fujitatomoya]

i think this is a right thing to do.

• the code is building extension modules, so Python3::Module is the correct target.
• Python3::Module avoids this by using the correct linker flags (on macOS, it uses -undefined dynamic_lookup instead of linking libpython directly).
• this actually aligns with CMake best practices as suggested.

lgtm with green CI.

[fujitatomoya]

Pulls: #253
Gist: https://gist.githubusercontent.com/fujitatomoya/913e2fc5dffd1d20f70b272bf267dac4/raw/6e607192490a7dbf9b2271def8f7f7c21e2a9c0f/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_generator_py
TEST args: --packages-above rosidl_generator_py
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18693

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[wep21]

This PR causes undefined references when building interface packages.

17:10:29 --- stderr: builtin_interfaces
17:10:29 /usr/bin/ld: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/rosidl_generator_py/builtin_interfaces/msg/_duration_s.c.o: in function `_import_array':
17:10:29 _duration_s.c:(.text+0x303): undefined reference to `PyImport_ImportModule'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x336): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x385): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x3a4): undefined reference to `PyCapsule_Type'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x3ba): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x3cf): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x41a): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x438): undefined reference to `PyCapsule_GetPointer'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x48a): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x4a0): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x4b5): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x4e7): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x505): undefined reference to `PyErr_Format'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x541): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x55f): undefined reference to `PyErr_Format'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x588): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x59d): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x5b1): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x5c6): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/rosidl_generator_py/builtin_interfaces/msg/_duration_s.c.o: in function `builtin_interfaces__msg__duration__convert_from_py':
17:10:29 _duration_s.c:(.text+0x60a): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x63e): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x69d): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x6c3): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x726): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x777): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x876): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x8c7): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x918): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x942): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0x9b4): undefined reference to `PyLong_AsLong'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xa0d): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xa29): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xa9b): undefined reference to `PyLong_AsUnsignedLong'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xaf5): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/rosidl_generator_py/builtin_interfaces/msg/_duration_s.c.o: in function `builtin_interfaces__msg__duration__convert_to_py':
17:10:29 _duration_s.c:(.text+0xb2c): undefined reference to `PyImport_ImportModule'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xb75): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xbf3): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xc07): undefined reference to `PyObject_CallObject'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xc56): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xc8d): undefined reference to `PyLong_FromLong'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xcab): undefined reference to `PyObject_SetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xcf9): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xd25): undefined reference to `PyLong_FromUnsignedLong'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xd43): undefined reference to `PyObject_SetAttrString'
17:10:29 /usr/bin/ld: _duration_s.c:(.text+0xd91): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/rosidl_generator_py/builtin_interfaces/msg/_time_s.c.o: in function `_import_array':
17:10:29 _time_s.c:(.text+0x303): undefined reference to `PyImport_ImportModule'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x336): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x385): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x3a4): undefined reference to `PyCapsule_Type'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x3ba): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x3cf): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x41a): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x438): undefined reference to `PyCapsule_GetPointer'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x48a): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x4a0): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x4b5): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x4e7): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x505): undefined reference to `PyErr_Format'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x541): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x55f): undefined reference to `PyErr_Format'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x588): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x59d): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x5b1): undefined reference to `PyExc_RuntimeError'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x5c6): undefined reference to `PyErr_SetString'
17:10:29 /usr/bin/ld: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/rosidl_generator_py/builtin_interfaces/msg/_time_s.c.o: in function `builtin_interfaces__msg__time__convert_from_py':
17:10:29 _time_s.c:(.text+0x60a): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x63e): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x69d): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x6c3): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x726): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x777): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x876): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x8c7): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x918): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x942): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0x9b4): undefined reference to `PyLong_AsLong'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xa0d): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xa29): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xa9b): undefined reference to `PyLong_AsUnsignedLong'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xaf5): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/rosidl_generator_py/builtin_interfaces/msg/_time_s.c.o: in function `builtin_interfaces__msg__time__convert_to_py':
17:10:29 _time_s.c:(.text+0xb2c): undefined reference to `PyImport_ImportModule'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xb75): undefined reference to `PyObject_GetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xbf3): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xc07): undefined reference to `PyObject_CallObject'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xc56): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xc8d): undefined reference to `PyLong_FromLong'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xcab): undefined reference to `PyObject_SetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xcf9): undefined reference to `_Py_Dealloc'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xd25): undefined reference to `PyLong_FromUnsignedLong'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xd43): undefined reference to `PyObject_SetAttrString'
17:10:29 /usr/bin/ld: _time_s.c:(.text+0xd91): undefined reference to `_Py_Dealloc'
17:10:29 collect2: error: ld returned 1 exit status
17:10:29 gmake[2]: *** [CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/build.make:117: libbuiltin_interfaces__rosidl_generator_py.so] Error 1
17:10:29 gmake[1]: *** [CMakeFiles/Makefile2:525: CMakeFiles/builtin_interfaces__rosidl_generator_py.dir/all] Error 2
17:10:29 gmake: *** [Makefile:146: all] Error 2
17:10:29 ---
17:10:29 Failed   <<< builtin_interfaces [26.8s, exited with code 2]
17:11:50 Aborted  <<< qt_gui_cpp [1min 54s]
17:20:36 Aborted  <<< rviz_ogre_vendor [12min 45s]
17:24:44 Aborted  <<< zenoh_cpp_vendor [17min 51s]
17:24:44 
17:24:44 Summary: 127 packages finished [18min 48s]
17:24:44   1 package failed: builtin_interfaces
17:24:44   3 packages aborted: qt_gui_cpp rviz_ogre_vendor zenoh_cpp_vendor
17:24:44   11 packages had stderr output: builtin_interfaces foonathan_memory_vendor google_benchmark_vendor gz_cmake_vendor gz_math_vendor gz_utils_vendor iceoryx_posh mimick_vendor qt_gui_cpp rviz_ogre_vendor zenoh_cpp_vendor
17:24:44   217 packages not processed
17:24:45 <== '/usr/bin/colcon build --base-paths "src" --build-base "build" --install-base "install" --event-handlers console_cohesion+ console_package_list+ --cmake-args -DBUILD_TESTING=ON --no-warn-unused-cli -DINSTALL_EXAMPLES=OFF -DSECURITY=ON -DAPPEND_PROJECT_NAME_TO_INCLUDEDIR=ON --packages-above-and-dependencies rosidl_generator_py' exited with return code '2'
17:24:48 Build step 'Execute shell' marked build as failure
17:24:48 $ ssh-agent -k
17:24:48 unset SSH_AUTH_SOCK;
17:24:48 unset SSH_AGENT_PID;
17:24:48 echo Agent pid 1777645 killed;
17:24:48 [ssh-agent] Stopped.
17:24:49 [CMakeGNU C Compiler (gcc)ClangClang-Tidy] Skipping execution of recorder since overall result is 'FAILURE'
17:24:49 [Coverage] Skipping execution of coverage recorder since overall result is 'FAILURE'
17:24:49 INFO: Processing CTest-Version 3.x (default)
17:24:49 INFO: [CTest-Version 3.x (default)] - No test report file(s) were found with the pattern 'ws/build/*/Testing/*/Test.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'CTest-Version 3.x (default)'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'CTest-Version 3.x (default)'?"
17:24:49 INFO: Processing GoogleTest-1.8
17:24:49 INFO: [GoogleTest-1.8] - No test report file(s) were found with the pattern 'ws/build/*/test_results/**/*.gtest.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'GoogleTest-1.8'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'GoogleTest-1.8'?"
17:24:49 INFO: Processing JUnit
17:24:49 INFO: [JUnit] - No test report file(s) were found with the pattern 'ws/build/*/pytest.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'JUnit'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'JUnit'?"
17:24:49 INFO: Processing JUnit
17:24:49 INFO: [JUnit] - No test report file(s) were found with the pattern 'ws/build/*/test_results/**/*.xunit.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'JUnit'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'JUnit'?"
17:24:49 INFO: Processing CTest-Version 3.x (default)
17:24:49 INFO: [CTest-Version 3.x (default)] - No test report file(s) were found with the pattern 'work space/build space/*/Testing/*/Test.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'CTest-Version 3.x (default)'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'CTest-Version 3.x (default)'?"
17:24:49 INFO: Processing GoogleTest-1.8
17:24:49 INFO: [GoogleTest-1.8] - No test report file(s) were found with the pattern 'work space/build space/*/test_results/**/*.gtest.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'GoogleTest-1.8'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'GoogleTest-1.8'?"
17:24:49 INFO: Processing JUnit
17:24:49 INFO: [JUnit] - No test report file(s) were found with the pattern 'work space/build space/*/pytest.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'JUnit'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'JUnit'?"
17:24:49 INFO: Processing JUnit
17:24:49 INFO: [JUnit] - No test report file(s) were found with the pattern 'work space/build space/*/test_results/**/*.xunit.xml' relative to '/home/jenkins-agent/workspace/ci_linux' for the testing framework 'JUnit'.
17:24:49 Did you enter a pattern relative to (and within) the workspace directory?
17:24:49 Did you generate the result report(s) for 'JUnit'?"
17:24:49 WARNING: All test reports are empty.
17:24:49 INFO: Check 'Failed Tests' threshold.
17:24:49 INFO: Check 'Skipped Tests' threshold.
17:24:49 [Checks API] No suitable checks publisher found.
17:24:49 INFO: Setting the build status to SUCCESS
17:24:49 Finished: FAILURE

[wep21]

I guess ${_target_name_lib} which contains generated c codes uses Pyhton C API, so it should be linked to Python3::Python.

[traversaro]

I guess ${_target_name_lib} which contains generated c codes uses Pyhton C API, so it should be linked to Python3::Python.

Unless you are embedding the Python interpreter, we should not link Python3::Python . I am not sure about the error but I suspect it is something else.

[traversaro]

That was a bit complex.

It is not clear to me why, but the rosidl_generator_py_generate_interfaces extension does not simply create a Python module named ${PROJECT_NAME}_s__${_typesupport_impl} in

rosidl_python/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake

Line 193 in 01af609

python3_add_library(${_target_name} MODULE

(for example, in uuid case unique_identifier_msgs_s__rosidl_typesupport_c.cpython-312-x86_64-linux-gnu.so) but it also creates a SHARED library (that as far as I can see is only linked by the Python module) called ${rosidl_generate_interfaces_TARGET}__rosidl_generator_py (for example, libunique_identifier_msgs__rosidl_generator_py.so) in

rosidl_python/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake

Line 155 in 01af609

add_library(${_target_name_lib} SHARED ${_generated_c_files})

.

That would not be particularly problematic, as creating shared library with undefined symbols is common practice in Linux. The problem is that then a completely different cmake extension rosidl_generator_rs_generate_interfaces is poisoning the global variable CMAKE_SHARED_LINKER_FLAGS with the -Wl,--no-undefined" option in https://github.com/ros2-rust/rosidl_rust/blame/14105edd83f96d8c481848dcbe72de3764342177/rosidl_generator_rs/cmake/rosidl_generator_rs_generate_interfaces.cmake#L17 . Unfortunately ament's generator extensions run completely unscoped, so the value set the leaks in the all build.

[traversaro]

I opened ros2-rust/rosidl_rust#22, I guess we do not want to merge this PR until ros2-rust/rosidl_rust#22 is merged and released.