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

Update ruby rosdep key facets for Focal, and add new keys metaruby and utilrb #28155

Merged
merged 4 commits into from
Feb 24, 2021

Conversation

meyerj
Copy link
Contributor

@meyerj meyerj commented Jan 27, 2021

Those packages are dependencies of orogen. Currently it is not planned to release orogen into ROS like it was the case until indigo. But it is still possible to build from source (orocos-toolchain/orogen#140 (comment)), and it would be nice if rosdep can install its dependencies without custom rosdep files like the one we provide in orocos-docker-images/ros2/ubuntu/prereqs.yaml.

Those are gem dependencies. Package ruby-facets is not available anymore for Ubuntu Focal (and newer?) and also needs to be installed as a gem now:

Questions:

  • Probably better to list keys bionic and xenial explicitly for facets, and use '*' for focal and newer? Likely the package won't come back.
  • I did not test all distributions for the new keys, and assumed that gem would work the same way in all major distributions. Only Ubuntu bionic and focal have been actually tested. Do you prefer to only define the keys for Ubuntu, or even the tested versions? If not, is it possible to omit the OS_NAME or just write '*': if all resolve to the same installer and package? It is probably the same for some Python packages which are only available via pip.

@tfoote
Copy link
Member

tfoote commented Jan 28, 2021

Probably better to list keys bionic and xenial explicitly for facets, and use '*' for focal and newer? Likely the package won't come back.

Yes, it's better to list all the old versions explicitly and have the '*' rule carry into the future.

Unfortunately the installer and os name levels don't support wildcard syntax. It's probably better to simply reduce it to the ones you're confident on. The extra lines there if they're unused will just be more to maintain.

@mabelzhang mabelzhang added the rosdep Issue/PR is for a rosdep key label Jan 29, 2021
rosdep/ruby.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

I'm not sure why CI is failing, but I suspect it might be because there already exist rosdistro keys for metaruby and utilrb in indigo.yaml (and older). Maybe we could remove the keys from the EOL distros or prefix the new keys with ruby- (or something). @tfoote @nuclearsandwich Thoughts?

@cottsay
Copy link
Member

cottsay commented Feb 11, 2021

Shouldn't the values being added be lists of package names and not bare strings?

@hidmic
Copy link
Contributor

hidmic commented Feb 11, 2021

@cottsay has a point. @meyerj mind to take a look?

@meyerj
Copy link
Contributor Author

meyerj commented Feb 16, 2021

@cottsay has a point. @meyerj mind to take a look?

Done: 0919587

I thought the list notation is optional, and there are examples without list brackets in the rosdep.yaml spec in REP-111, but it is still good to add them consistently.

@clalancette clalancette merged commit 8cdfdec into ros:master Feb 24, 2021
@meyerj meyerj deleted the rosdep-ruby-facets-metaruby-utilrb branch November 29, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rosdep Issue/PR is for a rosdep key
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants