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

Install headers to include/${PROJECT_NAME} #71

Merged
merged 3 commits into from
Mar 8, 2022
Merged

Install headers to include/${PROJECT_NAME} #71

merged 3 commits into from
Mar 8, 2022

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Feb 12, 2022

Part of ros2/ros2#1150 - this installs headers to a unique include directory to prevent include directory search order issues when overriding packages from a merged workspace.

@sloretz
Install headers to include/${PROJECT_NAME}
de92127 
Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
@sloretz sloretz self-assigned this Feb 12, 2022
@sloretz sloretz marked this pull request as draft February 16, 2022 01:02
@sloretz
Copy link
Contributor Author

sloretz commented Feb 16, 2022 •
edited
Loading

Converted to draft while I figure out if changes done to ros/urdfdom#167 also should be applied here. (ros/urdfdom#167 (comment))

@sloretz
Rename files to make this way easier
7f5a982 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
use CMAKE_INSTALL_INCLUDEDIR
f537521 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz marked this pull request as ready for review February 18, 2022 22:20
@sloretz sloretz mentioned this pull request Feb 18, 2022
Merged
@sloretz
Copy link
Contributor Author

sloretz commented Feb 18, 2022

CI (build: --packages-above-and-dependencies urdfdom_headers test: --packages-select-regex "urdf.*")

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

@sloretz
Copy link
Contributor Author

sloretz commented Feb 23, 2022

The compiler warnings in the aarch64 build are all in the nightly as well: https://ci.ros2.org/view/nightly/job/nightly_linux-aarch64_release/1852/ , so CI LGTM, just needs an approval.

clalancette
clalancette approved these changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This is certainly a much more traditional CMake setup. Looks good to me with green CI.

@sloretz
Copy link
Contributor Author

sloretz commented Mar 8, 2022

CI (build: --packages-above-and-dependencies urdfdom_headers test: --packages-select urdfdom_headers)

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

@sloretz sloretz merged commit 040de91 into master Mar 8, 2022
@sloretz sloretz deleted the sloretz__urdfdom_headers__include_projectname branch March 8, 2022 23:27
@traversaro traversaro mentioned this pull request Apr 2, 2022
Merged
5 tasks
@tdejager tdejager mentioned this pull request Apr 5, 2022
Closed
@scpeters scpeters mentioned this pull request May 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

Assignees

@sloretz sloretz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sloretz @clalancette