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

Adding fcl key to solve issues with version 0.6 #27789

Closed
wants to merge 1 commit into from
Closed

Adding fcl key to solve issues with version 0.6 #27789

wants to merge 1 commit into from

Conversation

marcoag
Copy link
Contributor

@marcoag marcoag commented Dec 17, 2020

Fcl in version 0.6 fcl comes with a package.xml where it declares itself as fcl. This creates issues like osrf/rmf_core#234. To solve this I propose an fcl key that points to the corresponding OS library.

Fcl [in version 0.6 fcl comes with a `package.xml`](https://github.com/flexible-collision-library/fcl/blob/da07e5b4b59a99fcc45ea17a7aa786d5e077fdc9/package.xml#L2) where it declares itself as `fcl`. This creates issues like osrf/rmf_core#234. To solve this I propose an `fcl` key that points to the corresponding OS library.
@@ -830,6 +830,8 @@ fcgi:
fedora: [fcgi, mod_fcgid, spawn-fcgi]
gentoo: [dev-ruby/fcgi]
ubuntu: [libfcgi-dev, libapache2-mod-fastcgi, spawn-fcgi]
fcl:
ubuntu: [libfcl0.5]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the contents here perhaps just be a copy/paste of the libfcl-dev entry?

@clalancette clalancette added the rosdep Issue/PR is for a rosdep key label Dec 17, 2020
@jacobperron
Copy link
Contributor

IIUC, if fcl 0.6.0 gets released into the applicable ROS distributions, then this will also fix the issue. Releasing 0.6.0 into ROS seems like a preferred solution. TBH, I'm not sure how our infrastructure handles duplicate keys in rosdep and distribution files (perhaps this is why CI is failing?).

@mxgrey
Copy link
Contributor

mxgrey commented Dec 18, 2020

I think releasing 0.6 should definitely happen at some point, but I don't know if the community is ready for it. It has some significant breaking API changes and isn't as thoroughly tested by users as 0.5.

There's an ongoing discussion on the matter here: #26527

I think making a temporarily redundant key that just maps to the old 0.5 library would be a good way to approach transitioning. Users who can only depend on 0.5 can just keep using the old libfcl-dev key. Users who are ready to support either can jump on the new fcl key. Eventually the fcl key will switch to targeting 0.6, whenever the overall community is ready.

Then again, there may be a fair argument for just taking this opportunity to release fcl 0.6 into ROS as ros-foxy-fcl, which won't have any install conflict with libfcl-dev, and users can choose between 0.5 and 0.6 that way. I don't have a strong opinion on this, but I think it may be worth gathering broader input from the community.

@clalancette
Copy link
Contributor

clalancette commented Dec 18, 2020

Looking at the contributing guide for rosdep, it looks like the rosdep keys should come from the operating system, or from pip. I know for a fact that we actually break that rule in a couple of cases, but I think it is a good guideline to start from.

Given that, I think it is reasonable to assume libfcl-dev is going to point to version 0.5 for quite a while. So my preference here would be to do the release of ros-foxy-fcl into the Foxy distribution. For users who are happy with fcl 0.5, nothing changes (until the underlying OS changes). For users who want to use fcl 0.6, they have to depend on a new key (but they were going to have to make a change anyway).

All of that said, I would welcome valuable input from @nuclearsandwich and @tfoote , since they have dealt with situations like this before.

@hidmic
Copy link
Contributor

hidmic commented Dec 23, 2020

@nuclearsandwich @tfoote friendly ping

@tfoote tfoote self-requested a review December 29, 2020 00:54
@gavanderhoorn
Copy link
Contributor

@rhaschke: would you know whether this will affect MoveIt in any way? I remember us having great problems with FCL in the past.

@rhaschke
Copy link
Contributor

rhaschke commented Jan 7, 2021

FCL 0.6.1 is released into ROS Noetic: http://repositories.ros.org/status_page/ros_noetic_default.html?q=fcl
Adding a generic rosdep key for all distros will definitely break (as indicated by failing CI).
In osrf/rmf_core#234 it was pointed out that version 0.6 is required. If that's the case, it should be released into other ROS distros as well. Packages can still decide to use the system default (by depending on libfcl-dev) or the newer ROS package (depending on fcl).

@mxgrey
Copy link
Contributor

mxgrey commented Jan 11, 2021

I think having a libfcl-dev and a fcl rosdep key live side-by-side where libfcl-dev is for 0.5 and and fcl is for 0.6 sounds like the best solution available (albeit, the difference between the two will be very unclear to anyone who is unaware of the context behind having both keys).

@jacobperron
Copy link
Contributor

jacobperron commented Jan 12, 2021

I think having a libfcl-dev and a fcl rosdep key live side-by-side where libfcl-dev is for 0.5 and and fcl is for 0.6 sounds like the best solution available

To clarify, do you mean adding an fcl key to rosdep/base.yaml or as a per-distro key?

I agree with the latter solution, in which case we can close out this ticket. fcl 0.6.1 is already released for Noetic, and can be released for other distros where needed.

If the former case, I think it gets more confusing as we'd have three different keys, libfcl-dev, fcl (per distro), and a new libfcl0.6 key.

@mxgrey
Copy link
Contributor

mxgrey commented Jan 13, 2021

do you mean adding an fcl key to rosdep/base.yaml or as a per-distro key?

I mean a per-distro key.

in which case we can close out this ticket

Ah, I didn't notice this was targeting rosdep/base.yaml. I agree that we should change this PR add this to foxy/distribution.yaml, or open a new PR altogether with that change.

@jacobperron
Copy link
Contributor

jacobperron commented Jan 15, 2021

I think we have a consensus. fcl should be released into Foxy instead of adding a rosdep key here.

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