Fix rosidl_generator_py assuming incorect library names #149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CI (build:
|
|
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
force-pushed
the
fix_python_assuming_library_names
branch
from
277ae86
to
ffb0eea
Compare
|
CI re-run
|
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 |
|
CI re-run one last time because of flaky tests above (build:
|
|
CI LGTM, only two windows test failures are rosbag2 ones that are also in the nightly. |
wjwwood
approved these changes
Jan 11, 2022
delete-merged-branch
bot
deleted the
fix_python_assuming_library_names
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires ament/ament_cmake#371
rosidl_generator_py was assuming all interface packages being depended
upon call their interface generation target name
"${PROJECT_NAME}", butthat's an incorrect assumption. The target name is a required argument
to
rosidl_generate_interfaces(). This fixes it by exporting the pythonlibrary target using an existing CMake macro for that purpose called
rosidl_export_typesupport_targets()andament_export_targets().I works by adding a variable
"${PROJECT_NAME}_TARGETS__rosidl_generator_py"which is set when theinterface package is
find_package()d. That variable contains thetargets generated by
rosidl_generator_pyso that downstream interfacepackages can depend on it.
Since targets are being exported, a bug where build time include paths were marked
PUBLICalso 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