ros2 / rcl_logging

Logging implementations for ROS 2.
Apache License 2.0
20 stars 36 forks source link

Change C++ version to 17 in `rcl_logging_log4cxx` #79

Closed homalozoa closed 2 years ago

homalozoa commented 3 years ago

Signed-off-by: Homalozoa xuhaiwang@xiaomi.com

Fix building error with log4cxx 0.12.0. shared_mutex is only available from C++ 17 onwards, ref: std::shared_mutex

-- stderr: rcl_logging_log4cxx                                                                                                                                                                                         
In file included from /usr/include/log4cxx/log4cxx.h:45,
                 from /usr/include/log4cxx/logstring.h:28,
                 from /usr/include/log4cxx/spi/appenderattachable.h:26,
                 from /usr/include/log4cxx/helpers/appenderattachableimpl.h:27,
                 from /usr/include/log4cxx/logger.h:30,
                 from /home/homalozoa/local_git/ros2_fork/src/ros2/rcl_logging/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp:23:
/usr/include/log4cxx/boost-std-configuration.h:10:18: error: ‘shared_mutex’ in namespace ‘std’ does not name a type
   10 |     typedef std::shared_mutex shared_mutex;
      |                  ^~~~~~~~~~~~
/usr/include/log4cxx/boost-std-configuration.h:10:13: note: ‘std::shared_mutex’ is only available from C++17 onwards
   10 |     typedef std::shared_mutex shared_mutex;
      |             ^~~
In file included from /home/homalozoa/local_git/ros2_fork/src/ros2/rcl_logging/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp:23:
/usr/include/log4cxx/logger.h:1729:25: error: ‘shared_mutex’ does not name a type
 1729 |                 mutable shared_mutex mutex;
      |                         ^~~~~~~~~~~~
In file included from /usr/include/log4cxx/writerappender.h:26,
                 from /usr/include/log4cxx/fileappender.h:23,
                 from /home/homalozoa/local_git/ros2_fork/src/ros2/rcl_logging/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp:28:
/usr/include/log4cxx/appenderskeleton.h:77:34: error: ‘shared_mutex’ in namespace ‘log4cxx’ does not name a type
   77 |                 mutable log4cxx::shared_mutex mutex;
      |                                  ^~~~~~~~~~~~
make[2]: *** [CMakeFiles/rcl_logging_log4cxx.dir/build.make:76: CMakeFiles/rcl_logging_log4cxx.dir/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:85: CMakeFiles/rcl_logging_log4cxx.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
clalancette commented 3 years ago

I'm actually going to close this one, since we are going to remove the log4cxx backend in #78.

aprotyas commented 2 years ago

@clalancette can #78 hit the galactic branch? If not, I think this PR should be merged into the galactic branch, or else the build error being addressed by this PR is still a problem in Galactic. One recent report: https://answers.ros.org/question/390793/ros2-fails-to-build-at-rcl_logging_log4cxx/

homalozoa commented 2 years ago

@clalancette can #78 hit the galactic branch? If not, I think this PR should be merged into the galactic branch, or else the build error being addressed by this PR is still a problem in Galactic. One recent report: https://answers.ros.org/question/390793/ros2-fails-to-build-at-rcl_logging_log4cxx/

Agreed

clalancette commented 2 years ago

@clalancette can #78 hit the galactic branch?

Hm, tough one. Technically we could probably do it, as it is not being linked against by the rest of the core. But on the other hand, we don't know if any downstream packages are depending on it (unlikely, but hard to tell). So I think the safest thing would be to not merge #78 into galactic.

Thus, I'm fine with reopening this and targeting it strictly to Galactic.

clalancette commented 2 years ago

CI: