Fix rosidl_generator_py assuming incorect library names by sloretz · Pull Request #149 · ros2/rosidl_python

sloretz · 2021-12-10T22:51:59Z

rosidl_generator_py was assuming all interface packages being depended
upon call their interface generation target name "${PROJECT_NAME}", but
that's an incorrect assumption. The target name is a required argument
to rosidl_generate_interfaces(). This fixes it by exporting the python
library target using an existing CMake macro for that purpose called
rosidl_export_typesupport_targets() and ament_export_targets().

I works by adding a variable
"${PROJECT_NAME}_TARGETS__rosidl_generator_py" which is set when the
interface package is find_package()d. That variable contains the
targets generated by rosidl_generator_py so that downstream interface
packages can depend on it.

Since targets are being exported, a bug where build time include paths were marked PUBLIC also needed to be fixed.

I discovered this bug while working on ament/ament_cmake#365, and I need it fixed for ros2/rcl_interfaces#133

sloretz · 2021-12-10T22:53:17Z

CI (build: --packages-above-and-dependencies rosidl_generator_py test: --packages-above rosidl_generator_py)

Linux
Linux-aarch64
macOS
Windows

sloretz · 2021-12-23T23:20:02Z

Trying Windows with repos file with this and ament/ament_cmake#371

Re-run

re-re-run

re-re-re run

re-re-re-re run

re-re-re-re-re run

rosidl_generator_py was assuming all interface packages being depended upon call their interface generation target name "${PROJECT_NAME}", but that's an incorrect assumption. The target name is a required argument to rosidl_generate_interfaces. This fixes it by exporting the python library target using an existing CMake macro for that purpose called rosidl_export_typesupport_targets() and amen_export_targets() I works by adding a variable "${PROJECT_NAME}_TARGETS__rosidl_generator_py" which is set when the interface package is `find_package()`d. That variable contains the targets generated by `rosidl_generator_py` so that downstream interface packages can depend on it. Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz · 2022-01-04T19:45:50Z

CI re-run

Linux
Linux-aarch64
macOS
Windows
- Re-run to see if test failures are consistent:
- All appear in the repeated job except launch_testing_ros.test.examples.set_param_launch_test.set_param_launch_test, investigating.

sloretz · 2022-01-10T21:03:41Z

launch_testing_ros.test.examples.set_param_launch_test.set_param_launch_test

I'm unable to reproduce this failure in a local windows VM. It's flakiness seems to be a known issue: ros2/launch_ros#280

sloretz · 2022-01-10T21:04:57Z

CI re-run one last time because of flaky tests above (build: --packages-above-and-dependencies rosidl_generator_py test: --packages-above rosidl_generator_py)

Linux
Linux-aarch64
Windows

sloretz · 2022-01-11T17:33:38Z

CI LGTM, only two windows test failures are rosbag2 ones that are also in the nightly.

…RGETS Replace the macOS-only patch with a cross-platform one: - Restore rosidl_generator_py_suffix ("__rosidl_generator_py"), which ros2/rosidl_python#140 accidentally dropped. The existing dependency loop then links other interface packages' Python bindings through the dedicated <pkg>_TARGETS__rosidl_generator_py list again, as intended by ros2/rosidl_python#149. - Export the library only in <pkg>_TARGETS__rosidl_generator_py (an installed export file plus a config extras file) instead of with ament_export_targets(), which always adds it to <pkg>_TARGETS. C/C++ consumers of an interface package no longer link it. - Link Python3::Module instead of Python3::Python on all platforms: the library is only linked by Python extension modules, which get the CPython symbols from the interpreter that loads them. rosidl_generator_py is bumped to build 26 so CI rebuilds it, together with rosidl_core_generators and rosidl_default_generators (so that rattler-build builds the generator before the interface packages that use it) and builtin_interfaces (std_msgs' Python bindings link its Python bindings through the new list). The regression test added in the previous commit now passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Maurice <mauricepurnawan@gmail.com>

…RGETS Replace the patch that compiled the Python bindings into the extension modules with a smaller one that is closer to upstream's original design (ros2/rosidl_python#149): - Restore rosidl_generator_py_suffix ("__rosidl_generator_py"), which ros2/rosidl_python#140 accidentally dropped. The existing dependency loop then links other interface packages' Python bindings through the dedicated <pkg>_TARGETS__rosidl_generator_py list again. - Export the library only in <pkg>_TARGETS__rosidl_generator_py (an installed export file plus a config extras file) instead of with ament_export_targets(), which always adds it to <pkg>_TARGETS. C/C++ consumers of an interface package no longer link it. - Link Python3::Module instead of Python3::Python on all platforms: the library is only linked by Python extension modules, which get the CPython symbols from the interpreter that loads them. Also update the std_msgs regression test as suggested in review: build the C++ consumer with cxx-compiler (no LDFLAGS, like a user environment) instead of compiler('cxx') + clearing LDFLAGS, and depend on ros2-rclpy. No build number bumps: this goes into the full rebuild, which rebuilds all interface packages with the new generator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Maurice <mauricepurnawan@gmail.com>

…RGETS Replace the patch that compiled the Python bindings into the extension modules with a smaller one that is closer to upstream's original design (ros2/rosidl_python#149): - Restore rosidl_generator_py_suffix ("__rosidl_generator_py"), which ros2/rosidl_python#140 accidentally dropped. The existing dependency loop then links other interface packages' Python bindings through the dedicated <pkg>_TARGETS__rosidl_generator_py list again. - Export the library without ament_export_targets(), which always adds it to <pkg>_TARGETS: install the export set and include it from a config extras file, and keep rosidl_export_typesupport_targets(), which adds the imported target to <pkg>_TARGETS__rosidl_generator_py only. C/C++ consumers of an interface package no longer link it. - Link Python3::Module instead of Python3::Python on all platforms: the library is only linked by Python extension modules, which get the CPython symbols from the interpreter that loads them. Also update the std_msgs regression test as suggested in review: build the C++ consumer with cxx-compiler (no LDFLAGS, like a user environment) instead of compiler('cxx') + clearing LDFLAGS, and depend on ros2-rclpy. No build number bumps: this goes into the full rebuild, which rebuilds all interface packages with the new generator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Maurice <mauricepurnawan@gmail.com>

sloretz self-assigned this Dec 10, 2021

sloretz mentioned this pull request Dec 10, 2021

Install headers to include/${PROJECT_NAME} and Depend on rosidl_typesupport_* targets directly ros2/rcl_interfaces#133

Merged

sloretz mentioned this pull request Dec 23, 2021

Make ament_include_directories_order a function to allow paths with backslashes on windows. ament/ament_cmake#371

Merged

sloretz closed this in ament/ament_cmake#371 Jan 4, 2022

sloretz reopened this Jan 4, 2022

sloretz force-pushed the fix_python_assuming_library_names branch from 277ae86 to ffb0eea Compare January 4, 2022 19:45

sloretz mentioned this pull request Jan 10, 2022

👨‍🌾 launch_testing_ros set_param_launch_test fails on Windows release ros2/launch_ros#280

Closed

wjwwood approved these changes Jan 11, 2022

View reviewed changes

sloretz merged commit 0f43ffc into master Jan 11, 2022

delete-merged-branch Bot deleted the fix_python_assuming_library_names branch January 11, 2022 18:05

Blast545 mentioned this pull request Mar 24, 2022

👨‍🌾 test_security -> test_processes_finished_gracefully and test_subscriber_terminates_in_a_finite_amount_of_time tests failing on nightlies linux + linux-rhel and windows repeated ros2/system_tests#498

Open

tobiasneumann mentioned this pull request Oct 4, 2023

Fix rhel and windows builds #164

Open

traversaro mentioned this pull request Sep 24, 2026

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

Merged

mini-1235 mentioned this pull request Sep 27, 2026

Keep the Python bindings library out of <pkg>_TARGETS #269

Open

Navigation Menu

Repository navigation

Fix rosidl_generator_py assuming incorect library names - #149

Fix rosidl_generator_py assuming incorect library names#149
sloretz merged 1 commit into
masterfrom
fix_python_assuming_library_names

sloretz commented Dec 10, 2021 •

edited

Loading

Uh oh!

sloretz commented Dec 10, 2021

Uh oh!

sloretz commented Dec 23, 2021 •

edited

Loading

Uh oh!

sloretz commented Jan 4, 2022 •

edited

Loading

Uh oh!

sloretz commented Jan 10, 2022

Uh oh!

sloretz commented Jan 10, 2022

Uh oh!

sloretz commented Jan 11, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sloretz commented Dec 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sloretz commented Dec 10, 2021

Uh oh!

sloretz commented Dec 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sloretz commented Jan 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sloretz commented Jan 10, 2022

Uh oh!

sloretz commented Jan 10, 2022

Uh oh!

sloretz commented Jan 11, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sloretz commented Dec 10, 2021 •

edited

Loading

sloretz commented Dec 23, 2021 •

edited

Loading

sloretz commented Jan 4, 2022 •

edited

Loading