search

ros2 / rcl_logging

Logging implementations for ROS 2.
Apache License 2.0
18 stars 34 forks source link

macOS build fix #76

Closed nkalupahana closed 3 years ago

nkalupahana commented 3 years ago

The newest version of log4cxx on macOS is now built with C++17 on homebrew (the expected way to install the package, based on the ROS2 installation guide). C++17 adds support for shared_mutex, so log4cxx defaults to using shared_mutex in std instead of boost. C++14, however, doesn't have shared_mutex, so building rcl_logging_log4cxx on macOS with the latest packages now fails because it uses C++14. The easiest way to fix this is to switch this package to build on C++17, which is what I've done here.

If you all have any other solutions to this issue feel free to propose them, but this is the simplest one I could come up with, and is imperative for all macOS ROS2 users building from source.

clalancette commented 3 years ago

One other thing we could do here is to remove the log4cxx backend completely, as we never use it. But as long as this passes CI, this is a fine short-term solution for me. Let's see what CI has to say about it:

nkalupahana commented 3 years ago

Is the Linux build an issue, or is this good to go? If the log4cxx backend really is never used, I can also just remove it, that's arguably easier :)

clalancette commented 3 years ago

Those Linux tests flake out every once in a while (see https://ci.ros2.org/view/nightly/job/nightly_linux_release/1919/ for a recent example). It's highly doubtful it is the result of this PR. I'm going to go ahead and merge this, thanks for the contribution!