From 63fc82d7249fd655c5313a83c82380a49f250b99 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 10 Jan 2019 16:53:41 -0800 Subject: [PATCH 1/3] Account for INTERFACE libraries when getting target include directories CMake does not allow getting the INCLUDE_DIRECTORIES property from INTERFACE libraries. Instead, first check if the property exists, if it does not then try to get the INTERFACE_INCLUDE_DIRECTORIES property. Note, if INTERFACE_INCLUDE_DIRECTORIES is not defined an empty list is returned, but we cannot assume the target is not an interface. This is why the implementation is conditional on INCLUDE_DIRECTORIES instead. Signed-off-by: Jacob Perron --- .../ament_cmake_cppcheck_lint_hook.cmake | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake b/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake index 788e51bb..315b71a9 100644 --- a/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake +++ b/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake @@ -33,18 +33,38 @@ if(_source_files) if(NOT CMAKE_VERSION VERSION_LESS "3.7.0") get_directory_property(_build_targets DIRECTORY ${CMAKE_SOURCE_DIR} BUILDSYSTEM_TARGETS) foreach(_target ${_build_targets}) - get_property(_include_dirs + # Check if INCLUDE_DIRECTORIES is defined first in case the target is an INTERFACE + get_property(_include_dirs_defined TARGET ${_target} PROPERTY INCLUDE_DIRECTORIES + DEFINED ) + if(_include_dirs_defined) + get_property(_include_dirs + TARGET ${_target} + PROPERTY INCLUDE_DIRECTORIES + ) + else() + # Target might be an interface + get_property(_include_dirs + TARGET ${_target} + PROPERTY INTERFACE_INCLUDE_DIRECTORIES + ) + endif() + # Only append include directories that are from the package being tested # This accomplishes two things: # 1. Reduces execution time (less include directories to search) # 2. cppcheck will not check for errors in external packages foreach(_include_dir ${_include_dirs}) - string(REGEX MATCH "^${CMAKE_SOURCE_DIR}.*" _is_match ${_include_dir}) - if(_is_match) + # TODO(jacobperron): Escape special regex characters in CMAKE_SOURCE_DIR + # Related CMake feature request: https://gitlab.kitware.com/cmake/cmake/issues/18409 + # Check if include directory is a subdirectory of the source directory + string(REGEX MATCH "^${CMAKE_SOURCE_DIR}/.*" _is_subdirectory ${_include_dir}) + # Check if include directory is part of a generator expression (e.g. $) + string(REGEX MATCH "^\\$<.*:${CMAKE_SOURCE_DIR}/.*>$" _is_genexp_subdirectory "${_include_dir}") + if(_is_subdirectory OR _is_genexp_subdirectory) list_append_unique(_all_include_dirs ${_include_dir}) endif() endforeach() From 434dadc04576263d1bb06d4f6823c48d33bd6bc3 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 11 Jan 2019 13:40:29 -0800 Subject: [PATCH 2/3] Use target type property as a condition on what include directories property to use Signed-off-by: Jacob Perron --- .../ament_cmake_cppcheck_lint_hook.cmake | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake b/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake index 315b71a9..d8a90ae3 100644 --- a/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake +++ b/ament_cmake_cppcheck/cmake/ament_cmake_cppcheck_lint_hook.cmake @@ -33,24 +33,12 @@ if(_source_files) if(NOT CMAKE_VERSION VERSION_LESS "3.7.0") get_directory_property(_build_targets DIRECTORY ${CMAKE_SOURCE_DIR} BUILDSYSTEM_TARGETS) foreach(_target ${_build_targets}) - # Check if INCLUDE_DIRECTORIES is defined first in case the target is an INTERFACE - get_property(_include_dirs_defined - TARGET ${_target} - PROPERTY INCLUDE_DIRECTORIES - DEFINED - ) - - if(_include_dirs_defined) - get_property(_include_dirs - TARGET ${_target} - PROPERTY INCLUDE_DIRECTORIES - ) + # Include directories property is different for INTERFACE libraries + get_target_property(_target_type ${_target} TYPE) + if(${_target_type} STREQUAL "INTERFACE_LIBRARY") + get_target_property(_include_dirs ${_target} INTERFACE_INCLUDE_DIRECTORIES) else() - # Target might be an interface - get_property(_include_dirs - TARGET ${_target} - PROPERTY INTERFACE_INCLUDE_DIRECTORIES - ) + get_target_property(_include_dirs ${_target} INCLUDE_DIRECTORIES) endif() # Only append include directories that are from the package being tested From bcca8201d0f350651f731302353106693af2c408 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 11 Jan 2019 20:50:20 -0800 Subject: [PATCH 3/3] Increase cppcheck test timeout to 120s Signed-off-by: Jacob Perron --- ament_cmake_cppcheck/cmake/ament_cppcheck.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/ament_cmake_cppcheck/cmake/ament_cppcheck.cmake b/ament_cmake_cppcheck/cmake/ament_cppcheck.cmake index 29dda86d..e3ea7b45 100644 --- a/ament_cmake_cppcheck/cmake/ament_cppcheck.cmake +++ b/ament_cmake_cppcheck/cmake/ament_cppcheck.cmake @@ -51,6 +51,7 @@ function(ament_cppcheck) ament_add_test( "${ARG_TESTNAME}" COMMAND ${cmd} + TIMEOUT 120 OUTPUT_FILE "${CMAKE_BINARY_DIR}/ament_cppcheck/${ARG_TESTNAME}.txt" RESULT_FILE "${result_file}" WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"