From 7d3eac2a0bfa3a5c1f46555d140fdbd309d860c7 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Nov 2023 20:38:40 +0000 Subject: [PATCH 1/2] Switch to target_link_libraries for linking. This allows us to make more of our dependencies PRIVATE. Signed-off-by: Chris Lalancette --- rmw_fastrtps_cpp/CMakeLists.txt | 61 +++++++++---------- rmw_fastrtps_cpp/package.xml | 2 + rmw_fastrtps_dynamic_cpp/CMakeLists.txt | 61 ++++++++----------- .../src/rmw_dynamic_message_type_support.cpp | 1 - rmw_fastrtps_shared_cpp/CMakeLists.txt | 38 ++++-------- rmw_fastrtps_shared_cpp/package.xml | 2 - rmw_fastrtps_shared_cpp/test/CMakeLists.txt | 37 ++++++----- 7 files changed, 92 insertions(+), 110 deletions(-) diff --git a/rmw_fastrtps_cpp/CMakeLists.txt b/rmw_fastrtps_cpp/CMakeLists.txt index 47520135a..0d2a5569d 100644 --- a/rmw_fastrtps_cpp/CMakeLists.txt +++ b/rmw_fastrtps_cpp/CMakeLists.txt @@ -45,6 +45,7 @@ find_package(FastRTPS 2.10 REQUIRED MODULE) find_package(rmw REQUIRED) find_package(rosidl_dynamic_typesupport REQUIRED) +find_package(rosidl_dynamic_typesupport_fastrtps REQUIRED) find_package(rosidl_runtime_c REQUIRED) find_package(rosidl_typesupport_fastrtps_c REQUIRED) find_package(rosidl_typesupport_fastrtps_cpp REQUIRED) @@ -97,24 +98,22 @@ add_library(rmw_fastrtps_cpp src/type_support_common.cpp src/rmw_get_endpoint_network_flow.cpp ) -target_link_libraries(rmw_fastrtps_cpp - fastcdr fastrtps) - -# specific order: dependents before dependencies -ament_target_dependencies(rmw_fastrtps_cpp - "rcpputils" - "rcutils" - "rosidl_typesupport_fastrtps_c" - "rosidl_typesupport_fastrtps_cpp" - "rmw_dds_common" - "rmw_fastrtps_shared_cpp" - "rmw" - "rosidl_runtime_c" - "tracetools" +target_link_libraries(rmw_fastrtps_cpp PUBLIC + fastcdr + fastrtps + rmw::rmw + rmw_fastrtps_shared_cpp::rmw_fastrtps_shared_cpp + rosidl_runtime_c::rosidl_runtime_c + rosidl_typesupport_fastrtps_cpp::rosidl_typesupport_fastrtps_cpp ) - -target_link_libraries(rmw_fastrtps_cpp - ${rmw_dds_common_LIBRARIES__rosidl_typesupport_fastrtps_cpp} +target_link_libraries(rmw_fastrtps_cpp PRIVATE + rcpputils::rcpputils + rcutils::rcutils + rmw_dds_common::rmw_dds_common_library + rosidl_dynamic_typesupport::rosidl_dynamic_typesupport + rosidl_dynamic_typesupport_fastrtps::rosidl_dynamic_typesupport_fastrtps + rosidl_typesupport_fastrtps_c::rosidl_typesupport_fastrtps_c + tracetools::tracetools ) configure_rmw_library(rmw_fastrtps_cpp) @@ -122,21 +121,13 @@ configure_rmw_library(rmw_fastrtps_cpp) # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} -PRIVATE "RMW_FASTRTPS_CPP_BUILDING_LIBRARY") + PRIVATE "RMW_FASTRTPS_CPP_BUILDING_LIBRARY") # Export old-style CMake variables ament_export_include_directories("include/${PROJECT_NAME}") ament_export_libraries(rmw_fastrtps_cpp) -# specific order: dependents before dependencies -ament_export_dependencies(rcutils) -ament_export_dependencies(rmw) -ament_export_dependencies(rmw_dds_common) -ament_export_dependencies(rmw_fastrtps_shared_cpp) -ament_export_dependencies(rosidl_runtime_c) -ament_export_dependencies(rosidl_typesupport_fastrtps_c) -ament_export_dependencies(rosidl_typesupport_fastrtps_cpp) -ament_export_dependencies(tracetools) +ament_export_dependencies(fastcdr rmw rmw_fastrtps_shared_cpp rosidl_runtime_c rosidl_typesupport_fastrtps_cpp) register_rmw_implementation( "c:rosidl_typesupport_c:rosidl_typesupport_fastrtps_c:rosidl_typesupport_introspection_c" @@ -152,14 +143,20 @@ if(BUILD_TESTING) ament_add_gtest(test_get_native_entities test/test_get_native_entities.cpp) - ament_target_dependencies(test_get_native_entities - osrf_testing_tools_cpp rcutils rmw test_msgs + target_link_libraries(test_get_native_entities + osrf_testing_tools_cpp::memory_tools + rcutils::rcutils + rmw::rmw + rmw_fastrtps_cpp + ${test_msgs_TARGETS} ) - target_link_libraries(test_get_native_entities rmw_fastrtps_cpp) ament_add_gtest(test_logging test/test_logging.cpp) - ament_target_dependencies(test_logging rmw) - target_link_libraries(test_logging rmw_fastrtps_cpp) + target_link_libraries(test_logging + fastrtps + rmw::rmw + rmw_fastrtps_cpp + ) endif() ament_package( diff --git a/rmw_fastrtps_cpp/package.xml b/rmw_fastrtps_cpp/package.xml index 92acfb328..e78bbc33f 100644 --- a/rmw_fastrtps_cpp/package.xml +++ b/rmw_fastrtps_cpp/package.xml @@ -29,6 +29,7 @@ rmw_dds_common rmw_fastrtps_shared_cpp rosidl_dynamic_typesupport + rosidl_dynamic_typesupport_fastrtps rosidl_runtime_c rosidl_runtime_cpp rosidl_typesupport_fastrtps_c @@ -54,6 +55,7 @@ rmw rmw_fastrtps_shared_cpp rosidl_dynamic_typesupport + rosidl_dynamic_typesupport_fastrtps tracetools ament_cmake_gtest diff --git a/rmw_fastrtps_dynamic_cpp/CMakeLists.txt b/rmw_fastrtps_dynamic_cpp/CMakeLists.txt index b25802941..ad886b04b 100644 --- a/rmw_fastrtps_dynamic_cpp/CMakeLists.txt +++ b/rmw_fastrtps_dynamic_cpp/CMakeLists.txt @@ -100,25 +100,21 @@ add_library(rmw_fastrtps_dynamic_cpp src/type_support_registry.cpp src/rmw_get_endpoint_network_flow.cpp ) -target_link_libraries(rmw_fastrtps_dynamic_cpp - fastcdr fastrtps) - -# specific order: dependents before dependencies -ament_target_dependencies(rmw_fastrtps_dynamic_cpp - "rcpputils" - "rcutils" - "rosidl_typesupport_fastrtps_c" - "rosidl_typesupport_fastrtps_cpp" - "rosidl_typesupport_introspection_c" - "rosidl_typesupport_introspection_cpp" - "rmw_dds_common" - "rmw_fastrtps_shared_cpp" - "rmw" - "rosidl_runtime_c" +target_link_libraries(rmw_fastrtps_dynamic_cpp PUBLIC + fastcdr + fastrtps + rcpputils::rcpputils + rcutils::rcutils + rmw::rmw + rmw_fastrtps_shared_cpp::rmw_fastrtps_shared_cpp + rosidl_runtime_c::rosidl_runtime_c + rosidl_typesupport_fastrtps_c::rosidl_typesupport_fastrtps_c + rosidl_typesupport_fastrtps_cpp::rosidl_typesupport_fastrtps_cpp + rosidl_typesupport_introspection_c::rosidl_typesupport_introspection_c + rosidl_typesupport_introspection_cpp::rosidl_typesupport_introspection_cpp ) - -target_link_libraries(rmw_fastrtps_dynamic_cpp - ${rmw_dds_common_LIBRARIES__rosidl_typesupport_introspection_cpp} +target_link_libraries(rmw_fastrtps_dynamic_cpp PRIVATE + rmw_dds_common::rmw_dds_common_library ) configure_rmw_library(rmw_fastrtps_dynamic_cpp) @@ -126,23 +122,14 @@ configure_rmw_library(rmw_fastrtps_dynamic_cpp) # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} -PRIVATE "RMW_FASTRTPS_DYNAMIC_CPP_BUILDING_LIBRARY") + PRIVATE "RMW_FASTRTPS_DYNAMIC_CPP_BUILDING_LIBRARY") # Export old-style CMake variables ament_export_include_directories("include/${PROJECT_NAME}") ament_export_libraries(rmw_fastrtps_dynamic_cpp) # specific order: dependents before dependencies -ament_export_dependencies(rcpputils) -ament_export_dependencies(rcutils) -ament_export_dependencies(rmw) -ament_export_dependencies(rmw_dds_common) -ament_export_dependencies(rmw_fastrtps_shared_cpp) -ament_export_dependencies(rosidl_runtime_c) -ament_export_dependencies(rosidl_typesupport_fastrtps_c) -ament_export_dependencies(rosidl_typesupport_fastrtps_cpp) -ament_export_dependencies(rosidl_typesupport_introspection_c) -ament_export_dependencies(rosidl_typesupport_introspection_cpp) +ament_export_dependencies(fastcdr rcpputils rcutils rmw rmw_fastrtps_shared_cpp rosidl_runtime_c rosidl_typesupport_fastrtps_c rosidl_typesupport_fastrtps_cpp rosidl_typesupport_introspection_c rosidl_typesupport_introspection_cpp) register_rmw_implementation( "c:rosidl_typesupport_c:rosidl_typesupport_introspection_c" @@ -158,14 +145,20 @@ if(BUILD_TESTING) ament_add_gtest(test_get_native_entities test/test_get_native_entities.cpp) - ament_target_dependencies(test_get_native_entities - osrf_testing_tools_cpp rcutils rmw test_msgs + target_link_libraries(test_get_native_entities + osrf_testing_tools_cpp::memory_tools + rcutils::rcutils + rmw::rmw + rmw_fastrtps_dynamic_cpp + ${test_msgs_TARGETS} ) - target_link_libraries(test_get_native_entities rmw_fastrtps_dynamic_cpp) ament_add_gtest(test_logging test/test_logging.cpp) - ament_target_dependencies(test_logging rmw) - target_link_libraries(test_logging rmw_fastrtps_dynamic_cpp) + target_link_libraries(test_logging + fastrtps + rmw::rmw + rmw_fastrtps_dynamic_cpp + ) endif() ament_package( diff --git a/rmw_fastrtps_dynamic_cpp/src/rmw_dynamic_message_type_support.cpp b/rmw_fastrtps_dynamic_cpp/src/rmw_dynamic_message_type_support.cpp index 1f3992c50..9c8cca436 100644 --- a/rmw_fastrtps_dynamic_cpp/src/rmw_dynamic_message_type_support.cpp +++ b/rmw_fastrtps_dynamic_cpp/src/rmw_dynamic_message_type_support.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include "rmw/allocators.h" #include "rmw/convert_rcutils_ret_to_rmw_ret.h" diff --git a/rmw_fastrtps_shared_cpp/CMakeLists.txt b/rmw_fastrtps_shared_cpp/CMakeLists.txt index 88b1fb533..606a0a415 100644 --- a/rmw_fastrtps_shared_cpp/CMakeLists.txt +++ b/rmw_fastrtps_shared_cpp/CMakeLists.txt @@ -36,7 +36,6 @@ endif() find_package(ament_cmake_ros REQUIRED) find_package(rosidl_dynamic_typesupport REQUIRED) -find_package(rosidl_dynamic_typesupport_fastrtps REQUIRED) find_package(rcpputils REQUIRED) find_package(rcutils REQUIRED) @@ -104,27 +103,26 @@ target_include_directories(rmw_fastrtps_shared_cpp PUBLIC "$" "$") -target_link_libraries(rmw_fastrtps_shared_cpp - fastcdr fastrtps +target_link_libraries(rmw_fastrtps_shared_cpp PUBLIC + fastcdr + fastrtps + rcpputils::rcpputils + rcutils::rcutils + rmw::rmw + rmw_dds_common::rmw_dds_common_library rosidl_dynamic_typesupport::rosidl_dynamic_typesupport - rosidl_dynamic_typesupport_fastrtps::rosidl_dynamic_typesupport_fastrtps ) -# specific order: dependents before dependencies -ament_target_dependencies(rmw_fastrtps_shared_cpp - "rcpputils" - "rcutils" - "rmw" - "rmw_dds_common" - "rosidl_typesupport_introspection_c" - "rosidl_typesupport_introspection_cpp" - "tracetools" +target_link_libraries(rmw_fastrtps_shared_cpp PRIVATE + rosidl_typesupport_introspection_c::rosidl_typesupport_introspection_c + rosidl_typesupport_introspection_cpp::rosidl_typesupport_introspection_cpp + tracetools::tracetools ) # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} -PRIVATE "RMW_FASTRTPS_SHARED_CPP_BUILDING_LIBRARY") + PRIVATE "RMW_FASTRTPS_SHARED_CPP_BUILDING_LIBRARY") # Export old-style CMake variables ament_export_include_directories("include/${PROJECT_NAME}") @@ -133,17 +131,7 @@ ament_export_libraries(rmw_fastrtps_shared_cpp) # Export modern CMake targets ament_export_targets(rmw_fastrtps_shared_cpp) -# specific order: dependents before dependencies -ament_export_dependencies(rcpputils) -ament_export_dependencies(rcutils) -ament_export_dependencies(rmw) -ament_export_dependencies(rmw_dds_common) -ament_export_dependencies(rosidl_typesupport_introspection_c) -ament_export_dependencies(rosidl_typesupport_introspection_cpp) -ament_export_dependencies(tracetools) - -ament_export_dependencies(rosidl_dynamic_typesupport_fastrtps) -ament_export_dependencies(rosidl_dynamic_typesupport) +ament_export_dependencies(fastcdr rcpputils rcutils rmw rmw_dds_common rosidl_dynamic_typesupport) if(BUILD_TESTING) find_package(ament_lint_auto REQUIRED) diff --git a/rmw_fastrtps_shared_cpp/package.xml b/rmw_fastrtps_shared_cpp/package.xml index 9a32e2d30..572b5fe36 100644 --- a/rmw_fastrtps_shared_cpp/package.xml +++ b/rmw_fastrtps_shared_cpp/package.xml @@ -20,7 +20,6 @@ ament_cmake - rosidl_dynamic_typesupport_fastrtps rosidl_dynamic_typesupport fastcdr fastrtps @@ -34,7 +33,6 @@ rosidl_typesupport_introspection_cpp tracetools - rosidl_dynamic_typesupport_fastrtps rosidl_dynamic_typesupport fastcdr fastrtps diff --git a/rmw_fastrtps_shared_cpp/test/CMakeLists.txt b/rmw_fastrtps_shared_cpp/test/CMakeLists.txt index 66478cdc6..d4643de1b 100644 --- a/rmw_fastrtps_shared_cpp/test/CMakeLists.txt +++ b/rmw_fastrtps_shared_cpp/test/CMakeLists.txt @@ -3,31 +3,40 @@ find_package(osrf_testing_tools_cpp REQUIRED) ament_add_gtest(test_dds_attributes_to_rmw_qos test_dds_attributes_to_rmw_qos.cpp) if(TARGET test_dds_attributes_to_rmw_qos) - ament_target_dependencies(test_dds_attributes_to_rmw_qos) - target_link_libraries(test_dds_attributes_to_rmw_qos ${PROJECT_NAME}) + target_link_libraries(test_dds_attributes_to_rmw_qos ${PROJECT_NAME}) endif() ament_add_gtest(test_dds_qos_to_rmw_qos test_dds_qos_to_rmw_qos.cpp) if(TARGET test_dds_qos_to_rmw_qos) - ament_target_dependencies(test_dds_qos_to_rmw_qos) - target_link_libraries(test_dds_qos_to_rmw_qos ${PROJECT_NAME}) + target_link_libraries(test_dds_qos_to_rmw_qos ${PROJECT_NAME}) endif() ament_add_gtest(test_rmw_qos_to_dds_attributes test_rmw_qos_to_dds_attributes.cpp) if(TARGET test_rmw_qos_to_dds_attributes) - target_link_libraries(test_rmw_qos_to_dds_attributes ${PROJECT_NAME}) + target_link_libraries(test_rmw_qos_to_dds_attributes + ${PROJECT_NAME} + rmw::rmw + rosidl_runtime_c::rosidl_runtime_c + ) endif() ament_add_gmock(test_security_logging test_security_logging.cpp) if(TARGET test_security_logging) - ament_target_dependencies(test_security_logging) - target_link_libraries(test_security_logging ${PROJECT_NAME}) + target_link_libraries(test_security_logging + ${PROJECT_NAME} + rcutils::rcutils + rmw::rmw + ) endif() ament_add_gtest(test_rmw_init_options test_rmw_init_options.cpp) if(TARGET test_rmw_init_options) - ament_target_dependencies(test_rmw_init_options osrf_testing_tools_cpp rcutils rmw) - target_link_libraries(test_rmw_init_options ${PROJECT_NAME}) + target_link_libraries(test_rmw_init_options + ${PROJECT_NAME} + osrf_testing_tools_cpp::memory_tools + rcutils::rcutils + rmw::rmw + ) endif() ament_add_gtest(test_guid_utils test_guid_utils.cpp) @@ -37,19 +46,15 @@ endif() ament_add_gtest(test_names test_names.cpp) if(TARGET test_names) - ament_target_dependencies(test_names rmw) - target_link_libraries(test_names ${PROJECT_NAME}) + target_link_libraries(test_names ${PROJECT_NAME} rmw::rmw) endif() ament_add_gtest(test_qos_profile_check_compatible test_qos_profile_check_compatible.cpp) if(TARGET test_qos_profile_check_compatible) - ament_target_dependencies(test_qos_profile_check_compatible rmw) - target_link_libraries(test_qos_profile_check_compatible ${PROJECT_NAME}) + target_link_libraries(test_qos_profile_check_compatible ${PROJECT_NAME} rmw::rmw) endif() ament_add_gtest(test_logging test_logging.cpp) if(TARGET test_logging) - ament_target_dependencies(test_logging - osrf_testing_tools_cpp rcutils rmw) - target_link_libraries(test_logging rmw_fastrtps_shared_cpp) + target_link_libraries(test_logging ${PROJECT_NAME} rmw::rmw) endif() From 0b034e3435f4637cbab875eb7e961b53c618e56d Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 27 Nov 2023 02:20:33 +0000 Subject: [PATCH 2/2] Fixes from review. Signed-off-by: Chris Lalancette --- rmw_fastrtps_dynamic_cpp/CMakeLists.txt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/rmw_fastrtps_dynamic_cpp/CMakeLists.txt b/rmw_fastrtps_dynamic_cpp/CMakeLists.txt index ad886b04b..f2331d677 100644 --- a/rmw_fastrtps_dynamic_cpp/CMakeLists.txt +++ b/rmw_fastrtps_dynamic_cpp/CMakeLists.txt @@ -128,8 +128,18 @@ target_compile_definitions(${PROJECT_NAME} ament_export_include_directories("include/${PROJECT_NAME}") ament_export_libraries(rmw_fastrtps_dynamic_cpp) -# specific order: dependents before dependencies -ament_export_dependencies(fastcdr rcpputils rcutils rmw rmw_fastrtps_shared_cpp rosidl_runtime_c rosidl_typesupport_fastrtps_c rosidl_typesupport_fastrtps_cpp rosidl_typesupport_introspection_c rosidl_typesupport_introspection_cpp) +ament_export_dependencies( + fastcdr + rcpputils + rcutils + rmw + rmw_fastrtps_shared_cpp + rosidl_runtime_c + rosidl_typesupport_fastrtps_c + rosidl_typesupport_fastrtps_cpp + rosidl_typesupport_introspection_c + rosidl_typesupport_introspection_cpp +) register_rmw_implementation( "c:rosidl_typesupport_c:rosidl_typesupport_introspection_c"