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

Noetic: ros-noetic-octomap (1.9) conflicts with system octomap (1.8) required by libfcl.so #26527

Closed
rhaschke opened this issue Sep 11, 2020 · 33 comments

Comments

@rhaschke
Copy link
Contributor

@sloretz, @ahornung, @tylerjw: I just noticed another issue building MoveIt on Noetic:
ros-noetic-octomap, which provides octomap 1.9 conflicts with system octomap (1.8), now newly required by system libfcl:
On Noetic:

ldd /usr/lib/x86_64-linux-gnu/libfcl.so
	linux-vdso.so.1 (0x00007ffd5733b000)
	liboctomap.so.1.8 => /usr/lib/liboctomap.so.1.8 (0x00007fdd6e2e9000)
	liboctomath.so.1.8 => /usr/lib/liboctomath.so.1.8 (0x00007fdd6e0e2000)
	libccd.so.2 => /usr/lib/x86_64-linux-gnu/libccd.so.2 (0x00007fdd6e0d6000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fdd6df52000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fdd6ddcf000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fdd6ddb5000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fdd6dbf2000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fdd6f07d000)

On Melodic:

ldd /usr/lib/x86_64-linux-gnu/libfcl.so
	linux-vdso.so.1 (0x00007ffe16d39000)
	libccd.so.2 => /usr/lib/x86_64-linux-gnu/libccd.so.2 (0x00007fd591d34000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fd5919ab000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fd59160d000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fd5913f5000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd591004000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fd592a27000)

Our unittests don't complain about broken ABI or the like, but linking two versions of libs into the same binary is never a good idea...
@j-rivero, can you comment why libfcl requires liboctomap in Noetic?

@sloretz
Copy link
Contributor

sloretz commented Sep 11, 2020

It doesn't seem like removing the liboctomap dependency from fcl in Ubuntu version is an option because that would break ABI by removing symbols from libfcl.so. It looks like Octomap is an optional dependency which gets linked to libfcl here. It is needed by libfcl.so because fcl::OcTree<> becomes fcl::OcTree<double> here.

@ros-discourse
Copy link

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/preparing-for-noetic-sync-2020-09-18/16429/3

@jspricke
Copy link
Member

Ubuntu 20.04 provided liboctomap-dev 1.9.3. Could we drop ros-noetic-octomap?

@rhaschke
Copy link
Contributor Author

Thanks for this hint. What about Debian Buster? If that provides octomap 1.9 as well, dropping ros-noetic-octomap might be the solution.

@jspricke
Copy link
Member

No, buster has 1.8.1. We could probably do a backport if you need the newer version (either in buster-backports or into ROS).

@rhaschke
Copy link
Contributor Author

Probably, MoveIt doesn't rely on 1.9 in particular. So, if ros-noetic-octomap is dropped this is probably fine for us. But, other ROS packages might rely on 1.9, which was probably the reason to introduce ros-noetic-octomap in first place.

@jspricke
Copy link
Member

@wxmerkt you added Octomap 1.9.5 in #24245
can you comment on the reasoning?

@wxmerkt
Copy link
Contributor

wxmerkt commented Sep 24, 2020

@jspricke thanks for tagging me - I only saw this issue now. I released octomap v1.9.x into Noetic in March since there's a difference between the versions available in Debian Buster and Fedora 32 (v1.8.1) and the one available in Ubuntu Focal (v1.9.x). The original discussion whether to release or not is captured in the comments following OctoMap/octomap#285 (comment)

@jspricke
Copy link
Member

jspricke commented Sep 24, 2020

I guess we either want a ros-noetic-fcl package or define Octomap 1.8.1 as the minimal required version for Noetic.
I vote for the second option.

@rhaschke
Copy link
Contributor Author

Given the argumentation in #285, we should add a ros-noetic-fcl package. @sloretz, could you please decide this?

@sloretz
Copy link
Contributor

sloretz commented Sep 25, 2020

Given the argumentation in #285, we should add a ros-noetic-fcl package.

It looks like there already is a ros-noetic-fcl-catkin package. It appears to be a newer version than the one released to Focal and to buster. @rhaschke would the issue be resolved if MoveIt and it's dependencies switched to that instead of the rosdep key libfcl-dev? If so, I think that's something we could add to the Noetic Migration guide.

@wxmerkt
Copy link
Contributor

wxmerkt commented Sep 25, 2020

fcl_catkin is not the same as fcl - it's a catkinised package of the bleeding edge of fcl master rather than a 3rd party package as it was required on older distros that did not have the latest fcl. It has all headers moved to subdirectories (fcl_catkin) etc. to not conflict because it was required on older distros. The key for libfcl-dev thus should not be switched to fcl_catkin. I am happy to remove fcl_catkin if ros-noetic-fcl is the latest fcl release to avoid confusion.

@sloretz
Copy link
Contributor

sloretz commented Sep 25, 2020

define Octomap 1.8.1 as the minimal required version for Noetic. I vote for the second option.

@jspricke I would be concerned that downgrading the version of octomap used on Buster would break API/ABI, but I'm not sure that's the case from the changelog

Octomap changelog 1.8.1 -> 1.9.5
v1.9.5: 2020-03-25
==================
- octovis: Update ROS dependencies to Qt5

v1.9.4: 2020-03-15
==================
- octovis: Enable Qt5 by default
- Support added for configurable libdir for packaging
- Increased minimum CMake version to 3.0.2

v1.9.3: 2019-12-26
==================
- Fixed Wpedantic warnings
- Removed problematic INSTALL_NAME_DIR (maxOS workaround)

v1.9.2: 2019-11-28
==================
- Fixed ROS buildfarm failures for octovis

v1.9.1: 2019-11-23
==================
- Improved compatibility with modern compilers and ROS2

v1.9.0: 2017-04-28
==================
- Fixed getUnknownLeafCenters to return true leaf centers (thx to A. Ecins)
- dynamicEDT3D templated over OctoMap type (thx to J.V. Gomez)
- Added optimized rendering option and command line option for tree cutoff 
  in octovis (thx to F. Endres)
- Added optional Qt5 support in octovis (thx to K. Stepanas)
- Improved the generation of config.cmake and version.cmake files, make them
  relocatable via CONFIGURE_PACKAGE_CONFIG_FILE (thx to J.V. Gomez)
- Enable rpath on OS X when the CMake version supports it (thx to J. Snape)
- Added version information to *-config.cmake files and exported targets to
  CMakeLists.txt (thx to J. Snape)
- Improved CMake build by specifically targeting local includes (thx to 
  C.-E. Hrabia)

Also, if ros-noetic-octomap was removed from the apt repo now, I think there will still be developer machines and robots with that package installed on them. Say they upgraded their installed MoveIt packages to ones that use system octomap, but they have other packages they built depending on the installed ros-noetic-octomap. I think if they use MoveIt and Octomap in the same library then they would see the same problem of two versions of octomap.

The key for libfcl-dev thus should not be switched to fcl_catkin.

@wxmerkt It sounds like it's not a drop in replacement, but it could be used if by downstream packages if they changed the headers they include fcl from.

@sloretz, could you please decide this?

@rhaschke I can offer recommendations, but whoever actually does the work gets to make the decisions :) . It sounds like the options discovered so far are @wxmerkt could unrelease octomap in Noetic, MoveIt maintainers could migrate to fcl_catkin, or someone could create and maintain a ros-noetic-fcl. Personally I would recommend the last option. Would the MoveIt maintainers be willing to take that on?

@jspricke
Copy link
Member

Note that you probably also want to maintain dart and mrpt then:

$ apt rdepends liboctomap1.9
liboctomap1.9
Reverse Depends:
  Depends: liboctomap-dev (= 1.9.3+dfsg-2)
  Depends: octovis (>= 1.9.3+dfsg)
  Depends: octomap-tools (>= 1.9.3+dfsg)
  Depends: liboctovis1.9 (>= 1.9.3+dfsg)
  Depends: libdart6 (>= 1.9.3+dfsg)
  Depends: libmrpt-maps2.1 (>= 1.9.3+dfsg)
  Depends: libfcl0.5

@sloretz
Copy link
Contributor

sloretz commented Sep 25, 2020

Note that you probably also want to maintain dart and mrpt then:

@jlblancoc FYI it looks like this discussion might affect the ROS Noetic release of mrpt2
@mxgrey FYI this might affect dartsim + ROS users

@ahornung
Copy link
Contributor

There were mostly buildsystem-related changes from 1.8.1 to 1.9.x, so the API could be compatible to 1.9.5, while the ABI probably is not (I'm not 100% sure though). The CMake usage could have changed as well, that was the reason for the version increase.

@rhaschke
Copy link
Contributor Author

As we are concerned about ABI, we should go for a new ROS package ros-noetic-fcl. I will have a look at this later this evening.

@rhaschke
Copy link
Contributor Author

Now I'm totally confused: The dependency of libfcl on liboctomap is gone on my 20.04 box now (in contrast to #26527 (comment) and #26527 (comment)):

$ apt rdepends liboctomap1.9
liboctomap1.9
Reverse Depends:
  Depends: liboctomap-dev (= 1.9.3+dfsg-2)
  Depends: octovis (>= 1.9.3+dfsg)
  Depends: octomap-tools (>= 1.9.3+dfsg)
  Depends: liboctovis1.9 (>= 1.9.3+dfsg)
  Depends: libdart6 (>= 1.9.3+dfsg)

How is this possible?

@jlblancoc
Copy link
Contributor

I'm not sure of following the discussion to the point of understanding what's the preferred action to take next, but regarding this:

@jlblancoc FYI it looks like this discussion might affect the ROS Noetic release of mrpt2

the mrpt2 package's dependency reported by @jspricke above

$ apt rdepends liboctomap1.9
liboctomap1.9
Reverse Depends:
  Depends: liboctomap-dev (= 1.9.3+dfsg-2)
...
  Depends: libmrpt-maps2.1 (>= 1.9.3+dfsg)

makes reference to the Ubuntu's system version of mrpt2, not the Noetic ros package which, as I just checked here had its dependency on octomap removed 2 years ago here because ros2 didn't support octomap by that time (and we apparently forgot to re-enable it!)... so, the Noetic mrpt2 package actually would not even notice any action taken on the octomap package, if I understand the situation correctly.

@rhaschke
Copy link
Contributor Author

I have got MoveIt compiling against the most recent libfcl 0.6.1. As fcl provides a package.xml since version 0.6.0, it should be easy to release it as a 3rd party package into ROS. @v4hn: What do you think about this approach?
@sloretz: Is it possible to depend on the ROS package (ros-noetic-fcl) in Noetic, but on libfcl-dev in Melodic? I guess, the rosdep key "fcl" should point to libfcl-dev on Melodic (and below) to get this working?

@simonschmeisser
Copy link
Contributor

why did the current version 0.6.1 only get released to debian experimental but not testing (and therefore noetic)? @j-rivero could this be promoted for future versions?

@jspricke
Copy link
Member

@simonschmeisser afaik there is no noetic for Debian testing. Packages are uploaded to experimental for testing or unstable from where they transition to testing. fcl has build problems on a lot of architectures which need to be resolved first: https://buildd.debian.org/status/package.php?p=fcl&suite=experimental. Anyway, it would not help us for Noetic.

@simonschmeisser
Copy link
Contributor

simonschmeisser commented Sep 26, 2020

@jspricke sorry for my lax wording and thanks for the explanations. I saw in the changelog for the debian packaging that 0.6.1 was uploaded to experimental in time for propagation to Ubuntu Focal (which I identified with ROS Noetic) and was therefore wondering why it wasn't uploaded to Debian Unstable/Testing from where it would have propagated. Thanks for linking to buildd

Some of those build failures look worrying ...

Upstream issue: flexible-collision-library/fcl#474

@sloretz
Copy link
Contributor

sloretz commented Sep 30, 2020

@sloretz: Is it possible to depend on the ROS package (ros-noetic-fcl) in Noetic, but on libfcl-dev in Melodic? I guess, the rosdep key "fcl" should point to libfcl-dev on Melodic (and below) to get this working?

@rhaschke You mean, from the same branch on the source repo? IIRC I think conditional dependencies can do it

<depend condition="$ROS_DISTRO == melodic">libfcl-dev</depend>
<depend condition="$ROS_DISTRO == noetic">fcl</depend>

@rhaschke
Copy link
Contributor Author

rhaschke commented Oct 1, 2020

Ping @v4hn: Could you please vote for sticking with FCL 0.5 or upgrading to FCL 0.6 in Noetic?

@v4hn
Copy link
Contributor

v4hn commented Oct 1, 2020

TLDR: In order to have users rely on the current code-base by default it makes a lot of sense to have a ros-specific fcl 0.6 package and I do support this.

Ping @v4hn

Sorry, I did not think you would wait for my input here. I always build MoveIt with a custom FCL 0.6 checkout myself.
0.5 has been obsolete for years and there are many fixes/changes in the newer release.
But it's non-trivial to integrate the package correctly in the workspace as even some of my colleagues have problems setting it up because there is a chance the system headers might be preferred somewhere.

The test errors of FCL 0.6 on non-AMD64 systems (notably i386 and arm64) should obviously be addressed and it's sad to see upstream did not even properly comment on them in 4 months. Most of them do not seem relevant for MoveIt though, e.g., we don't have capsule support and exceeding thresholds of something like 1e-9 by 5e-10 is not necessarily a problem for planned trajectories, assuming these thresholds can be interpreted in Cartesian space.
The most worrying failure I could find, FCL_GJK_EPA.ComputeVisiblePatchColinearNewVertex, is documented with

The behavior is sensitive to numerical accuracy issues

So I'm not surprised to see it fail if other tests have such problems as well.

At least the failing capsule tests seem to be new in 0.6 and the same issues might exist in 0.5..

All in all I'm a big fan of using system libraries wherever possible, but in this case the update got blocked (mostly) because of numeric stability issues on ARM64 which might exist in the old version as well. So I prefer to go with a 0.6 ros specific package.

@jspricke
Copy link
Member

jspricke commented Oct 1, 2020

Now I'm totally confused: The dependency of libfcl on liboctomap is gone on my 20.04 box now (in contrast to #26527 (comment) and #26527 (comment)):

$ apt rdepends liboctomap1.9
liboctomap1.9
Reverse Depends:
  Depends: liboctomap-dev (= 1.9.3+dfsg-2)
  Depends: octovis (>= 1.9.3+dfsg)
  Depends: octomap-tools (>= 1.9.3+dfsg)
  Depends: liboctovis1.9 (>= 1.9.3+dfsg)
  Depends: libdart6 (>= 1.9.3+dfsg)

How is this possible?

I guess because Ubuntu is linking with --as-needed (and Debian switched to it as well after buster). So if you look at the last build log in Debian: https://buildd.debian.org/status/fetch.php?pkg=fcl&arch=amd64&ver=0.5.0-5%2Bb2&stamp=1564953366&raw=0 the dependency is there. But Ubuntu did a rebuild in March where it's gone: https://launchpadlibrarian.net/470268571/buildlog_ubuntu-focal-amd64.fcl_0.5.0-5build1_BUILDING.txt.gz

(seems like as-needed is not visible from the build log)

@mxgrey
Copy link
Contributor

mxgrey commented Oct 1, 2020

@sloretz Thanks for pointing this out to me!

Regarding DART, building DART from source has been compatible with both FCL 0.5 and FCL 0.6 since something like DART 6.4 (using various #if techniques), so I don't have any particular worries about whether FCL 0.5 or 0.6 is chosen. However, I believe there have been very significant improvements in some aspects of FCL 0.6, so I would second the opinion put forward by @v4hn .

@rhaschke
Copy link
Contributor Author

rhaschke commented Oct 1, 2020

@mxgrey, building DART from source shouldn't be a problem with neither version of FCL. However, the released binary package necessarily builds against the system FCL 0.5. If a downstream package/user links MoveIt (using FCL 0.6 in future) and DART in a single program, the FCL libs will obviously conflict. I think that's what was pointed out in #26527 (comment) and #26527 (comment).

@mxgrey
Copy link
Contributor

mxgrey commented Oct 1, 2020

We had a ros-kinetic-dartsim in the Kinetic distro. We could add a ros-noetic-dartsim that builds off of ros-noetic-fcl to deal with that use case.

@traversaro
Copy link

We had a ros-kinetic-dartsim in the Kinetic distro. We could add a ros-noetic-dartsim that builds off of ros-noetic-fcl to deal with that use case.

Note that w.r.t. to the past Gazebo binaries now link against dart (see gazebosim/gazebo-classic#2752), so adding a non-ABI compatible ros-noetic-dartsim could create problems at that level, unless a ros-noetic-gazebo package is added as well.

@peci1
Copy link
Contributor

peci1 commented Apr 27, 2021

I'm now adding FCL-related notes to http://wiki.ros.org/action/login/noetic/Migration . Please, keep in mind that if octomap is also released into noetic as a ROS package, somebody should add it to the migrations notes, too. And maybe (as this is mid-live distro), write about it to discourse.

@rhaschke
Copy link
Contributor Author

Having released ros-noetic-fcl, I think this can be closed now.

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

No branches or pull requests