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

avoid absolute OGRE path in exported targets #558

Merged

Conversation

dirk-thomas
Copy link
Member

@dirk-thomas dirk-thomas commented Jun 4, 2020

Fixes one more parts of #556 (comment).

Both targets already link against rviz_ogre_vendor::OgreMain / rviz_ogre_vendor::OgreOverlay which provide the necessary include directory.

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas added the bug Something isn't working label Jun 4, 2020
@dirk-thomas dirk-thomas self-assigned this Jun 4, 2020
@dirk-thomas
Copy link
Member Author

CI (only FastRTPS) building up to rviz_default_plugins and testing the touched packages:

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

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, but is this cleanup or require for it to work?

Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Yeah, I'm a bit surprised by this contributing to the fix.

@dirk-thomas
Copy link
Member Author

dirk-thomas commented Jun 4, 2020

Yes, it is necessary. Otherwise the exported target will also contain that absolute path and the result is this error: see the second half of #556 (comment)

@dirk-thomas dirk-thomas merged commit cb86e92 into ros2 Jun 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/avoid-absolute-ogre-path-exported-target branch June 4, 2020 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants