Transition from ament_target_dependencies to target_link_libraries#182
Transition from ament_target_dependencies to target_link_libraries#182fbe555 wants to merge 2 commits intoICube-Robotics:jazzyfrom
Conversation
fbe555
left a comment
fbe555
left a comment
There was a problem hiding this comment.
I seem to have had something conveniently left over in my install dir, and not made a clean build. I cannot build this after cleaning. I will look into it tomorrow, and leave another comment when I have fixed it. Apologies for the premature PR.
fbe555
force-pushed
the
felbe/ament-target-dependencies-deprecation
branch
from
beafaaf to
a8679c4
Compare
|
I have now pushed a commit with a fix, and the PR now builds on my system for Rolling. I had forgotten the project name subfolder in the install interface target include dir in the ethercat_interface CMakeLists.txt: @@ -35,7 +30,7 @@ add_library(
target_include_directories(${PROJECT_NAME} PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
- $<INSTALL_INTERFACE:include>
+ $<INSTALL_INTERFACE:include/${PROJECT_NAME}>
${ETHERLAB_DIR}/include
)The PR should now be ready for running the repo test pipeline. |
|
Hi, |
Since kilted, the use of ament_target_dependencies has been deprecated. This commit replaces all uses of the macro with target_link_libraries as suggested in the deprecation warning, and does some additional steps in for the ethercat_interface package to ensure exports are done in the "modern" target-based fashion.
When using the yaml_cpp_vendor package, the correct way to link agains the yaml libraries is to first use: find_package(yaml_cpp_vendor REQUIRED) find_package(PkgConfig REQUIRED) pkg_check_modules(YAML_CPP REQUIRED yaml-cpp) to find the concrete yaml vendor package, and then use the YAML_CPP_* CMake variables to get the link targets, include libraries, and cmake options. This commit updates the relevant cmake files to comply with this pattern.
fbe555
force-pushed
the
felbe/ament-target-dependencies-deprecation
branch
from
a8679c4 to
2e67f5a
Compare
|
I have now rebased the branch. I have also added a new commit, addressing the way the yaml library from yaml_cpp_vendor is linked, as this directly overlaps with this feature in that the target_link_libraries needs updating. I hope you don't mind the slight expansion of the PR. If so let me know and I'll try to split it. |
|
I'm going on christmas holiday starting today, but I'll keep an eye on this in case the PR fails the pipeline or review, but response times might be slightly longer than usual. |
|
ping |
|
Bumping this thread :) |
2 participants
While using this repo with ROS Rolling Ridley, I have removed all uses of ament_target_dependencies, since it is now deprecated. I haven't tested this on anything but Rolling, and I'm no CMake wizard, but I thought I would create a PR to see if this might be useful. Let me know if there are any issues, and I'll be glad to try and address them as best I can.