Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

traversaro
Copy link
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
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
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
Member

@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 ami-iit/bipedal-locomotion-framework that referenced this pull request Mar 26, 2021
Fixed InstallBasicPackageFiles to avoid the same problem of ami-iit/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.

None yet

2 participants