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

Jump to bottom

use ament_cmake_export_interfaces #22

Merged
merged 1 commit into from Mar 30, 2017
Merged

use ament_cmake_export_interfaces #22

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion ament_index_cpp/CMakeLists.txt
View file
Open in desktop
Expand Up @@ -14,9 +14,12 @@ add_library(${PROJECT_NAME} SHARED
src/get_search_paths.cpp
src/has_resource.cpp)
target_compile_definitions(${PROJECT_NAME} PRIVATE "AMENT_INDEX_CPP_BUILDING_DLL")
target_include_directories(${PROJECT_NAME} PUBLIC include)
target_include_directories(${PROJECT_NAME} PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include>")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary? Does $<INSTALL_INTERFACE:include> even exist at build time of this target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The BUILD_INTERFACE is being used to build the library. The INSTALL_INTERFACE is being used when downstream packages try to link against this target. See https://cmake.org/cmake/help/v3.5/command/target_include_directories.html


ament_export_include_directories(include)
ament_export_interfaces(export_${PROJECT_NAME})
Copy link
Contributor

@wjwwood wjwwood Mar 30, 2017 •
edited

Choose a reason for hiding this comment

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

Even after reading the documentation for ament_export_interfaces() I can't tell, is this an input variable or an output variable? Is export_${PROJECT_NAME} set before this is called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The argument has to match an EXPORT of an install(..) call. See comment below.

ament_export_libraries(${PROJECT_NAME})

if(BUILD_TESTING)
Expand All @@ -38,6 +41,7 @@ install(DIRECTORY include/
DESTINATION include)

install(TARGETS ${PROJECT_NAME}
EXPORT export_${PROJECT_NAME}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary, given the code in the ament_export_interfaces code?

https://github.com/ament/ament_cmake/blob/master/ament_cmake_export_interfaces/cmake/ament_cmake_export_interfaces_package_hook.cmake#L28-L33

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This install rule is being "identified" by the name export_${PROJECT_NAME}. ament_export_interfaces(<same_identifier>) creates the necessary part for the CMake config file that the installed targets can be imported in downstream packages.

ARCHIVE DESTINATION lib
LIBRARY DESTINATION lib
RUNTIME DESTINATION bin)