Skip to content

Update Ignition rosdep keys#25789

Merged
tfoote merged 2 commits intoros:masterfrom
chapulina:chapulina/update_ignition
Jul 20, 2020
Merged

Update Ignition rosdep keys#25789
tfoote merged 2 commits intoros:masterfrom
chapulina:chapulina/update_ignition

Conversation

@chapulina
Copy link
Copy Markdown
Contributor

Originally on #24934 we added rosdep keys that matched the debian package names as recommended by the reviewers: #24934 (comment).

However, this makes usage of these keys complicated when compiling Ignition libraries (pure CMake packages) and ROS packages together in the same colcon workspace (our team often does this).

Here's an example that can be in the same colcon workspace as Ignition:

  <depend>ignition-msgs5</depend>
  <depend>ignition-transport8</depend>

However, the above doesn't work with rosdep install. Ideally, we'd declare Ignition dependencies once, and they would work with either rosdep or colcon.

Therefore, I propose here that we change the -dev keys to match the CMake project names.

Let me know if this is a big taboo and we can discuss alternatives.

CC @j-rivero @sloretz @dirk-thomas

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina requested a review from a team as a code owner July 13, 2020 20:29
@dirk-thomas
Copy link
Copy Markdown
Member

Sounds reasonable to me. (To pass CI the renamed entries must be moved into alphabetical order.)

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Copy link
Copy Markdown
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.

LGTM

@nuclearsandwich
Copy link
Copy Markdown
Member

@tfoote has been the driving editorial force behind rosdep keys. I'd like to give him the opportunity to review or explicitly abstain from doing so.

Copy link
Copy Markdown
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Yeah, in general we pick the debian names because it's the most generic. However if this can work with the upstream packages from source too then an exception to that makes sense.

The biggest thing though that I'd point out is that if you do this you do not have the ability to explicitly call out the runtime libraries versus the development libraries which is valuable for packing. But I guess in the future we can consider explicitly adding lib* and *-dev keys in parallel. But for now if this works for both source space and rosdep resolutions that sounds like a win.

@chapulina
Copy link
Copy Markdown
Contributor Author

you do not have the ability to explicitly call out the runtime libraries

So... I left the lib* keys in, so people can still use those for runtime dependencies. And I turned the lib*-dev keys into ignition-*. Not sure if it was a good idea, the assumption is that when compiling from source, you often want the -dev packages.

@tfoote
Copy link
Copy Markdown
Member

tfoote commented Jul 20, 2020

That's a good compromise.

@tfoote tfoote merged commit 6ae977e into ros:master Jul 20, 2020
@chapulina chapulina deleted the chapulina/update_ignition branch April 16, 2021 18:21
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.

5 participants