search

SteveMacenski / slam_toolbox

Slam Toolbox for lifelong mapping and localization in potentially massive maps with ROS
GNU Lesser General Public License v2.1
1.5k stars 497 forks source link

FindCSparse: search for headers in PATH_SUFFIXES also in non-standard location #699

Closed traversaro closed 2 months ago

traversaro commented 2 months ago

Basic Info

Info Please fill out this column
Ticket(s) this addresses None
Primary OS tested on Windows
Robotic platform tested on Not relevant, compilation fix.

Description of contribution in a few bullet points

The current version of find_package(CSparse) can fail with error:

2024-05-03T04:23:50.6918446Z   Could NOT find CSparse (missing: CSPARSE_INCLUDE_DIR)
2024-05-03T04:23:50.6918881Z Call Stack (most recent call first):
2024-05-03T04:23:50.6920132Z   C:/Users/runneradmin/micromamba/envs/testpr_env/conda-bld/ros-0_1714698402507/_build_env/Library/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
2024-05-03T04:23:50.6921325Z   CMake/FindCSparse.cmake:24 (find_package_handle_standard_args)
2024-05-03T04:23:50.6921779Z   CMakeLists.txt:79 (find_package)

when CSparse is not installed in one of the locations hardcoded in the PATHS argument of find_path. For example, it would also fail if you compiled suitesparse inside a colcon workspace instead of installing it from binary .deb packages.

CMake's find_path offers the PATH_SUFFIXES option (already available in CMake 3.5 https://cmake.org/cmake/help/v3.5/command/find_path.html, minimum required version for this package). In theory we could remove the other arguments passed to PATHS, but as they do not hurt and leaving them minimize the risk of regressions, I think we can leave them.

A similar fix done in g2o: https://github.com/RainerKuemmerle/g2o/commit/4f092a43e5214b979f9eca221f0f6f62781c2d5d .

Description of documentation updates required from your changes

Future work that may be required in bullet points