InstallBasicPackageFiles: Fix bug of OVERRIDE_MODULE_PATH that corrupt CMAKE_MODULE_PATH values set by blf transitive dependencies

[traversaro]

@davidegorbani in ami-iit/biomechanical-analysis-framework#26 found a subtle bug that caused:

find_package(BipedalLocomotionFramework REQUIRED)
find_package(BipedalLocomotionFramework REQUIRED)

to fail, only when called two times.

The problem was related to the OVERRIDE_MODULE_PATH was not interacting nicely with with find_package that also added paths to CMAKE_MODULE_PATH . Why?

Basically the problem is that in its current implementation, OVERRIDE_MODULE_PATH saves the value of CMAKE_MODULE_PATH before all the dependencies are found via find_package, and then restores its value at the end, adding in the CMake config files that is called by find_package(BipedalLocomotionFramework) something like:

include(CMakeFindDependencyMacro)
set(CMAKE_MODULE_PATH_BK_BipedalLocomotionFramework ${CMAKE_MODULE_PATH})
set(CMAKE_MODULE_PATH ${PACKAGE_PREFIX_DIR}/share/BipedalLocomotionFramework/cmake)
find_package(iDynTree 3.0.0)
find_package(Eigen3 3.2.92)
find_package(spdlog 1.5.0)
find_package(YARP 3.7.0 COMPONENTS companion profiler dev os idl_tools)
find_package(casadi)
find_package(cppad)
find_package(manif 0.0.4)
find_package(matioCpp)
find_package(LieGroupControllers 0.2.0)
find_package(OpenCV)
find_package(PCL)
find_package(realsense2)
find_package(tomlplusplus 3.0.1)
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_BK_BipedalLocomotionFramework})

The problem here is that find_package(YARP) transitively calls find_package(YCM), that appends some variables in the CMAKE_MODULE_PATH. However, this values are erased by the set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_BK_BipedalLocomotionFramework}) call.

The second time find_package(BipedalLocomotionFramework) is called, YCM_FOUND is set to TRUE, so find_package(YCM) is not executing all its logic, but the CMAKE_MODULE_PATH does not contain the CMAKE_MODULE_PATH values required by YARP, and hence the failure experienced in ami-iit/biomechanical-analysis-framework#26 .

This PR fixes this problem by just prepending the value specified in OVERRIDE_MODULE_PATH, and then removing the added items from the CMAKE_MODULE_PATH at the end of the find_package calls, in this way preserving any value that was added in the meanwhile.

To catch possible failures like this in the future, I modified cmake-package-check to always look for a package two times (https://github.com/ami-iit/cmake-package-check).

[S-Dafarra]

Nice catch!

[traversaro]

To catch possible failures like this in the future, I modified cmake-package-check to always look for a package two times (https://github.com/ami-iit/cmake-package-check).

This was added in CI in 49622d2 .

[traversaro]

The logic is not working correctly, I see:

CMake Warning at /usr/local/lib/cmake/BipedalLocomotionFramework/BipedalLocomotionFrameworkConfig.cmake:31 (find_package):
  By not providing "Findcppad.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "cppad", but
  CMake did not find one.

  Could not find a package configuration file provided by "cppad" with any of
  the following names:

    cppadConfig.cmake
    cppad-config.cmake

  Add the installation prefix of "cppad" to CMAKE_PREFIX_PATH or set
  "cppad_DIR" to a directory containing one of the above files.  If "cppad"
  provides a separate development package or SDK, be sure it has been
  installed.
Call Stack (most recent call first):

[S-Dafarra]

Does it make sense to set a flag and simply avoid that logic from the second run?

[traversaro]

Does it make sense to set a flag and simply avoid that logic from the second run?

I think you would still find corner cases in which the system would not behave as expected. For example, currently:

find_package(BipedalLocomotionFramework REQUIRED)
find_package(YCM REQUIRED)

results in an emtpy CMAKE_MODULE_PATH (and hence the repo will not be able to find YCM modules), while:

find_package(YCM REQUIRED)
find_package(BipedalLocomotionFramework REQUIRED)

will work as expected.

[traversaro]

The logic is not working correctly, I see:

CMake Warning at /usr/local/lib/cmake/BipedalLocomotionFramework/BipedalLocomotionFrameworkConfig.cmake:31 (find_package):
  By not providing "Findcppad.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "cppad", but
  CMake did not find one.

  Could not find a package configuration file provided by "cppad" with any of
  the following names:

    cppadConfig.cmake
    cppad-config.cmake

  Add the installation prefix of "cppad" to CMAKE_PREFIX_PATH or set
  "cppad_DIR" to a directory containing one of the above files.  If "cppad"
  provides a separate development package or SDK, be sure it has been
  installed.
Call Stack (most recent call first):

Problem solved, just a typo and the fact tthat foreach() does not support IN LISTS if you are passing a literal list (as opposed to a variable list).

[GiulioRomualdi]

We currently have this failure

cd build
  cmake --install . --config Release
  cmake-package-check BipedalLocomotionFramework
  shell: /usr/bin/bash -l {0}
  env:
    INPUT_RUN_POST: true
    CONDA: /usr/share/miniconda[3](https://github.com/ami-iit/bipedal-locomotion-framework/actions/runs/8480631605/job/23236690685?pr=827#step:15:3)
    CONDA_PKGS_DIR: /home/runner/conda_pkgs_dir
CMake Error at src/ContinuousDynamicalSystem/cmake_install.cmake:52 (file):
  file INSTALL cannot copy file
  "/home/runner/work/bipedal-locomotion-framework/bipedal-locomotion-framework/build/lib/libBipedalLocomotionFrameworkContinuousDynamicalSystem.so.0.18.100"
-- Installing: /usr/local/lib/libBipedalLocomotionFrameworkContinuousDynamicalSystem.so.0.18.100
  to
  "/usr/local/lib/libBipedalLocomotionFrameworkContinuousDynamicalSystem.so.0.18.100":
  Permission denied.
Call Stack (most recent call first):
  src/cmake_install.cmake:[4](https://github.com/ami-iit/bipedal-locomotion-framework/actions/runs/8480631605/job/23236690685?pr=827#step:15:4)7 (include)
  cmake_install.cmake:52 (include)

[traversaro]

We currently have this failure

cd build
  cmake --install . --config Release
  cmake-package-check BipedalLocomotionFramework
  shell: /usr/bin/bash -l {0}
  env:
    INPUT_RUN_POST: true
    CONDA: /usr/share/miniconda[3](https://github.com/ami-iit/bipedal-locomotion-framework/actions/runs/8480631605/job/23236690685?pr=827#step:15:3)
    CONDA_PKGS_DIR: /home/runner/conda_pkgs_dir
CMake Error at src/ContinuousDynamicalSystem/cmake_install.cmake:52 (file):
  file INSTALL cannot copy file
  "/home/runner/work/bipedal-locomotion-framework/bipedal-locomotion-framework/build/lib/libBipedalLocomotionFrameworkContinuousDynamicalSystem.so.0.18.100"
-- Installing: /usr/local/lib/libBipedalLocomotionFrameworkContinuousDynamicalSystem.so.0.18.100
  to
  "/usr/local/lib/libBipedalLocomotionFrameworkContinuousDynamicalSystem.so.0.18.100":
  Permission denied.
Call Stack (most recent call first):
  src/cmake_install.cmake:[4](https://github.com/ami-iit/bipedal-locomotion-framework/actions/runs/8480631605/job/23236690685?pr=827#step:15:4)7 (include)
  cmake_install.cmake:52 (include)

Fixed by 0abd88c (if it works I will rebase). This will not be necessary if conda-forge/cmake-feedstock#208 is merged.

[traversaro]

Homebrew failures are unrelated and addressed in #822 , so I guess they can be ignored. There is a failure in conda-forge CI, but it seems related to some kind of network issue, that anyone persists even by restarting the CI.

[GiulioRomualdi]

Accordingly to #827 (comment), the PR can be merged, @traversaro do you agree?

[GiulioRomualdi]

Rebased on master, let's see

[traversaro]

There is a failure in conda-forge CI, but it seems related to some kind of network issue, that anyone persists even by restarting the CI.

xref:

• Seeing timeouts with https://conda.anaconda.org/rapidsai-nightly conda/infrastructure#906
• General slowness observed on anaconda.org (web, API, file serving) conda/infrastructure#899

Note that the problems seems mostly on the robostack-staging, not on conda-forge .

[GiulioRomualdi]

Rebased on master let's seen now

[traversaro]

Cool, now there is a legit error. On Windows on conda the cmake-package-check BipedalLocomotionFramework command fails with:

2024-04-05T10:37:36.7993177Z -- Found Python3: C:/hostedtoolcache/windows/Python/3.12.2/x64/python3.exe (found version "3.12.2") found components: Interpreter
2024-04-05T10:37:36.8585636Z Traceback (most recent call last):
2024-04-05T10:37:36.8586720Z   File "C:\Miniconda3\envs\test\Library\share\ament_cmake_core\cmake\package_templates\templates_2_cmake.py", line 21, in <module>
2024-04-05T10:37:36.8588457Z     from ament_package.templates import get_environment_hook_template_path
2024-04-05T10:37:36.8589122Z ModuleNotFoundError: No module named 'ament_package'
2024-04-05T10:37:36.8627884Z CMake Error at C:/Miniconda3/envs/test/Library/share/ament_cmake_core/cmake/ament_cmake_package_templates-extras.cmake:41 (message):
2024-04-05T10:37:36.8629421Z   execute_process(C:/hostedtoolcache/windows/Python/3.12.2/x64/python3.exe
2024-04-05T10:37:36.8630907Z   C:/Miniconda3/envs/test/Library/share/ament_cmake_core/cmake/package_templates/templates_2_cmake.py
2024-04-05T10:37:36.8632414Z   C:/Users/runneradmin/AppData/Local/Temp/tmpm7jsskty/ament_cmake_package_templates/templates.cmake)
2024-04-05T10:37:36.8633336Z   returned error code 1
2024-04-05T10:37:36.8634000Z Call Stack (most recent call first):
2024-04-05T10:37:36.8634707Z   C:/Miniconda3/envs/test/Library/share/ament_cmake_core/cmake/ament_cmake_coreConfig.cmake:41 (include)
2024-04-05T10:37:36.8635873Z   C:/Miniconda3/envs/test/Library/share/rclcpp/cmake/ament_cmake_export_include_directories-extras.cmake:8 (find_package)
2024-04-05T10:37:36.8636942Z   C:/Miniconda3/envs/test/Library/share/rclcpp/cmake/rclcppConfig.cmake:41 (include)
2024-04-05T10:37:36.8637916Z   C:/Miniconda3/envs/test/Library/cmake/BipedalLocomotionFrameworkConfig.cmake:28 (find_package)
2024-04-05T10:37:36.8638628Z   CMakeLists.txt:5 (find_package)

Because we are probably call find_package(rclcpp) in the CMake's BLF config file, and those somehow call Python (cps can't come soon enough to deal with this). Anyhow, that is some kind of ros package bug, let me look into that.

[traversaro]

Ok, the problem is the usual one of CMake that prefers to find a Python in the system (i.e. C:/hostedtoolcache/windows/Python/3.12.2/x64/python3.exe) instead of the one in the conda environment. I think it should be sufficient to use a recent enough policy in CMake to fix that, let's do this in cmake-package-check and see if it fixes the problem.

[GiulioRomualdi]

CI passing! Merging! Thank you @traversaro

[traversaro]

xref: robotology/robometry#153