Skip to content

Ros2 nightly rebased#55

Merged
ruffsl merged 9 commits intomasterfrom
ros2_nightly_rebased
Mar 29, 2019
Merged

Ros2 nightly rebased#55
ruffsl merged 9 commits intomasterfrom
ros2_nightly_rebased

Conversation

@mikaelarguedas
Copy link
Copy Markdown
Contributor

@mikaelarguedas mikaelarguedas commented Mar 22, 2019

Rebased and modified version of #49

This PR includes the commits of #51 #52 #53 #54
It builds on top of the commits from #49 and will replace #49

Once the PR it depends on it'll be rebased to include only the following changes

Relevant Changes:

  • original commits from 49 adding the nightly template:
    99bf866 and 7331d8e
  • declare wget as template dependency: 56835e2
  • use template to install bootstrapping packages 1c4ec0d
  • make ROS2_BINARY_URL a build arg d9b4476
  • provide mechanism to add rosdep rule files via rosdep_override: 4ccbd61
  • use snippet to invoke rosdep install 4ccbd61
  • install ros-ROS_DISTRO-ros-workspace package to get setup files 4ccbd61
  • set ROS_PACKAGE_PATH to trick rosdep into finding ROS2 packages b2c1ea9

Resulting Dockerfile available at osrf/docker_images#247

@[if 'rosdep' in locals()]@
@[ if 'install' in rosdep]@
@(TEMPLATE(
'snippet/install_rosdep_dependencies.Dockerfile.em',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back to using the snipper here for consistency with other templates and config. I can revert back to e161765 if preferred

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to --ignore-src? And how are we now --skip-keys?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to --ignore-src?

Yes, without it we would install the debs for all the rosdep keys listed even if there is a version of the packages already built in the workspace.
In the case of the nightly image, this would result in downloading the crystal debs of all ROS packages listed as deps in ros2.repos.

And how are we now --skip-keys?

For the nightly, the following keys will still be needed:

  • libopensplice69
  • rti-connext-dds-5.3.1

Thy can be removed if we want to provide users with multiple rmw implementations

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thy can be removed if we want to provide users with multiple rmw implementations

I wouldn't be opposed to this. How much larger would this make the docker image? We'd have to add all that harry logic to accept RTI's debian installation. Also, does that licence prohibit distribution as an installed package? On the other hand, perhaps we should just let the user install it if they need it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, perhaps we should just let the user install it if they need it.

It seems it was the stand taken for all ROS 2 images so far. I'm fine providing only the default implementation in the image and rely on other to opt-in additional vendors (as opposed to opt-out).
I do not remember how permissive the licence of the RTI package is, I believe it is for non commercial purposes only and does not include some components like security plugins.

ruffsl and others added 8 commits March 25, 2019 19:35
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
update rosdep in separate statement to allow none or multiple rule files
use rosdep install like in other templates
install ros-workspace to get ROS 2 setup files

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@mikaelarguedas
Copy link
Copy Markdown
Contributor Author

All blocking PRs have now been merged. THis has been rebased and is ready for review

@ruffsl
Copy link
Copy Markdown
Member

ruffsl commented Mar 25, 2019

Is osrf/docker_images#245 still the latest incarnation of this nightly template?

@mikaelarguedas
Copy link
Copy Markdown
Contributor Author

Is osrf/docker_images#245 still the latest incarnation of this nightly template?

👍

@mikaelarguedas
Copy link
Copy Markdown
Contributor Author

Is osrf/docker_images#245 still the latest incarnation of this nightly template?

+1

I misread this, I believe we were referring to the same PR but just to be sure, the PR with the generated dockerfiles is osrf/docker_images#247 and not osrf/docker_images#245

@ruffsl ruffsl merged commit 5893973 into master Mar 29, 2019
@mikaelarguedas mikaelarguedas deleted the ros2_nightly_rebased branch April 2, 2019 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants