Skip to content

rosdep: add Ignition Citadel#24934

Merged
nuclearsandwich merged 6 commits intoros:masterfrom
chapulina:citadel
Jun 17, 2020
Merged

rosdep: add Ignition Citadel#24934
nuclearsandwich merged 6 commits intoros:masterfrom
chapulina:citadel

Conversation

@chapulina
Copy link
Copy Markdown
Contributor

@chapulina chapulina commented May 16, 2020

Some of these keys are already being used by the ros_ign packages to bloom it into https://packages.osrfoundation.org:

https://github.com/osrf/osrf-rosdep/blob/master/ignition/ignition.yaml


This is a follow up to #24646

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina requested a review from a team as a code owner May 16, 2020 01:43
Copy link
Copy Markdown
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

I don't believe these packages are available for older supported versions of Debian or Ubuntu.

I think these keys need to be rewritten so they're scoped to just focal and buster. e.g.

ignition-citadel:
  debian:
    buster: [ignition-citadel]
    '*': null
  ubuntu:
    focal: [ignition-citadel]
    '*': null

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented May 18, 2020

'*': null

@nuclearsandwich does the null need to be explicitly stated? If so, should I add it to #24646 ?

@nuclearsandwich
Copy link
Copy Markdown
Member

@nuclearsandwich does the null need to be explicitly stated? If so, should I add it to #24646 ?

No you're right. I had my wires crossed. the explicit null is used when they key is defined for most distributions but not one particular one. So the PR could either assume forward compatibility:

keyname:
  debian:
    '*': [packagename]
    jessie: null
    stretch: null
  ubuntu:
    '*': [packagename]
    bionic: null
    xenial: null

or not:

keyname:
  debian:
    buster: [packagename]
  ubuntu:
    focal: [packagename]

Given the fact that these packages are versioned and many of them are being imported directly not upstreamed I'd suggest the latter form.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Copy Markdown
Contributor Author

scoped to just focal and buster

done

Comment thread rosdep/base.yaml Outdated
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Comment thread rosdep/base.yaml Outdated
Comment thread rosdep/base.yaml
@clalancette clalancette added the rosdep Issue/PR is for a rosdep key label May 22, 2020
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@nuclearsandwich
Copy link
Copy Markdown
Member

@chapulina it looks like there are some outstanding checks failing. Alphabetical ordering is at least part of it.

   ERR: list out of alphabetical order line 2609.  'libignition-math6-dev' should come before 'libignition-math6-eigen3'
  ERR: list out of alphabetical order line 2639.  'libignition-plugin-dev' should come before 'libignition-plugin1'

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Copy Markdown
Contributor Author

Sorry for overlooking that, @nuclearsandwich . Updated in 73aa168

Copy link
Copy Markdown
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

A note for fellow reviewers: Not all of these packages are available upstream. Those that are not have been imported into the ROS buildfarm bootstrap repository directly (see ros-infrastructure/reprepro-updater#75)

@nuclearsandwich nuclearsandwich requested a review from sloretz June 15, 2020 20:25
Comment thread rosdep/base.yaml Outdated
Signed-off-by: Louise Poubel <louise@openrobotics.org>
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.

5 participants