robotology / icub-main

The iCub Main Software Repository
Other
110 stars 103 forks source link

icub_SIM does not compile with ODE 0.15.2_1 on macOS with brew #514

Closed claudiofantacci closed 6 years ago

claudiofantacci commented 6 years ago

Just tried to compile icub-main/devel on macOS High Sierra unsuccesfully.

With ICUBMAIN_COMPILE_SIMULATORS = ON I get:

Undefined symbols for architecture x86_64:
  "_ccdFirstDirDefault", referenced from:
      ccdCollide(dxGeom*, dxGeom*, int, dContactGeom*, int, void*, void (*)(void const*, _ccd_vec3_t const*, _ccd_vec3_t*), void (*)(void const*, _ccd_vec3_t*), void*, void (*)(void const*, _ccd_vec3_t const*, _ccd_vec3_t*), void (*)(void const*, _ccd_vec3_t*)) in libode.a(collision_libccd.o)
  "_ccdMPRIntersect", referenced from:
      ccdCollide(dxGeom*, dxGeom*, int, dContactGeom*, int, void*, void (*)(void const*, _ccd_vec3_t const*, _ccd_vec3_t*), void (*)(void const*, _ccd_vec3_t*), void*, void (*)(void const*, _ccd_vec3_t const*, _ccd_vec3_t*), void (*)(void const*, _ccd_vec3_t*)) in libode.a(collision_libccd.o)
  "_ccdMPRPenetration", referenced from:
      ccdCollide(dxGeom*, dxGeom*, int, dContactGeom*, int, void*, void (*)(void const*, _ccd_vec3_t const*, _ccd_vec3_t*), void (*)(void const*, _ccd_vec3_t*), void*, void (*)(void const*, _ccd_vec3_t const*, _ccd_vec3_t*), void (*)(void const*, _ccd_vec3_t*)) in libode.a(collision_libccd.o)
ld: symbol(s) not found for architecture x86_64

while linking iCub_SIM.

It seems to be related to libccd, which is version 2.0_2 (brew installation) on my system (version 2.0 was released in 2014). While compiling iCub_SIM, I also get a lot of deprectaion warning by one of the macOS Framework (which is a known problem).
Is anyone on macOS High Sierra having this problem? Just to understand if is a problem of my setup or not 😄 (iCub_SIM is not working on macOS, so it is not a big deal).

traversaro commented 6 years ago

I don't think iCub_SIM is calling ccd directly, only through ODE . Are you sure that it is not a problem of your brew installation, with inconsistent versions of ODE and ccd installed on your machine?

claudiofantacci commented 6 years ago

@traversaro thanks for the suggestion. Indeed version 0.15.2_1 from brew seems not to work. I had a previously installed version 0.14 that instead works nicely.

I'll reword this issue and will keep track of future releases of ODE.

traversaro commented 6 years ago

Related issue (at least the initial part): https://github.com/RoboCup-SSL/grSim/issues/64 .

nunoguedelha commented 6 years ago

I've had the exact same issue since I upgraded my brew (was not supposed to upgrade my OS from Sierra to High-Sierra, but upgraded the SDK and other stuff). So the ODE from brew went from version 0.15.2 to 0.15.2_1. Thanks for the details you proveided on the issue. I'll just disable the ICUBMAIN_COMPILE_SIMULATORS.

claudiofantacci commented 6 years ago

@nunoguedelha, just a precisation: were you abel to compile iCub_SIM with version 0.15.2?

nunoguedelha commented 6 years ago

@claudiofantacci yes, as per the brew info I get from my system, ODE version went from 0.15.2 to 0.15.2_1 right before superbuild stopped compiling. But I haven't tried to revert the ODE version to check, doing it now..

For the record, here's what I get from my current config:

$ brew info ode
ode: stable 0.15.2 (bottled), HEAD
Simulating articulated rigid body dynamics
http://www.ode.org/
/usr/local/Cellar/ode/0.15.2 (35 files, 1.8MB)
  Built from source on 2017-10-25 at 12:39:33 with: --with-double-precision
/usr/local/Cellar/ode/0.15.2_1 (35 files, 1.8MB) *
  Built from source on 2018-04-24 at 14:18:10 with: --with-double-precision
From: https://github.com/Homebrew/homebrew-core/blob/master/Formula/ode.rb

$ brew list ode
/usr/local/Cellar/ode/0.15.2_1/bin/ode-config
/usr/local/Cellar/ode/0.15.2_1/include/ode/ (26 files)
/usr/local/Cellar/ode/0.15.2_1/lib/pkgconfig/ode.pc
/usr/local/Cellar/ode/0.15.2_1/lib/libode.a

→ Switch back to ODE 0.15.2:

$ brew switch ode 0.15.2
Cleaning /usr/local/Cellar/ode/0.15.2
Cleaning /usr/local/Cellar/ode/0.15.2_1
4 links created for /usr/local/Cellar/ode/0.15.2

→ Switch flag ICUBMAIN_COMPILE_SIMULATORS back to ON → Rebuild ICUB

ICUB repo compiles again.

claudiofantacci commented 6 years ago

@nunoguedelha that's great! it seems that a formula revision introduced something wrong, at least for our settings.

The revision 1 of the formulae has been made with this PR https://github.com/Homebrew/homebrew-core/pull/24751, setting by default the option --enable-libccd. It seems that there is an incompatibility with ODE v 0.13+ and libccd.

Maybe we should consider setting up our own ODE formula without the --enable-libccd option.

claudiofantacci commented 6 years ago

By modifying the formula of ODE and removing the --enable-libccd option, I was able to succesfully compile iCub_SIM!

@traversaro we should consider makeing our own formula.

claudiofantacci commented 6 years ago

Opened PR https://github.com/robotology/homebrew-formulae/pull/19.

traversaro commented 6 years ago

I think I understood the underlying problem.

Differently from most other icub deps, by default in homebrew ODE is compiled as a static library, while ccd is compiled by default as a shared library. For these reasons, even if ODE does not expose any ccd header, every executable that links the static version of ODE also needs to link the shared version of ccd, i.e. ccd is a public dependency of ODE. However, most CMake projects use some FindODE.cmake script that does not detect this strange corner case (as they are typically written in Linux where ODE is distributed by the distro package manager as a shared library), and consequently does not list the ccd library as a public dependency of ODE target and/or does not add the ccd library to the ODE_LIBRARIES variable:

Note: DART is not affected by this problem because the dart target already links to ccd, so as soon as ODE is linked, ccd is already linked to the library, shadowing the problem.

To be honest, this kind of problem in the case in which a static library depends on some shared libraries I think affects most of the CMake projects that I know of, so I am not sure if it make sense trying to fix it at the FindODE.cmake level or directly at the ODE-generated ode-config.cmake file (ode-config.cmake is not available in homebrew as homebrew still uses the autotools-based configuration of ODE).

An easy solution for this homebrew problem would be to make the ODE library shared by default, for example by switching to use the CMake-based build system instead of the autotools-based one, at least for having the situation in homebrew similar to the one that you obtain in Debian-based Linux distributions when installing ODE and ccd from apt.

Another solution is to modify the FindODE.cmake or the ode-config.cmake file to detect when ODE is compiled as a static library while ccd is compiled as a shared library, and inserting in that case the ccd library in ODE_LIBRARIES or even add it to the INTERFACE_LINK_LIBRARIES property of the ODE target. To be completely I am not aware of any C++ CMake-based projects that actually implement this logic in its build system, so I am not sure as feasible it is.

cc fyi @mxgrey @jslee02 as the problem indirectly affects also the FindODE.cmake shipped in DART and ignition-cmake .

claudiofantacci commented 6 years ago

Well, as far as homebrew is concerned, I would switch to CMake-based build system first, with default option of shared library (which is already the default choice). We can then add an option to switch back to static.

The static ODE + dyanamic ccd will still be a problem. So I guess that either FindODE.cmake or ode-config.cmake need to be updated to cover this possibility. Am I wrong?

traversaro commented 6 years ago

The static ODE + dyanamic ccd will still be a problem. So I guess that either FindODE.cmake or ode-config.cmake need to be updated to cover this possibility. Am I wrong?

In theory yes, in practice I am not aware of any C++ project or Find<package>.cmake module that correctly exports the private dependencies if the library is compiled as static and its dependecies are compiled as dynamic. For Find<package>.cmake in particular, this case is quite tricky: when you find that ode is a static library, how do you know if ccd was used as a static or dynamic library?

claudiofantacci commented 6 years ago

I'm guessing here, as I don't have enough experience about this: CMake target config files should have this kind of information, inside the target as a property. I (almost) understand that the Find<package>.cmake may not hold this. At a certain point that dependency information was available and should be added to the compiling library and exported accordingly so that proper linking can be made by the end-user.

Setting aside this interesting discussion, we may want to have an ODE formula that compiles a dynamic library and eventually we can push it upstream in homebrew-core. Do we agree on this 😄 ?

traversaro commented 6 years ago

I'm guessing here, as I don't have enough experience about this: CMake target config files should have this kind of information, inside the target as a property.

Exactly, but this mean that the <package>-config.cmake would need to have a find_dependency call even for what we usually call "private" dependencies, at least when the libraries in the <package> are built as static and the dependencies are built as shared.

Do we agree on this smile ?

Yes!

claudiofantacci commented 6 years ago

Ok cool!

Then step 0 is to update the PR I made for our own ODE formula. I'll work on it as soon as possible.

DanielePucci commented 6 years ago

While installing the robotology-superbuild, I got the following error instead

[ 64%] Building CXX object src/simulators/iCubSimulation/CMakeFiles/iCub_SIM.dir/wrapper/iCubSimulationControl.cpp.o
In file included from /Users/dpucci/src/robotology-superbuild/robotology/ICUB/src/simulators/iCubSimulation/wrapper/iCubSimulationControl.cpp:33:
In file included from /Users/dpucci/src/robotology-superbuild/robotology/ICUB/src/simulators/iCubSimulation/odesdl/OdeInit.h:32:
In file included from /Users/dpucci/src/robotology-superbuild/robotology/ICUB/src/simulators/iCubSimulation/odesdl/iCub.h:35:
In file included from /Users/dpucci/src/robotology-superbuild/robotology/ICUB/src/simulators/iCubSimulation/odesdl/rendering.h:32:
In file included from /usr/local/include/ode/ode.h:30:
/usr/local/include/ode/common.h:59:2: error: You can only #define dSINGLE or dDOUBLE, not both.
#error You can only #define dSINGLE or dDOUBLE, not both.
 ^
1 error generated.
make[5]: *** [src/simulators/iCubSimulation/CMakeFiles/iCub_SIM.dir/wrapper/iCubSimulationControl.cpp.o] Error 1
make[4]: *** [src/simulators/iCubSimulation/CMakeFiles/iCub_SIM.dir/all] Error 2
make[3]: *** [all] Error 2
make[2]: *** [robotology/ICUB/CMakeFiles/YCMStamp/ICUB-build] Error 2
make[1]: *** [CMakeFiles/ICUB.dir/all] Error 2
make: *** [all] Error 2
traversaro commented 6 years ago

PR related to ODE / ccd homebrew formulas : https://github.com/Homebrew/homebrew-core/pull/28434 .

claudiofantacci commented 6 years ago

Closing after merged https://github.com/robotology/homebrew-formulae/pull/19