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

Remove automoc completely. #545

Merged
merged 2 commits into from May 15, 2020
Merged

Remove automoc completely. #545

merged 2 commits into from May 15, 2020

Conversation

clalancette
Copy link
Contributor

Continuing on from #540, disable automoc in the rest of rviz. There are 2 main reasons to do this:

  1. Reduce the memory consumption again (though this is a lot less important than it was for rviz_default_plugins).
  2. Get rid of warning messages on macOS of the form:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libgmock_main.a(mocs_compilation.cpp.o) has no symbols

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette requested a review from wjwwood May 12, 2020 12:31
@clalancette
Copy link
Contributor Author

clalancette commented May 12, 2020

CI:

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

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM, but mind to explain the unusual include paths?

@clalancette
Copy link
Contributor Author

LGTM, but mind to explain the unusual include paths?

Yeah, I agree they are weird. There are two problems here:

  1. Some of the tests are reaching into the source directory to include "private" headers (that are obviously not on the include path).
  2. Some of the tests depend on "common" test headers. These are also not on the include path.

Instead of using relative includes for these, I guess I could instead do a target_include_directories for the tests in question. @hidmic Do you think that would be better?

@hidmic
Copy link
Contributor

hidmic commented May 15, 2020

Instead of using relative includes for these, I guess I could instead do a target_include_directories for the tests in question. @hidmic Do you think that would be better?

That'd be nice, but I don't feel strongly about it. I was just wondering.

@clalancette
Copy link
Contributor Author

That'd be nice, but I don't feel strongly about it. I was just wondering.

Honestly, there are actually a bunch of these throughout rviz, so I'd prefer to do another pass there and fix them all together. So for now I'll go ahead and merge this one. Thanks for the reviews.

@clalancette clalancette merged commit 50e1baa into ros2 May 15, 2020
@clalancette clalancette deleted the no-automoc branch May 15, 2020 18:32
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

3 participants