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

The uninstall target gets exported when FRAMEWORK_COMPILE_RosImplementation is ON causing configuration failures on WalkingControllers #606

Closed
S-Dafarra opened this issue Feb 16, 2023 · 11 comments

Comments

@S-Dafarra
Copy link
Member

We recently noticed that walking-controllers was failing to configure. The culprit is the UninstallTarget added in https://github.com/robotology/walking-controllers/blob/2872a321138d53c5cacc519808cee82fe8e75287/CMakeLists.txt#L89

By editing the CMakeLists using the following code taken from https://discourse.cmake.org/t/cmake-list-of-all-project-targets/1077/17

function (_get_all_cmake_targets out_var current_dir)
    get_property(targets DIRECTORY ${current_dir} PROPERTY BUILDSYSTEM_TARGETS)
    get_property(subdirs DIRECTORY ${current_dir} PROPERTY SUBDIRECTORIES)

    foreach(subdir ${subdirs})
        _get_all_cmake_targets(subdir_targets ${subdir})
        list(APPEND targets ${subdir_targets})
    endforeach()

    set(${out_var} ${targets} PARENT_SCOPE)
endfunction()

# Run at end of top-level CMakeLists
_get_all_cmake_targets(all_targets ${CMAKE_CURRENT_LIST_DIR})

I managed to find that a target named uninstall was added after

find_package(BipedalLocomotionFramework 0.9.0
  COMPONENTS VectorsCollection IK ParametersHandlerYarpImplementation
             ContinuousDynamicalSystem ManifConversions
             ParametersHandlerYarpImplementation REQUIRED)

Checking with @GiulioRomualdi, we noticed that this does not happen if we force FRAMEWORK_USE_rclcpp to OFF

@S-Dafarra S-Dafarra changed the title The uninstall target gets exported when FRAMEWORK_COMPILE_RosImplementation is ON causing compilation failures on WalkingControllers The uninstall target gets exported when FRAMEWORK_COMPILE_RosImplementation is ON causing configuration failures on WalkingControllers Feb 16, 2023
@S-Dafarra
Copy link
Member Author

Here a picture of the output
image

@GiulioRomualdi
Copy link
Member

maybe @traversaro has some hints

@traversaro
Copy link
Collaborator

traversaro commented Feb 16, 2023

maybe @traversaro has some hints

In general it is quite common that the uninstall may be defined multiple times (for example when using FetchContent), hence the reason why it is only defined if not existing uninstall target exists. So, for fixing the issue I think we just need to import the latest AddUninstallTarget from the latest YCM that include the fix robotology/ycm-cmake-modules#237 (see for example robotology/bayes-filters-lib#101), or just switch walking-controllers to just depend on YCM.

Having said that, it is indeed quite strange that find_package(rclcpp) defines uninstall, let me look into this.

@traversaro
Copy link
Collaborator

Having said that, it is indeed quite strange that find_package(rclcpp) defines uninstall, let me look into this.

It seems that find_package(rclcpp) calls find_package(ament_cmake_core), and then ament_cmake_coreConfig.cmake includes the ament_cmake_uninstall_target-extras.cmake file that define the uninstall target. Note that the uninstall target seems to be defined only if the option AMENT_CMAKE_UNINSTALL_TARGET is set to ON, so a quick workaround is to set AMENT_CMAKE_UNINSTALL_TARGET to OFF, see https://github.com/ament/ament_cmake/blob/1.5.3/ament_cmake_core/ament_cmake_uninstall_target-extras.cmake#L17 .

Anyhow it seems to me that a find_package(..) calls that silently defines a uninstall target is probably not intended, I will open an issue upstream for that.

@GiulioRomualdi
Copy link
Member

Thank you @traversaro!

@traversaro
Copy link
Collaborator

I opened a bug upstream in ament/ament_cmake#427 .

@traversaro
Copy link
Collaborator

I did not understood this affected downstream libraries compilation, sorry for misreading it.

@traversaro
Copy link
Collaborator

At least for the walking-controllers case, the issue can also be solved by robotology/walking-controllers#141 .

@traversaro
Copy link
Collaborator

Ad discussed in chat with Giulio, a possible workaround for this is to add:

set(AMENT_CMAKE_UNINSTALL_TARGET OFF CACHE BOOL "" FORCE)

before find_package(rclcpp). However, if we are able to solve this in downstream projects (for example, via robotology/walking-controllers#141) we can then just wait for the proper fix landing in ament_cmake_core .

@GiulioRomualdi
Copy link
Member

What if another package links blf and rclcpp and it relies on the uninstall target provided by ament? (It is not our problem for the time being )

@traversaro
Copy link
Collaborator

traversaro commented Mar 7, 2023

What if another package links blf and rclcpp and it relies on the uninstall target provided by ament? (It is not our problem for the time being )

Yes, this is the good point. For this reason fixing the issue downstream (like in robotology/walking-controllers#141) or upstream (like in ament/ament_cmake#427) is probably the way to go. At this point probably we can close this issue, as it is not anymore blf related.

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

No branches or pull requests

3 participants