Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow ros2_rust to be built within a distro's colcon workspace #370

Merged
merged 2 commits into from
Mar 15, 2024

Conversation

mxgrey
Copy link
Collaborator

@mxgrey mxgrey commented Mar 12, 2024

I attempted to build the ros2_rust repo directly inside the colcon workspace of a ROS 2 distro and found that it doesn't work because of some rosidl_generator_rs behavior that wanted to discover all the RMW implementations in the environment, and that just doesn't quite work with colcon's approach to isolated builds.

However, after investigating it carefully I realized the cmake code that wanted to find all the RMW implementations wasn't actually doing anything helpful. I'm guessing it's dead code that had a specific purpose many years ago, but some redesign since then made it unnecessary.

This PR reworks the dependencies so that there's no need to create a separate colcon workspace to build ros2_rust.

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
jhdcs
jhdcs previously approved these changes Mar 12, 2024
Copy link
Collaborator

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any major issues with the changes. If anything, the code looks cleaner with all the extra calls removed.

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
@mxgrey
Copy link
Collaborator Author

mxgrey commented Mar 12, 2024

Sorry @jhdcs but my last tweak staleified your review. It'll need a new approval now.

@jhdcs jhdcs merged commit 62fea75 into ros2-rust:main Mar 15, 2024
3 checks passed
@mxgrey mxgrey deleted the distro_workspace branch March 18, 2024 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants