Build system refactor #64
Conversation
|
It appears the changes to the generator that are required for H-Turtle/Rolling aren't backwards-compatible. Is there a way we can deal with this? |
|
Managed to fix it. This should be good for review! |
Allows IDE to parse dependencies. Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
the compilation syntax Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
jhdcs
force-pushed
the
improvement/build_changes
branch
from
a27a5f8
to
bd6ffd2
Compare
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
rclrs/Cargo.toml
Outdated
| @@ -13,6 +16,10 @@ core-error = "0.0.0" | |||
| parking_lot = {version = "0.11.2", optional = true} | |||
| spin = "0.9.2" | |||
|
|
|||
| [dependencies.rclrs_common] | |||
| version = "*" | |||
| path = "../rclrs_common" | |||
There was a problem hiding this comment.
Is there a way to avoid this? It'd be unfortunate because it's bypassing the ROS dependency resolution mechanism, but it makes rust-analyzer, it's fine as a compromise. Or maybe, it'd just make sense to integrate rclrs_common into rclrs. What do you think?
There was a problem hiding this comment.
I had been debating merging the two crates for some time. I will give that a shot and see how it goes.
I am doing something similar in rclrs_examples as well. I'm assuming it's a problem there as well? What I'd like to do is figure out some way to make ROS 2's dependency resolution work, while also allowing cargo the freedom to build crates as it likes, since that's what most IDEs will be expecting...
There was a problem hiding this comment.
Okay, that took quite a bit more effort than I was expecting, but I have managed to combine most of rclrs_common with rclrs. The message traits had to be spun off into their own crate, otherwise we ended up with circular build dependencies.
I can do a bit more thinking on how to avoid the explicit paths in the manifest. Perhaps we should use an auto-generated .cargo/config.toml file, such as the one that I use for the std_msgs dependencies?
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
format 3 schema Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
| @@ -0,0 +1,19 @@ | |||
| <?xml version="1.0"?> | |||
There was a problem hiding this comment.
Correct me if I'm wrong, but it seems that rclrs_msg_utilities serves a similar purpose as rclrs_common, that is, code shared between rclrs and downstream packages / generators. Would it make sense to rework rclrs_common to include the additions from rclrs_msg_utilities ?
There was a problem hiding this comment.
It cannot, as best as I am able to tell. Every time I try, it introduces a circular build dependency.
There was a problem hiding this comment.
Oh, sorry, misunderstood your question. So basically what rclrs_msg_utilities is is just rclrs_common stripped of everything that doesn't have to do with messages. My goal with it is to eventually use it to help with message generation via CMake.
Since I'm not fully sure what the plan is anymore - We were going to switch over to r2r, but then I got comments saying that some other people preferred the CMake implementation...
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
|
Please stand by, working on a solution to having explicit paths in the manifests for the packages... |
Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
|
Okay, should be refactored now. Basically, you feed in the packages you want (and listed in the |
|
@jhdcs thank you so much! Looks great 👍 |
Closes #61, Closes #34, Closes #38, Closes #31, Closes #13
I have refactored the build system so that things can be build in a more Rust-like way: The biggest surprise people may have is that the
rclrs_examplescrate will generate a.cargo/config.tomlfile in order to patch in the file dependency forstd_msgs.Do the changes look acceptable? While I'm still not super-fond of the dependency search we have to do for
rclrs_examples, I'm sure that we can find something else to make that easier in another PR.