Skip to content

Transition from ament_target_dependencies to target_link_libraries #182

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

Open
wants to merge 1 commit into
base: jazzy
Choose a base branch
from

Conversation

fbe555
Copy link

@fbe555 fbe555 commented Aug 5, 2025

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.

Copy link
Author

@fbe555 fbe555 left a comment

Choose a reason for hiding this comment

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

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.

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.
@fbe555 fbe555 force-pushed the felbe/ament-target-dependencies-deprecation branch from beafaaf to a8679c4 Compare August 6, 2025 07:13
@fbe555
Copy link
Author

fbe555 commented Aug 6, 2025

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.

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.

1 participant