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

Add mixin to force clang and libcxx #16

Merged
merged 5 commits into from
Mar 13, 2019

Conversation

emersonknapp
Copy link
Contributor

@emersonknapp emersonknapp commented Mar 5, 2019

Edited up to date summary:

  • Adds a mixin that performs the build using clang6, with LLVM's libcxx instead of gnu libstdc++
  • Adding some CMake variables to force building some vendor packages

This mixin would be used on Clang CI build to enable Thread Safety Analysis, and generally validate that ROS2 works with libcxx

Related to:

Connects to ros2/ros2#664

@thomas-moulard
Copy link

Could you elaborate when this global flags is necessary? compared to e.g. using CXXFLAGS?

@emersonknapp
Copy link
Contributor Author

@thomas-moulard here is an example that uses both flags ros2/poco_vendor#24

@dirk-thomas dirk-thomas added the in progress Actively being worked on (Kanban column) label Mar 5, 2019
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp
Copy link
Contributor Author

I was able to "force libcxx" down the line by using full passthrough of CMAKE_CXX_FLAGS, as per @dirk-thomas suggestion. This seems to have worked for all downstream packages. As for FORCE_BUILD_ALL_VENDOR, there were only two existing vendor packages that didn't build by default, so I went in favor of specificity

@emersonknapp
Copy link
Contributor Author

With this mixin, plus a few other minor pull requests that are incoming, colcon build fully succeeds and I get the following colcon test results. I think that starting from here it would be a meaningful CI build.

Summary: 231 packages finished [10min 8s]
  1 package failed: sros2
  13 packages had stderr output: class_loader composition demo_nodes_cpp demo_nodes_cpp_native image_tools intra_process_demo logging_demo osrf_pycommon pendulum_control pluginlib qt_gui_cpp rosidl_parser rqt
  11 packages had test failures: class_loader composition demo_nodes_cpp demo_nodes_cpp_native image_tools intra_process_demo logging_demo pendulum_control pluginlib qt_gui_cpp rosidl_parser

@nuclearsandwich
Copy link

I think that starting from here it would be a meaningful CI build.

@emersonknapp are you planning to switch our existing linux-clang job to use libcxx or add an additional job?

libcxx.mixin Outdated Show resolved Hide resolved
libcxx.mixin Outdated Show resolved Hide resolved
libcxx.mixin Outdated Show resolved Hide resolved
@nuclearsandwich
Copy link

Full CI on a clang+libcxx on Linux run

  • Linux Clang+libcxx Build Status

@emersonknapp
Copy link
Contributor Author

I think that starting from here it would be a meaningful CI build.

@emersonknapp are you planning to switch our existing linux-clang job to use libcxx or add an additional job?

The way I see it, there is value in having CI builds for both:

  • Linux Clang with libstdc++
  • Linux Clang with libc++

libcxx.mixin Outdated Show resolved Hide resolved
libcxx.mixin Outdated Show resolved Hide resolved
libcxx.mixin Outdated Show resolved Hide resolved
@nuclearsandwich nuclearsandwich merged commit 9eb43d2 into colcon:master Mar 13, 2019
@nuclearsandwich nuclearsandwich removed the in progress Actively being worked on (Kanban column) label Mar 13, 2019
@emersonknapp emersonknapp deleted the libcxx branch March 13, 2019 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants