Clean up finding external dependencies and use imported targets where available #196
Conversation
|
Note that master is currently broken: https://travis-ci.org/flexible-collision-library/fcl/builds/188952125. |
| if(NOT Eigen3_FOUND) | ||
| find_package(Eigen3 3.0.5 QUIET MODULE) | ||
| set(Eigen3_FOUND ON) | ||
| endif() |
There was a problem hiding this comment.
Why do we need to find packages in config mode and module mode explicitly? I though find_package() is for it. Is this for searching the config file first? If so, why is that necessary?
There was a problem hiding this comment.
Because CMake find_package uses MODULE mode first and falls back to CONFIG mode, which unfortunately is the opposite to what we would want.
There was a problem hiding this comment.
Why do we want to search in that order then?
There was a problem hiding this comment.
The config file is maintained by Eigen will adapt to any changes that they make. Also it defines imported targets. The reason that find_package defaults to the other direction so as to not unknowingly break existing projects.
| set(CCD_LIBRARIES "${CCD_LIBRARY}" "${M_LIBRARY}") | ||
| set(ccd_FOUND ON) | ||
|
|
||
| mark_as_advanced(CCD_INCLUDE_DIR CCD_LIBRARY) | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
I would like to suggest encapsulating this logic (and for other dependencies as well) as a separate custom module like FindCCD.cmake. That would make this main CMakeLists.txt cleaner.
There was a problem hiding this comment.
We could do this later if needed. I'm merging this PR now.
pkg-configas input tofind_library()andfind_path()calls so that CMake is always provided with the absolute paths to libraries, as is recommended. Also avoids the need to use the ambiguous and frowned uponlink_directories().IMPORTEDtargets for external libraries where available. Recent versions of Eigen, libccd, and OctoMap all provide these. IfIMPORTEDtargets are not available*_INCLUDE_DIRSand*_LIBRARIESvariables are used, as previously.INTERFACE_INCLUDE_DIRECTORIESforfclwith the locations of all necessary external headers via eithertarget_link_libraries()forIMPORTEDtargets ortarget_include_directories()otherwise. The locations of the headers for libccd and OctoMap were missing before. Also removes redundant calls toinclude_directories().eigen3,libccd, andoctomap, if necessary, to theRequiresfield offcl.pc. All were missing before. Also adds the missingDescription.-gfrom the compiler flags always passed to GCC. TheDebugandRelWithDebInfoCMake build types exist for this purpose.