search

ami-iit / matio-cpp

A C++ wrapper of the matio library, with memory ownership handling, to read and write .mat files.
https://ami-iit.github.io/matio-cpp/
BSD 2-Clause "Simplified" License
59 stars 9 forks source link

Do not use find_dependency in matioCppConfig.cmake if OVERRIDE_MODULE_PATH is used #41

Closed traversaro closed 3 years ago

traversaro commented 3 years ago

Before this PR, the matioCppConfig.cmake file contained this lines:

include(CMakeFindDependencyMacro)
set(CMAKE_MODULE_PATH_BK ${CMAKE_MODULE_PATH})
set(CMAKE_MODULE_PATH ${PACKAGE_PREFIX_DIR}/share/matioCpp/cmake)
find_dependency(MATIO QUIET)
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_BK})

Unfortunately these lines do not work correctly if MATIO is not present in the system but matioCpp is present. In that case, the matioCppConfig.cmake gets executed till to the find_dependency(MATIO QUIET) call, and then the rest of the script is not executed as, find_dependency calls return() if the package is not found (see https://github.com/Kitware/CMake/blob/v3.19.6/Modules/CMakeFindDependencyMacro.cmake#L59). The net effect is that if MATIO is not found, the CMAKE_MODULE_PATH gets corrupted by any call to find_package(matioCpp) , even if it was just find_package(matioCpp QUIET).

This PR proposes to fix this problem by using find_package there instead of find_dependency . The main downside is that the error if MATIO is not found will be less clear, but this is in any case better then silently corruping the CMAKE_MODULE_PATH value.

traversaro commented 3 years ago

Background: I hit this problem because I was debugging a failure in building the conda binary package for bipedal-locomotion-framework in https://github.com/traversaro/robotology-superbuild/runs/1998698606?check_suite_focus=true , as I was getting the error: 

2021-02-28T16:52:51.3120575Z CMake Error at CMakeLists.txt:63 (include):
2021-02-28T16:52:51.3121761Z   include could not find load file:
2021-02-28T16:52:51.3122316Z 
2021-02-28T16:52:51.3123021Z     AddInstallRPATHSupport
2021-02-28T16:52:51.3123613Z 
2021-02-28T16:52:51.3124106Z 
2021-02-28T16:52:51.3124855Z CMake Error at CMakeLists.txt:64 (add_install_rpath_support):
2021-02-28T16:52:51.3125772Z   Unknown CMake command "add_install_rpath_support".

The actual root cause was that installing the matio-cpp conda package was not installing also the libmatio conda package (this should be solved by https://github.com/conda-forge/libmatio-feedstock/pull/34), but the fact that if matio was not found the CMAKE_MODULE_PATH was corrupted was actually caused by the issue that this PR should solve.

traversaro commented 3 years ago

macOS/Homebrew failures are unrelated, see https://github.com/traversaro/github-actions-brew-update-upgrade-check/actions/runs/608289319 . macOS/conda by the way works fine instead.