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.

Sign up for GitHub

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

Use target_link_libraries everywhere #52

Merged
merged 2 commits into from
Jul 1, 2024
Merged

Use target_link_libraries everywhere #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbe5f4e
Use target_link_libraries everywhere
ahcorde Jun 28, 2024
4cc1003
make linters happy
ahcorde Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions draco_point_cloud_transport/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@ find_package(std_msgs REQUIRED)
find_package(Draco REQUIRED)

set(dependencies
pluginlib
point_cloud_interfaces
point_cloud_transport
rclcpp
rcpputils
sensor_msgs
std_msgs
pluginlib::pluginlib
${point_cloud_interfaces_TARGETS}
point_cloud_transport::point_cloud_transport
rclcpp::rclcpp
rcpputils::rcpputils
${sensor_msgs_TARGETS}
${std_msgs_TARGETS}
)


include_directories(include ${DRACO_INCLUDE_DIR})
include_directories(include)

add_library(${PROJECT_NAME}
SHARED
Expand All @@ -36,9 +35,15 @@ add_library(${PROJECT_NAME}
src/manifest.cpp
)

target_link_libraries(${PROJECT_NAME} ${DRACO_LIBRARY})

ament_target_dependencies(${PROJECT_NAME} ${dependencies})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please give motivation for this change? Is ament_target_dependencies deprecated or no longer good practice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This allows us to mark libraries as PRIVATE, so we can export less to downstream consumers.

target_link_libraries(${PROJECT_NAME} PRIVATE
${DRACO_LIBRARY}
${dependencies}
)
target_include_directories(${PROJECT_NAME} PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>"
${DRACO_INCLUDE_DIR}
)

install(TARGETS ${PROJECT_NAME}
ARCHIVE DESTINATION lib
Expand Down
18 changes: 11 additions & 7 deletions plugin_template/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# TODO (YourNameHere): This is not a working CMake Project!
# TODO (YourNameHere): This is not a working CMake Project!
# Some assembly required for your plugin. (-:

cmake_minimum_required(VERSION 3.10.2)
Expand All @@ -15,13 +15,15 @@ find_package(rclcpp REQUIRED)
# TODO (YourNameHere): You might need more dependencies

set(dependencies
pluginlib
point_cloud_interfaces
point_cloud_transport
rclcpp
pluginlib::pluginlib
${point_cloud_interfaces_TARGETS}
point_cloud_transport::point_cloud_transport
rclcpp::rclcpp
)

include_directories(include)
target_include_directories(${PROJECT_NAME} PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")

add_library(${PROJECT_NAME}
SHARED
Expand All @@ -31,7 +33,9 @@ add_library(${PROJECT_NAME}
src/template_subscriber.cpp
)

ament_target_dependencies(${PROJECT_NAME} ${dependencies})
target_link_libraries(${PROJECT_NAME} PRIVATE
${dependencies}
)

install(TARGETS ${PROJECT_NAME}
ARCHIVE DESTINATION lib
Expand Down
22 changes: 12 additions & 10 deletions zlib_point_cloud_transport/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,12 @@ find_package(rclcpp REQUIRED)
find_package(ZLIB REQUIRED)

set(dependencies
pluginlib
point_cloud_interfaces
point_cloud_transport
rclcpp
pluginlib::pluginlib
${point_cloud_interfaces_TARGETS}
point_cloud_transport::point_cloud_transport
rclcpp::rclcpp
)


include_directories(include)

add_library(${PROJECT_NAME}
SHARED
src/zlib_publisher.cpp
Expand All @@ -29,9 +26,14 @@ add_library(${PROJECT_NAME}
src/manifest.cpp
)

target_link_libraries(${PROJECT_NAME} ZLIB::ZLIB)

ament_target_dependencies(${PROJECT_NAME} ${dependencies})
target_link_libraries(${PROJECT_NAME} PRIVATE
ZLIB::ZLIB
${dependencies}
)
target_include_directories(${PROJECT_NAME} PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>"
)

install(TARGETS ${PROJECT_NAME}
ARCHIVE DESTINATION lib
Expand Down
19 changes: 11 additions & 8 deletions zstd_point_cloud_transport/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,27 @@ find_package(point_cloud_transport REQUIRED)
find_package(rclcpp REQUIRED)

set(dependencies
pluginlib
point_cloud_interfaces
point_cloud_transport
rclcpp
pluginlib::pluginlib
${point_cloud_interfaces_TARGETS}
point_cloud_transport::point_cloud_transport
rclcpp::rclcpp
)

include_directories(include)

add_library(${PROJECT_NAME}
SHARED
src/zstd_publisher.cpp
src/zstd_subscriber.cpp
src/manifest.cpp
)

target_link_libraries(${PROJECT_NAME} zstd)
target_link_libraries(${PROJECT_NAME} PRIVATE
zstd
${dependencies}
)

ament_target_dependencies(${PROJECT_NAME} ${dependencies})
target_include_directories(${PROJECT_NAME} PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")

install(TARGETS ${PROJECT_NAME}
ARCHIVE DESTINATION lib
Expand Down