Fix the way in which Eigen is included

[haudren]

Currently, Eigen is added via target_include_directories as a PUBLIC
or INTERFACE tag. Unfortunately, this makes a built tesseract
non-reusable on a system where Eigen is installed in a different folder.
For example, Tesseract built on a system where Eigen was downloaded in
the ROS workspace is not usable on a system where Eigen is installed in
the default system directory.

For a few more details:

• I have some binary builds coming from a Docker image, but those are not reusable on the host system :(
• I test by moving my install folder around on various machines

In both cases, CMake errors out with:

CMake Error in CMakeLists.txt:
  Imported target "tesseract::tesseract_environment_core" includes
  non-existent path

    "/home/herve/ros/ws/install/include/eigen3"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

As indeed, the user on machine B is not the same as on machine A, but Eigen is installed on both.

The root cause is that CMake generates the tesseract_*-targets.cmake, and they contain the dependency list. Without this fix, they look like:

set_target_properties(tesseract::tesseract_environment_core PROPERTIES
  INTERFACE_COMPILE_FEATURES "cxx_std_17"
  INTERFACE_COMPILE_OPTIONS "-mno-avx"
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include;/home/herve/ros/ws/install/include/eigen3"
  INTERFACE_LINK_LIBRARIES "tesseract::tesseract_common;tesseract::tesseract_collision_core;tesseract::tesseract_collision_bullet;tesseract::tesseract_collision_fcl;tesseract::tesseract_scene_graph;tesseract::tesseract_urdf;tesseract::tesseract_kinematics_kdl;tesseract::tesseract_kinematics_opw"
  INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "/home/herve/ros/ws/install/include/eigen3"
)

Note the two absolute paths to the Eigen libraries. With this fix:

set_target_properties(tesseract::tesseract_environment_core PROPERTIES
  INTERFACE_COMPILE_FEATURES "cxx_std_17"
  INTERFACE_COMPILE_OPTIONS "-mno-avx"
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "Eigen3::Eigen;tesseract::tesseract_common;tesseract::tesseract_collision_core;tesseract::tesseract_collision_bullet;tesseract::tesseract_collision_fcl;tesseract::tesseract_scene_graph;tesseract::tesseract_urdf;tesseract::tesseract_kinematics_kdl;tesseract::tesseract_kinematics_opw"
)

Closing thoughts:

• This is useful for ROS Buildfarm Release "Noetic and Foxy" #521
• The same should probably be done for the other libraries linked to in this fashion, but they are harder to wrangle out. Orocos is marked as having a bug, TinyXML2 doesn't provide a CMake integration file on Ubuntu18.04, Boost relies on components, and those were fixed in CMake 3.15 which is only available on 20.04.
• Finally, it's probably worth setting up a CMakeLists formatter and / or linter as they are getting quite large. We have internally set up cmake-format and it's been quite helpful in harmonizing all of them.

[gavanderhoorn]

I'm pretty sure @Levi-Armstrong did things like this because the target you're using now is not universally supported on all platforms which Tesseract wants to support.

[Levi-Armstrong]

Thank you for the PR. I have been meaning to go through and do this for Eigen and Boost for a while since we dropped support for 16.04.

The same should probably be done for the other libraries linked to in this fashion, but they are harder to wrangle out. Orocos is marked as having a bug, TinyXML2 doesn't provide a CMake integration file on Ubuntu18.04, Boost relies on components, and those were fixed in CMake 3.15 which is only available on 20.04.

This is the goal to get everything to using target over cmake variables.

I believe the Orocos marked as a having a bug may not be valid any more. I believe the issue was related to catkin not correctly handling interface targets which I believe have been fixed so its worth testing again.

What was the issues with Boost on 18.04?

Finally, it's probably worth setting up a CMakeLists formatter and / or linter as they are getting quite large. We have internally set up cmake-format and it's been quite helpful in harmonizing all of them.

I agree, I will take a look.

[codecov]

Codecov Report

Merging #570 (f519210) into master (51d95fe) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #570   +/-   ##
=======================================
  Coverage   91.29%   91.29%           
=======================================
  Files         160      160           
  Lines        9978     9978           
=======================================
  Hits         9109     9109           
  Misses        869      869           

[haudren]

What was the issues with Boost on 18.04?

Main issue is that different Tesseract libraries look for different Boost components. In CMake 3.10, doing:

find_package(Boost COMPONENTS thread)
find_pacakge(Boost COMPONENTS filesystem)

Correctly finds both boost::thread and boost::filesystem. However, if you do:

find_dependency(Boost COMPONENTS thread)
find_dependency(Boost COMPONENTS filesystem)

Only boost::thread is found, because find_dependency contains a shortcutting instruction in case the package (not the components...) has already been found.

This means that:

• You can't find_package(Boost COMPONENTS xxx) before any package that itself needs Boost
• You can't depend on two packages that both need different Boost packages

This behavior fixed in 3.15, now find_dependency always calls find_package.

[Levi-Armstrong]

@haudren Thank you for the information. I have updated both tesseract and tesseract_planning to leverage boost targets and update the cmake config to use find_package if the cmake version is less than 3.15 due to the issue that you outlined.

Also the issues with kdl has be resolved so that has also been updated.

Next I will work on TinyXml2 to leverage targets. Do you know if anything else that needs updating?