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

Remove blanket dependency on boost? #24

Open
mikaelarguedas opened this issue Feb 22, 2020 · 10 comments
Open

Remove blanket dependency on boost? #24

mikaelarguedas opened this issue Feb 22, 2020 · 10 comments
Labels
backlog enhancement New feature or request

Comments

@mikaelarguedas
Copy link
Member

This package was flagged as one of the only ROS 2 packages using the blanket boost rosdep key.

This looks less sed-able than the other packages as this is used in some diffs that I don't know where they're applied.

Is it possible to audit if / how this dependency can be replaced with more specific ones ?

Some context and examples at:
specific boost keys: ros/rosdistro#23624
example of use: ros-drivers/transport_drivers#10
DIscourse discussion about more precise dependency definition: https://discourse.ros.org/t/generating-dev-and-runtime-artefacts-from-ros-packages/12448

@emersonknapp
Copy link
Contributor

Definitely agree that boost is an unecessarily large dependency. It's awkward in this case because this dependency is coming from building ros_comm's rosbag_storage. There seems to be a pretty heavy legacy dependency on many parts of boost in there - e.g. it's got boost::function and boost::shared_ptr going on at a quick glance, which are now available as stdlib concepts.

It might be possible to apply more patches to remove parts of boost in favor of stlib. Also might be possible to narrow down the dependency. Also maybe a branch of ros_comm for this build would be in order, once the noetic API is finalized, to make it easier to build here. Just wanted to add this context of what it's being used for.

@mikaelarguedas
Copy link
Member Author

Maybe this PR will help ros/ros_comm#1871

@Karsten1987
Copy link
Contributor

I am closing this as we only implicitly depend on boost through ros_comm.

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Jul 27, 2020

@Karsten1987 does this mean that this explicit boost dependency

<build_depend>boost</build_depend>
can be removed and that all the necessary dependencies will be exported by ros_comm ?

@Karsten1987
Copy link
Contributor

Karsten1987 commented Jul 27, 2020

Hmm, I might have misunderstood your original intention for this ticket then.

What I meant is that I don't think we can easily get rid of this as we implicitly get the boost dependency from ros_comm. Note, we are currently using the melodic version of (ros1, ros_comm) rosbag here which brings boost with it.

I am personally not working on changing this to noetic, but I am happy to review any PRs for it.

EDIT: Maybe we can change the title of this ticket to make it more explicit that a change to rosbag noetic would help?

@Karsten1987 Karsten1987 reopened this Jul 27, 2020
@Karsten1987 Karsten1987 added the enhancement New feature or request label Jul 27, 2020
@mikaelarguedas
Copy link
Member Author

I believe all the changes in rosbag have been made (#24 (comment))

As per a PR. I see master is v0.0.9 and foxy is 0.0.10. Should the PR target the foxy branch or master (which one is targetting noetic?) ?

@Karsten1987
Copy link
Contributor

I am not sure how I feel about backporting such a change to foxy. Please feel free to open a PR on master and we'll release it either into rolling or galactic at some point.

@mikaelarguedas
Copy link
Member Author

IIUC Galactic wont have any overlap with ROS 1 distros. maybe its not worth spending the effort to fix this then.

@dirk-thomas
Copy link
Member

Galactic wont have any overlap with ROS 1 distros.

Galactic will still target Ubuntu 20.04 and therefore overlap with ROS 1 Noetic.

@mikaelarguedas
Copy link
Member Author

You're absolutely right, I was off by one for some reason

@emersonknapp emersonknapp self-assigned this Jan 15, 2021
@emersonknapp emersonknapp removed their assignment Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants