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

Add rosdep update options for EOLs #316

Merged

Conversation

chikuta
Copy link

@chikuta chikuta commented Aug 31, 2019

rosdep update command require additional option "--include-eol-distros" for eol packages like trusy.
This option was merged by this pull request (ros-infrastructure/rosdep@ae0e89a) .

Please feel free to comment what to improve, I may have done something wrong as this is my first pull request.

@chikuta chikuta force-pushed the add_rosdep_update_option_for_eols branch from d993df8 to c762fb3 Compare September 22, 2019 03:18
Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

Thanks @chikuta for the PR! and sorry you didnt receive feedback earlier.

This is the correct fix for indigo and jade 👍. I'll merge this as is and will ticket the need of a more global solution for all EOL ROS distributions.

@ruffsl
Copy link
Member

ruffsl commented Oct 19, 2019

We'll want to make sure the dockerfile changes propagate to images on docker hub as well.

@mikaelarguedas
Copy link
Contributor

We'll want to make sure the dockerfile changes propagate to images on docker hub as well

This is not obvious to me, can you clarify why ?

From my perspective it looks like:

  • the images on dockerhub had their rosdep database updated before going EOL. I would actually prefer NOT updating it as it reflects the state of the distro before it went down.
  • not sure we can convince the librarian to reenable trusty and older builds for us just for this.

Related to the first bullet, breaking changes have been integrated in the rosdep database since the distros went EOL (e.g. ros/rosdistro#19843) and there is a discussion to how to safely update rosdep for EOL distros going on at ros/rosdistro#18296

@mikaelarguedas
Copy link
Contributor

@ruffsl I opened #327 as a discussion ticket for updating rosdep to EOL distros

@ruffsl
Copy link
Member

ruffsl commented Oct 19, 2019

Ah, good points. I had forgot the old db is already archived within the image. If users want to invoke rosdep update downstream, then they'll need to tweek their Dockerfile to include the --include-eol-distros arg anyhow, but this doesn't impact the viability of the base images already archived. I was also thinking this only affects the legacy images, which are hosted entirely under the osrf docker hub org, but time has marched on such that EOL now include distros released in the official docker hub library.

not sure we can convince the librarian to reenable trusty and older builds for us just for this.

No, probably not, nor do I think this would warrant us to necromance those official library builds.

@mikaelarguedas
Copy link
Contributor

Merging this to credit original contributor,
opened #327 and #330 as follow up

@mikaelarguedas mikaelarguedas merged commit 85ac8f1 into osrf:master Oct 22, 2019
@chikuta
Copy link
Author

chikuta commented Oct 23, 2019

@mikaelarguedas @ruffsl
My bad, I didn't notice these posts.
Thank you for your kindness. Next time, I will make a ticket at first.

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.

3 participants