-
Notifications
You must be signed in to change notification settings - Fork 172
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
Move ros1-bridge images to the official docker library #415
Conversation
Here is the package list difference (Extracted from existing images The only one that doesn't seem right is the removal of Packages removed compared to ros-base image
Packages added compared to ros-base image
|
Do we still want to install the ros1 tutorial and ros2 demo packages? Although they give a quick means to test the bridge container with itself, those packages are not necessary dependencies for the docker_images/ros/eloquent/ubuntu/bionic/ros1-bridge/Dockerfile Lines 21 to 25 in 1c9b345
I forget, did we have a good reason to unset the helper envs?
Do those conflict with ros1 packages an any meaningful way?
I'm guessing this may be a bit shorter with foxy/noetic, with pruning of boost and python3 switch? |
Do you think it might be helpful to change the default docker_images/ros/eloquent/ubuntu/bionic/ros1-bridge/Dockerfile Lines 30 to 31 in 1c9b345
Also, the name of the entrypoint is unchanged, this being redundant and could be omitted as well. |
Also, is
Would we want to use a meta-package like |
See potential patch for this PR here: #416 |
Unfortunately they are. At lease the ROS 1 ones as they provide some service definitions used to compile the bridge: ros2/ros1_bridge#228 The ROS 2 demo packages were added for consistency and simpler testing. The space that would be saved removing them is pretty small
I believe it was just to limit our environment footprint as these variables have nothing official (we made them up to simplify our sourcing).
I guess we could keep them, it wouldnt cost anything to do it.
The image is based on the ros-base image, these were removed because they were conflicting with the ROS1 install so reinstalling them would remove the ROS 1 packages.
A bit though not significantly because despite effort theres still a lost of libboost-all in ROS 1 packages.
|
Ok, that is unfortunate but see why now. We'll keep the demos installed then.
I've updated the endpoint to let them be, as well removed the redundant Dockerfile derectives. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Perhaps we could check if @tfoote or @dirk-thomas have any last feedback.
9d9f588
to
5a40ac1
Compare
I made an attempt to install back the rosdep-modules but it failed because they are currently not side by side installable ros-infrastructure/rosdep#759 So it installs only It needed osrf/docker_templates#88 to be able to do so |
Dang. Although, this should be a non issue for foxy/noetic given the complete move to python3? |
Yes that's my expectation |
I confirmed that the foxy ROS 1 bridge does bring in all boost libraries, main culprits seem to be actionlib, tf and tf2-ros. It seems that we could remove the need for all these dependencies if the bridge doesnt build with support for tf1 messages (deprecated for many years now) ros2-gbp/ros1_bridge-release#11. That would save the installation of ~380MB of stuff
|
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> regenerate Makefiles and Dockerfiles update docker library Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> reinstall rosdep and rosdep-modules so that the tool can be used in the final image Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> python3-rosdep-modules not side by side installable due to ros-infrastructure/rosdep#759 Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update Dockerfiles Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update docker library Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> remove Python2 only hack from default image config template Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> remove foxy ros1_bridge Dockerhub hook Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update docker library Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> update manifest after foxy merge Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
5a40ac1
to
7e7076d
Compare
shipping these now to get the Foxy official ros1-bridge image for the announcement |
This also deleted the hooks.
If / once merged we'll need to remove the triggers on dockerhub and to announce the shift on discourse.
Fixes #414