Install includes to include/${PROJECT_NAME} and export library target #28
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
|
CI (supplemental repos build:
|
|
@sloretz This fix has caused the header file to be installed in include/angles/angles/angles.h and causes packages including angles/angles.h to fail with file not found since it is installed in the wrong place |
|
|
||
| ament_export_include_directories(include) | ||
| install(DIRECTORY include/ DESTINATION include/angles) |
There was a problem hiding this comment.
should be
install(DIRECTORY include/ DESTINATION include)
There was a problem hiding this comment.
@sloretz This fix has caused the header file to be installed in include/angles/angles/angles.h and causes packages including angles/angles.h to fail with file not found since it is installed in the wrong place
This change is intentional, see ros2/ros2#1150 for why. This PR also removed the old-style CMake variable angles_INCLUDE_DIR. If a package isn't building, that's probably why. The correct fix is to link against the exported interface target
# If your target includes angles headers in it's public headers use this:
target_link_libraries(mylibrary PUBLIC angles::angles)
# If your target only includes angles headers in source files or non-installed headers, use this:
target_link_libraries(mylibrary PRIVATE angles::angles)|
@sloretz I actually think we need to do some additional work here. In particular, the |
| FILES_MATCHING PATTERN "*.h") | ||
| add_library(angles INTERFACE) | ||
| target_link_libraries(angles INTERFACE | ||
| "$<INSTALL_INTERFACE:install/angles>") |
There was a problem hiding this comment.
Whoops, this is broken! It should be target_include_directories
This installs includes to
include/${PROJECT_NAME}to mitigate include directory search order issues when overriding packages indesktop.It also replaces the old-style CMake variable export with a modern CMake target export. Dependencies using
ament_target_dependencies(MyTarget angles)will still work, but now they can use modern CMake targets instead.Part of ros2/ros2#1150
Part of ament/ament_cmake#365