From ae9dd2431b647be4c1581be3b4dbf0c275a4710f Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sun, 8 Mar 2020 23:08:25 -0400 Subject: [PATCH 1/4] Use ament_export_link_flags() to export -rdynamic from tracetools Signed-off-by: Christophe Bedard --- tracetools/CMakeLists.txt | 8 ++++++++ tracetools_test/CMakeLists.txt | 14 -------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/tracetools/CMakeLists.txt b/tracetools/CMakeLists.txt index 1e9ffb8..741216f 100644 --- a/tracetools/CMakeLists.txt +++ b/tracetools/CMakeLists.txt @@ -112,6 +112,13 @@ install( ament_export_include_directories(include) if(TRACETOOLS_LTTNG_ENABLED) ament_export_libraries(${PROJECT_NAME} ${LTTNG_LIBRARIES}) + # Export -rdynamic for downtream packages to use when calling + # ament_target_dependencies() + # which is needed to resolve function addresses to symbols when + # using function pointers directly/without std::bind() + # (the flag should not be used on Windows, but TRACETOOLS_LTTNG_ENABLED + # should never be true on Windows anyway, so there is no need to check) + ament_export_link_flags("-rdynamic") else() ament_export_libraries(${PROJECT_NAME}) endif() @@ -127,6 +134,7 @@ if(BUILD_TESTING) if(TRACETOOLS_LTTNG_ENABLED) ament_add_gtest(test_utils test/test_utils.cpp) if(TARGET test_utils) + # Since we cannot use ament_target_dependencies(... tracetools) in the same file target_link_libraries(test_utils ${PROJECT_NAME} -rdynamic) endif() endif() diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index 24211d0..9bace43 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -31,13 +31,6 @@ if(BUILD_TESTING) find_package(std_srvs REQUIRED) find_package(tracetools REQUIRED) - # Avoid using -rdynamic on Windows - if(NOT WIN32) - set(RDYNAMIC_FLAG "-rdynamic") - else() - set(RDYNAMIC_FLAG "") - endif() - add_executable(test_publisher src/test_publisher.cpp ) @@ -46,7 +39,6 @@ if(BUILD_TESTING) std_msgs tracetools ) - target_link_libraries(test_publisher "${RDYNAMIC_FLAG}") add_executable(test_intra src/test_intra.cpp ) @@ -55,7 +47,6 @@ if(BUILD_TESTING) std_msgs tracetools ) - target_link_libraries(test_intra "${RDYNAMIC_FLAG}") add_executable(test_ping src/test_ping.cpp ) @@ -64,7 +55,6 @@ if(BUILD_TESTING) std_msgs tracetools ) - target_link_libraries(test_ping "${RDYNAMIC_FLAG}") add_executable(test_pong src/test_pong.cpp ) @@ -73,7 +63,6 @@ if(BUILD_TESTING) std_msgs tracetools ) - target_link_libraries(test_pong "${RDYNAMIC_FLAG}") add_executable(test_timer src/test_timer.cpp ) @@ -81,7 +70,6 @@ if(BUILD_TESTING) rclcpp tracetools ) - target_link_libraries(test_timer "${RDYNAMIC_FLAG}") add_executable(test_service_ping src/test_service_ping.cpp ) @@ -90,7 +78,6 @@ if(BUILD_TESTING) std_srvs tracetools ) - target_link_libraries(test_service_ping "${RDYNAMIC_FLAG}") add_executable(test_service_pong src/test_service_pong.cpp ) @@ -99,7 +86,6 @@ if(BUILD_TESTING) std_srvs tracetools ) - target_link_libraries(test_service_pong "${RDYNAMIC_FLAG}") install(TARGETS test_intra From c086d8b84381a698d809a2fa1f394c857559edef Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sun, 8 Mar 2020 23:17:57 -0400 Subject: [PATCH 2/4] Move test_utils from tracetools to tracetools_test Signed-off-by: Christophe Bedard --- tracetools/CMakeLists.txt | 11 ----------- tracetools/package.xml | 1 - tracetools_test/CMakeLists.txt | 10 ++++++++-- tracetools_test/package.xml | 1 + {tracetools => tracetools_test}/test/test_utils.cpp | 0 5 files changed, 9 insertions(+), 14 deletions(-) rename {tracetools => tracetools_test}/test/test_utils.cpp (100%) diff --git a/tracetools/CMakeLists.txt b/tracetools/CMakeLists.txt index 741216f..34742da 100644 --- a/tracetools/CMakeLists.txt +++ b/tracetools/CMakeLists.txt @@ -125,19 +125,8 @@ endif() if(BUILD_TESTING) set(ament_cmake_cppcheck_ADDITIONAL_INCLUDE_DIRS ${LTTNG_INCLUDE_DIRS}) - - find_package(ament_cmake_gtest REQUIRED) find_package(ament_lint_auto REQUIRED) ament_lint_auto_find_test_dependencies() - - # Only build tracetools utils tests if LTTng is enabled and found - if(TRACETOOLS_LTTNG_ENABLED) - ament_add_gtest(test_utils test/test_utils.cpp) - if(TARGET test_utils) - # Since we cannot use ament_target_dependencies(... tracetools) in the same file - target_link_libraries(test_utils ${PROJECT_NAME} -rdynamic) - endif() - endif() endif() ament_package() diff --git a/tracetools/package.xml b/tracetools/package.xml index 35e0276..201e985 100644 --- a/tracetools/package.xml +++ b/tracetools/package.xml @@ -13,7 +13,6 @@ ament_cmake_ros pkg-config - ament_cmake_gtest ament_lint_auto ament_lint_common diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index 9bace43..14141fa 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -103,8 +103,15 @@ if(BUILD_TESTING) # Only build tracing tests if LTTng is enabled and found if(TRACING_ENABLED) - find_package(ament_cmake_pytest REQUIRED) + find_package(ament_cmake_gtest REQUIRED) + ament_add_gtest(test_utils test/test_utils.cpp) + if(TARGET test_utils) + ament_target_dependencies(test_utils + tracetools + ) + endif() + find_package(ament_cmake_pytest REQUIRED) # Run each test in its own pytest invocation set(_tracetools_test_pytest_tests test/test_intra.py @@ -114,7 +121,6 @@ if(BUILD_TESTING) test/test_subscription.py test/test_timer.py ) - foreach(_test_path ${_tracetools_test_pytest_tests}) get_filename_component(_test_name ${_test_path} NAME_WE) ament_add_pytest_test(${_test_name} ${_test_path} diff --git a/tracetools_test/package.xml b/tracetools_test/package.xml index 62142d9..f8cc79e 100644 --- a/tracetools_test/package.xml +++ b/tracetools_test/package.xml @@ -20,6 +20,7 @@ std_msgs std_srvs + ament_cmake_gtest ament_cmake_mypy ament_cmake_pytest ament_lint_auto diff --git a/tracetools/test/test_utils.cpp b/tracetools_test/test/test_utils.cpp similarity index 100% rename from tracetools/test/test_utils.cpp rename to tracetools_test/test/test_utils.cpp From 8bf8255b02d708d871c32ab8662bf08386dec8f6 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sun, 8 Mar 2020 23:21:29 -0400 Subject: [PATCH 3/4] Add TRACETOOLS_NO_RDYNAMIC option to not export -rdynamic Signed-off-by: Christophe Bedard --- tracetools/CMakeLists.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tracetools/CMakeLists.txt b/tracetools/CMakeLists.txt index 34742da..3de8d4d 100644 --- a/tracetools/CMakeLists.txt +++ b/tracetools/CMakeLists.txt @@ -20,6 +20,7 @@ else() set(DISABLED_DEFAULT OFF) endif() option(TRACETOOLS_DISABLED "Explicitly disable support for tracing" ${DISABLED_DEFAULT}) +option(TRACETOOLS_NO_RDYNAMIC "Disable export of -rdynamic link flag" OFF) if(NOT TRACETOOLS_DISABLED) # Set TRACETOOLS_LTTNG_ENABLED if we can find lttng-ust @@ -118,7 +119,9 @@ if(TRACETOOLS_LTTNG_ENABLED) # using function pointers directly/without std::bind() # (the flag should not be used on Windows, but TRACETOOLS_LTTNG_ENABLED # should never be true on Windows anyway, so there is no need to check) - ament_export_link_flags("-rdynamic") + if(NOT TRACETOOLS_NO_RDYNAMIC) + ament_export_link_flags("-rdynamic") + endif() else() ament_export_libraries(${PROJECT_NAME}) endif() From cb149f8bc015838710b44ba913859f503598376c Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sun, 8 Mar 2020 23:36:58 -0400 Subject: [PATCH 4/4] Only target tracetools transitively for tracetools_test tests Signed-off-by: Christophe Bedard --- tracetools_test/CMakeLists.txt | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index 14141fa..df8c4a9 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -26,10 +26,10 @@ endif() # Tests if(BUILD_TESTING) + # tracetools is exported by rclcpp find_package(rclcpp REQUIRED) find_package(std_msgs REQUIRED) find_package(std_srvs REQUIRED) - find_package(tracetools REQUIRED) add_executable(test_publisher src/test_publisher.cpp @@ -37,7 +37,6 @@ if(BUILD_TESTING) ament_target_dependencies(test_publisher rclcpp std_msgs - tracetools ) add_executable(test_intra src/test_intra.cpp @@ -45,7 +44,6 @@ if(BUILD_TESTING) ament_target_dependencies(test_intra rclcpp std_msgs - tracetools ) add_executable(test_ping src/test_ping.cpp @@ -53,7 +51,6 @@ if(BUILD_TESTING) ament_target_dependencies(test_ping rclcpp std_msgs - tracetools ) add_executable(test_pong src/test_pong.cpp @@ -61,14 +58,12 @@ if(BUILD_TESTING) ament_target_dependencies(test_pong rclcpp std_msgs - tracetools ) add_executable(test_timer src/test_timer.cpp ) ament_target_dependencies(test_timer rclcpp - tracetools ) add_executable(test_service_ping src/test_service_ping.cpp @@ -76,7 +71,6 @@ if(BUILD_TESTING) ament_target_dependencies(test_service_ping rclcpp std_srvs - tracetools ) add_executable(test_service_pong src/test_service_pong.cpp @@ -84,7 +78,6 @@ if(BUILD_TESTING) ament_target_dependencies(test_service_pong rclcpp std_srvs - tracetools ) install(TARGETS @@ -103,6 +96,7 @@ if(BUILD_TESTING) # Only build tracing tests if LTTng is enabled and found if(TRACING_ENABLED) + find_package(tracetools REQUIRED) find_package(ament_cmake_gtest REQUIRED) ament_add_gtest(test_utils test/test_utils.cpp) if(TARGET test_utils)