feat: add selective interface package inclusion#6
Conversation
Signed-off-by: Esteve Fernandez <esteve@apache.org>
|
@maspe36 let me know what you think of this, or if you think it's best to try a different approach. The relevant changes in rclrs are in https://github.com/ros2-rust/ros2_rust/pull/625/changes#diff-2f8219e965041e968786c959ba3d4469dcdc468c5f17502dffd2054e81a886a6R38 and https://github.com/ros2-rust/ros2_rust/pull/625/changes#diff-2f8219e965041e968786c959ba3d4469dcdc468c5f17502dffd2054e81a886a6R67 The default behavior in |
|
|
||
| Cargo feature selection is additive across the workspace: if any dependency enables a feature, it is unified for the final build. | ||
|
|
||
| For selective inclusion, disable default features and opt into the package features you need. `rclrs_core` is a feature alias for the common core interface set used by `rclrs` (`action_msgs`, `builtin_interfaces`, `rcl_interfaces`, `rosgraph_msgs`, and `unique_identifier_msgs`): |
There was a problem hiding this comment.
My only concern with this approach is that users may think that they can selectively include any msg package, when in reality they can only include the ones defined in our feature set.
Perhaps we should lean on 2 features, include_all and manual (or some other name) and then if the later feature is selected we read a predefined metadata field and only include those deps?
ros-env = {
version = "0.2",
default-features = false,
features = ["manual"]
}
# Okay, this is close in name to the other metadata we look for.
# Maybe not `includes` but you get the point
[package.metadata.ros-env]
includes = [
"action_msgs",
"builtin_interfaces",
"rcl_interfaces",
"my_msg"
]There was a problem hiding this comment.
Okay, metadata approach won't work as downstream crates won't be able to read the metadata from consumer crates :/
This PR allows downstream dependencies to select whether they want all the discovered craates with messages or only a predefined subset that
rclrsrequires. These changes are for supporting ros2-rust/ros2_rust#625