feat: vendorize messages so that cargo update works #509
Conversation
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
| @@ -159,17 +159,17 @@ where | |||
| /// signatures and which returns a `()` (a.k.a. nothing). | |||
| /// ``` | |||
| /// # use rclrs::*; | |||
| /// # use rclrs::vendor::test_msgs::srv::{Empty, Empty_Request, Empty_Response}; | |||
| /// # use crate::rclrs::vendor::test_msgs; | |||
There was a problem hiding this comment.
I'm a bit confused about the leading crate:: here since I thought doctests give a compilation error when you do use crate::. But I guess if the tests are passing then 🤷
There was a problem hiding this comment.
I don't think we even need crate::. Locally, doctests seem to be passing with out it
There was a problem hiding this comment.
If it makes you feel any better about the vendoring, r2r is currently vendoring many more interface packages than we are: https://docs.rs/r2r/latest/r2r/#modules
Since the only interface packages we're adding here are for examples and tests, I don't think there's any substantial risk that users will run into the diamond dependency problem with them, the way they currently do with builtin_interfaces::msg::Time. So I don't think this PR will make the vendoring problem any worse.
|
@mxgrey thanks for the review. I'd really like to find a different path, but for now it is what it is and vendorizing the messages gets the job done. We might end up just uploading all the standard messages to |
| @@ -189,7 +193,7 @@ mod service; | |||
| mod subscription; | |||
| mod time; | |||
| mod time_source; | |||
| mod vendor; | |||
| pub mod vendor; | |||
There was a problem hiding this comment.
Oh this needs to be pub because doctest only links to public stuff... Jeez, I can see this causing some confusion. I think we need to re-evaluate our message consumption more and more everyday 😅
There was a problem hiding this comment.
Yeah, and I had to modify the vendorize script to not complain about missing docs for messages because now they are public 😅
| @@ -159,17 +159,17 @@ where | |||
| /// signatures and which returns a `()` (a.k.a. nothing). | |||
| /// ``` | |||
| /// # use rclrs::*; | |||
| /// # use rclrs::vendor::test_msgs::srv::{Empty, Empty_Request, Empty_Response}; | |||
| /// # use crate::rclrs::vendor::test_msgs; | |||
There was a problem hiding this comment.
I don't think we even need crate::. Locally, doctests seem to be passing with out it
This PR vendorizes the
example_interfacesandtest_msgspackages so thatcargo updatecan work during the release-plz job (see https://github.com/ros2-rust/ros2_rust/actions/runs/16942715079/job/48016392561)