Skip to content
This repository has been archived by the owner on Nov 13, 2017. It is now read-only.

Dependencies on fcl->octomap #299

Closed
anderwm opened this issue Jun 22, 2016 · 35 comments
Closed

Dependencies on fcl->octomap #299

anderwm opened this issue Jun 22, 2016 · 35 comments

Comments

@anderwm
Copy link

anderwm commented Jun 22, 2016

Not sure if there is any motivation to have this package on Kinetic. However, core doesn't seem to compile with the newest version of fcl, which is needed to support the newer version of octomap (1.8), which is required in kinetic. On first glance it looks like fcl has moved from boost to c++ 11.
flexible-collision-library/fcl#129
http://wiki.ros.org/kinetic/Migration

It's also possible I'm not doing something right.

@davetcoleman
Copy link
Member

We certainly need the kinetic-devel branch of MoveIt! to work with the version of FCL and octomap included in ROS Kinetic. I do not think many of us have spent much time working with Kinetic yet but keep us updated if you figure out the necessary fixes. Thanks for reporting!

@anderwm
Copy link
Author

anderwm commented Jun 23, 2016

FCL isn't included in ROS Kinetic, at least not yet. But it takes their newest version to work with octomap 1.8 which is in Kinetic. The problem does seem to be that all of FCL's constructors and function calls now require c++11 things instead of the boost things that currently exist in moveit collision_detection_fcl

@v4hn
Copy link
Contributor

v4hn commented Jun 23, 2016

Which version of FCL are you talking about?
Their latest release is 0.4.0 (https://github.com/flexible-collision-library/fcl/releases)

And that one works fine even with indigo-devel over here.

@anderwm
Copy link
Author

anderwm commented Jun 23, 2016

I'm referring to their latest commit, after they merged in the code to be compatible with octomap 1.8. Link to the PR above.

@v4hn
Copy link
Contributor

v4hn commented Jun 23, 2016 via email

@tfoote
Copy link
Contributor

tfoote commented Jun 23, 2016

octomap is released by @ahornung from what I see it does not depend on fcl: https://github.com/OctoMap/octomap/blob/devel/octomap/package.xml fcl just needed to be compatible with the same build flags.

In general our policy is to use upstream versions by default. There is already a rosdep rule: https://github.com/ros/rosdistro/blob/master/rosdep/base.yaml#L1433-L1436

0.3.* is available on wily, xenial, and jessie:
http://packages.ubuntu.com/xenial/libfcl-dev
https://packages.debian.org/jessie/libfcl-dev

@scpeters you added the libfcl-dev rule to rosdep. Do you know if it's in use yet?

@scpeters
Copy link
Contributor

Yes, libfcl-dev is used by moveit_core as of #288, which corresponds to the 0.3 versions available upstream.

@scpeters
Copy link
Contributor

I could add, though, that moveit_core is the only ROS package that I know of that depends directly on fcl.

@davetcoleman
Copy link
Member

just a note - i do not think anyone is using the kinetic-devel branch yet that @scpeteres created, because its 34 commits behind indigo, so no one has tested the new fcl dependency yet.

@v4hn
Copy link
Contributor

v4hn commented Jun 24, 2016

@tfoote: sorry, you got it the wrong way around.
_fcl_ optionally depends on _octomap_.

Because moveit_core in kinetic would use xenial's libfcl
(and its octomap support) but kinetic's octomap 1.8, this breaks.
Incidentally, xenial's libfcl even has its octomap support disabled.
We need a not-yet-released version of libfcl (that will support
octomap 1.8) in kinetic, with octomap support enabled, to support
moveit_core there. Only then it makes sense to adhere
to the new libfcl interfaces in moveit_core's kinetic-devel branch.

This definitely blocks a moveit_core release in kinetic.

@anderwm
Copy link
Author

anderwm commented Jun 24, 2016

@v4hn Thanks, that is exactly what I had found, but couldn't describe nearly that well.

@scpeters
Copy link
Contributor

I'll add briefly that xenial has octomap 1.6.8. I don't know why the Ubuntu version of libfcl doesn't use that octomap version, though someone mentioned that it requires octomap 1.8.

cc: @j-rivero

@j-rivero
Copy link

I don't know why the Ubuntu version of libfcl doesn't use that octomap version, though someone mentioned that it requires octomap 1.8.

Speaking from the point of view of the maintainer of fcl, we can add optional support at any time if we have a good reason to do it. Current Debian testing, Debian unstable and Ubuntu Yakkety are using 1.8 octomap.

@v4hn
Copy link
Contributor

v4hn commented Jun 27, 2016 via email

@j-rivero
Copy link

@j-rivero: Well, if you ever want to see moveit_core using debian's libfcl package,
we will need octomap support in there. I suppose this is "a good reason"?

Removing bundles from other libraries and share them from a system installation looks like a good reason to me. I will change the Debian Sid package and the change will be propagated to Ubuntu (Yaketty).

@davetcoleman
Copy link
Member

I just finished getting MoveIt! to compile on Kinetic with @scpeters' commit #288 - are we sure FCL needs to be released with octomap support? Note I have not testing anything during runtime.

@anderwm
Copy link
Author

anderwm commented Jun 27, 2016

I was not using the kinetic branch of Core, as it was many commits behind, but are you sure it is using octomap 1.8?

@davetcoleman
Copy link
Member

I had to sync indigo into kinetic - will push the changes after #300 is merged

Yes, octomap 1.8 as that is what has been released in kinetic

@v4hn
Copy link
Contributor

v4hn commented Jun 27, 2016

You seem to be right.

It actually compiles successfully,
I just checked on a fresh xenial just to be sure.

The trouble is, collision_detection_fcl makes use
of headers of the fcl that should only be accessible
when the library was compiled with octomap support
(without octomap support some functions in these
header files are not compiled into the library and
some headers behave differently).
I opened a merge request for this at the fcl project
and the devs seem to agree with me as it is merged now:
flexible-collision-library/fcl#136

I'm not sure whether moveit_core could run with this
swapped-in octomap and "disabled" octomap support in fcl,
but from a distribution maintainer point of view it is rather
horrible to have this setup running instead of the simple
dependency chain "moveit_core -> fcl -> octomap".

The trouble is that we can't release moveit_core in kinetic unless
(1) fcl pushes out a release and osrf packages it
or (2) we are sure this unstable setup is working as expected
in xenial and we fix it in ubuntu-y.

@davetcoleman
Copy link
Member

Sounds like a question for @j-rivero

@j-rivero
Copy link

Sounds like a question for @j-rivero

Without looking very deeply into the problem, I mostly agree with @v4hn diagnostic. It could be the case that things are working now thanks to a bug. We should fix the bug and get the correct chain of dependencies (avoiding bundles) in place. If moveit_core does not need to know anything about octomap and only cares about fcl then the proposed solution looks good to me.

@v4hn
Copy link
Contributor

v4hn commented Jun 28, 2016

On Tue, Jun 28, 2016 at 07:08:53AM -0700, Jose Luis Rivero wrote:

It could be the case that things are working now thanks to a bug.
We should fix the bug and get the correct chain of dependencies (avoiding bundles) in place.

As far as I can see, this would mean that we have to wait for a new fcl release,
build it with octomap support and bundle it with kinetic instead of using
the current debian package. Do you agree?

If moveit_core does not need to know anything about octomap
and only cares about fcl then the proposed solution looks good to me.

I'm not sure I understand what you mean.
moveit_core directly includes octomap in multiple places.
Additionally it uses headers of fcl that include octomap (even if fcl was not built with octomap support).
So moveit_core does need to know about octomap.
Directly and indirectly.

@ahornung
Copy link

Is there anything needed in OctoMap to ease transition or is this simply a matter of depending on the right version (ROS release vs. debian release)?

Are you actually using parts of the OctoMap API that changed in 1.8? Otherwise there shouldn't be a problem using the new version besides the different ABI.

@j-rivero
Copy link

As far as I can see, this would mean that we have to wait for a new fcl release, build it with octomap support and bundle it with kinetic instead of using the current debian package. Do you agree?

If look for a short-term solution (something that affects releases previous to Yaketty) then yes, we should find a custom solution within the scope of the ROS repos.

I'm not sure I understand what you mean. moveit_core directly includes octomap in multiple places. Additionally it uses headers of fcl that include octomap (even if fcl was not built with octomap support). So moveit_core does need to know about octomap. Directly and indirectly.

The dependency chain should be something like:
moveit -> octomap, fcl. | fcl -> octomap (right?)

@anderwm
Copy link
Author

anderwm commented Jun 30, 2016

I also was able to get the kinetic-devel branch to build (except warehouse but that's a different problem) from @davetcoleman 's fork. However, I don't see how it can possibly work as intended. libfcl-dev (0.3.2-1) is released into Xenial, and (I believe) that version has function calls to octomap that don't exist in octomap 1.8.

It seems like you either have to have a version of fcl that has the octomap 1.8 fixes but not the purging of Boost, or you have to support the new c++11 fcl calls in moveit_core.

Obviously you guys are way out in front of me on this, so probably I'm just missing something?

Also I thought the dependency chain more like:
moveit_core -> fcl -> octomap
-> octomap
moveit_everything_else -> octomap

@v4hn
Copy link
Contributor

v4hn commented Jun 30, 2016

On Thu, Jun 30, 2016 at 03:25:48AM -0700, Armin Hornung wrote:

Is there anything needed in OctoMap to ease transition
or is this simply a matter of depending on the right version (ROS release vs. debian release)?

I don't think so. Octomap 1.8 is not an issue at all here.
The fact that kinetic features Octomap 1.8 when there is no FCL
release that supports this new version yet is an issue though.
If you know one of the FCL maintainers, that would be pretty helpful
to get information on their plans to release because we will probably
have to wait for their next release. (see referenced issue)

Are you actually using parts of the OctoMap API that changed in 1.8?
Otherwise there shouldn't be a problem using the new version besides the different ABI.

As moveit can be compiled successfully, there doesn't seem to be a problem there.

@v4hn
Copy link
Contributor

v4hn commented Jun 30, 2016

On Thu, Jun 30, 2016 at 04:18:19AM -0700, Jose Luis Rivero wrote:

If look for a short-term solution (something that affects releases previous to Yaketty) then yes, we should find a custom solution within the scope of the ROS repos.

Ok.

The dependency chain should be something like:
moveit -> octomap, fcl. | fcl -> octomap (right?)

moveit_core instead of moveit, but yes.

@v4hn
Copy link
Contributor

v4hn commented Jun 30, 2016

On Thu, Jun 30, 2016 at 07:28:41AM -0700, anderwm wrote:

However, I don't see how it can possibly work as intended.
libfcl-dev (0.3.2-1) is released into Xenial,
and (I believe) that version has function calls
to octomap that don't exist in octomap 1.8.

The debian package libfcl-dev currently does not have any function calls to octomap
in the library, because it was compiled without octomap support. See earlier explanation.

It seems like you either have to have a version of fcl that has the octomap 1.8 fixes but not the purging of Boost,
or you have to support the new c++11 fcl calls in moveit_core.

We will probably wait for the next FCL release, that will include the octomap 1.8 fixes
(among other changes), and then support the new c++11 fcl interfaces from that release in the kinetic branch.
This will resolve your issue.

@j-rivero
Copy link

j-rivero commented Jul 1, 2016

If you know one of the FCL maintainers, that would be pretty helpful to get information on their plans to release because we will probably have to wait for their next release. (see referenced issue)

@jslee02, Do we have any plans to get a new FCL release sometime soon? BTW, we are in a good time to get new releases into Debian so Ubuntu Yaketty can sync during the standard syncing period.

@jslee02
Copy link

jslee02 commented Jul 1, 2016

I don't know if any of other maintainers have plans, but I wouldn't have any objection on releasing FCL 0.5. Also, I can help the release if there is something I can do.

@davetcoleman
Copy link
Member

Earlier in this thread I stated that MoveIt! builds fine with the current misconfiguration of FCL & Octomap dependencies in Kinetic. I had only been testing that MoveIt builds and passes the tests, but there is very little test coverage in MoveIt! right now. Turns out I am getting runtime errors as well, as pointed out by @anderwm

@jslee02 that would be really great if you could take the lead on releasing FCL, see flexible-collision-library/fcl#137

@davetcoleman
Copy link
Member

davetcoleman commented Jul 25, 2016

@jslee02 has just released FCL 0.5 - I am assuming this is what we want to target in Kinetic then since it includes Octomap 1.8 support?

@v4hn
Copy link
Contributor

v4hn commented Jul 25, 2016

@davetcoleman: yes. We need the new version with octomap 1.8 support enabled available in kinetic.
This was the whole plan all along.

@tfoote How do we proceed to get the freshly released libfcl 0.5 in kinetic? We need it for moveit_core.
Afair libfcl was distributed in ROS before, so there should be a release repo for this already, is there?

@tfoote
Copy link
Contributor

tfoote commented Jul 26, 2016

To release into kinetic as a ros package it's just running bloom again. However I'm not sure if we can do it the default way to avoid colliding with the upstream packages. @scpeters @j-rivero @wjwwood I know you've been discussing this.

@mikeferguson
Copy link
Contributor

This is now released and should be good to go

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants