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.

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

Install headers to include/${PROJECT_NAME} #658

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Feb 5, 2022

Part of ros2/ros2#1150 - this installs resource_retriever's header files to a unique directory in a merged workspace.

This only installs headers from rosidl projects to unique directories. I did not yet tackle making the headers generated by these packages install to a unique directory.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz self-assigned this Feb 5, 2022
@sloretz
Copy link
Contributor Author

sloretz commented Feb 5, 2022

CI (build: all of them test: --packages-select rosidl_runtime_c rosidl_typesupport_introspection_cpp rosidl_typesupport_interface rosidl_runtime_cpp rosidl_typesupport_introspection_c)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@@ -41,7 +44,7 @@ if(BUILD_TESTING)

ament_add_gtest(test_bounded_vector test/test_bounded_vector.cpp)
if(TARGET test_bounded_vector)
target_include_directories(test_bounded_vector PUBLIC include)
target_link_libraries(test_bounded_vector ${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.

Not that I find this incorrect but caught my attention. The test did not link to the library before? Did it work?

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 library in this project is an INTERFACE library. It's a header only library that exports the include directory via ... INTERFACE "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>". It seemed fitting to get the include directory from that target just as targets in downstream packages would do when they depend on it. That may make a difference if in the future it also passes on compile options or definitions in addition to the include directories.

@sloretz sloretz merged commit 0c48c48 into master Feb 10, 2022
@delete-merged-branch delete-merged-branch bot deleted the sloretz__rosidl__include_projectname branch February 10, 2022 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants