Skip to content

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

Merged
S-Dafarra merged 1 commit into
masterfrom
traversaro-patch-1
Mar 3, 2021
Merged

Do not use find_dependency in matioCppConfig.cmake if OVERRIDE_MODULE_PATH is used#41
S-Dafarra merged 1 commit into
masterfrom
traversaro-patch-1

Conversation

@traversaro
Copy link
Copy Markdown
Collaborator

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
Copy link
Copy Markdown
Collaborator Author

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 conda-forge/libmatio-feedstock#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
Copy link
Copy Markdown
Collaborator Author

traversaro commented Feb 28, 2021

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.

Copy link
Copy Markdown
Collaborator

@S-Dafarra S-Dafarra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah dammit, thanks for noticing.

@S-Dafarra S-Dafarra merged commit 0f1cc62 into master Mar 3, 2021
@S-Dafarra S-Dafarra deleted the traversaro-patch-1 branch March 3, 2021 17:14
S-Dafarra added a commit to S-Dafarra/bipedal-locomotion-framework that referenced this pull request Mar 26, 2021
GiulioRomualdi added a commit to gbionics/bipedal-locomotion-framework that referenced this pull request Mar 26, 2021
Fixed InstallBasicPackageFiles to avoid the same problem of gbionics/matio-cpp#41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants