ros / rosconsole

17 stars 61 forks source link

be compatible with `log4cxx` `0.11` and `0.12` #54

Open dreuter opened 2 years ago

dreuter commented 2 years ago

log4cxx uses std::shared_ptr since version 0.12

Unfortunately Ubuntu 22.04 ships with 0.12, which means that rosconsole with the log4cxx backend wouldn't compile. This is also a problem on other distros such as Arch or Gentoo.

I have carefully applied explicit construction/conversion and &* instead of .get() to make the code compile with both versions of log4cxx.

Fixes #50

dreuter commented 2 years ago

If you want to test this easily I have created two quick and dirty containers that I have tested with: branch: https://github.com/dreuter/rosconsole/tree/docker-to-verify

podman build -t rosconsole-test:jammy jammy
podman run -v$(pwd):/root/ws/src/rosconsole rosconsole-test:jammy

Does work with docker as well (just replace podman with docker)

AchmadFathoni commented 2 years ago

Got this error when compile it with log4cxx 0.13

 50%] Building CXX object CMakeFiles/rosconsole_log4cxx.dir/src/rosconsole/impl/rosconsole_log4cxx.cpp.o
/home/toni/.cache/yay/ros-noetic-rosconsole/src/rosconsole-1.14.3/src/rosconsole/impl/rosconsole_log4cxx.cpp: In function ‘void ros::console::impl::print(void*, ros::console::Level, const char*, const char*, const char*, int)’:
/home/toni/.cache/yay/ros-noetic-rosconsole/src/rosconsole-1.14.3/src/rosconsole/impl/rosconsole_log4cxx.cpp:187:98: error: no matching function for call to ‘log4cxx::spi::LocationInfo::LocationInfo(const char*&, const char*&, int&)’
  187 |     logger->forcedLog(g_level_lookup[level], str, log4cxx::spi::LocationInfo(file, function, line));
      |                                                                                                  ^
In file included from /usr/include/log4cxx/logger.h:33,
                 from /usr/include/log4cxx/spi/loggingevent.h:28,
                 from /usr/include/log4cxx/layout.h:29,
                 from /usr/include/log4cxx/appenderskeleton.h:28,
                 from /home/toni/.cache/yay/ros-noetic-rosconsole/src/rosconsole-1.14.3/src/rosconsole/impl/rosconsole_log4cxx.cpp:42:
/usr/include/log4cxx/spi/location/locationinfo.h:94:17: note: candidate: ‘log4cxx::spi::LocationInfo::LocationInfo(const log4cxx::spi::LocationInfo&)’
   94 |                 LocationInfo( const LocationInfo& src );
      |                 ^~~~~~~~~~~~
/usr/include/log4cxx/spi/location/locationinfo.h:94:17: note:   candidate expects 1 argument, 3 provided
/usr/include/log4cxx/spi/location/locationinfo.h:88:17: note: candidate: ‘log4cxx::spi::LocationInfo::LocationInfo()’
   88 |                 LocationInfo();
      |                 ^~~~~~~~~~~~
/usr/include/log4cxx/spi/location/locationinfo.h:88:17: note:   candidate expects 0 arguments, 3 provided
/usr/include/log4cxx/spi/location/locationinfo.h:80:17: note: candidate: ‘log4cxx::spi::LocationInfo::LocationInfo(const char*, const char*, const char*, int)’
   80 |                 LocationInfo( const char* const fileName,
      |                 ^~~~~~~~~~~~
/usr/include/log4cxx/spi/location/locationinfo.h:80:17: note:   candidate expects 4 arguments, 3 provided
make[2]: *** [CMakeFiles/rosconsole_log4cxx.dir/build.make:76: CMakeFiles/rosconsole_log4cxx.dir/src/rosconsole/impl/rosconsole_log4cxx.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:353: CMakeFiles/rosconsole_log4cxx.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
dreuter commented 2 years ago

The culprit seems to be: https://github.com/apache/logging-log4cxx/pull/95

I will need to see how I can fix that :/

jspricke commented 2 years ago

for log4cxx 0.13 you can do something like:

#ifdef LOG4CXX_VERSION_MAJOR
    logger->forcedLog(g_level_lookup[level], str, log4cxx::spi::LocationInfo(file, log4cxx::spi::LocationInfo::calcShortFileName(file), function, line));
#else
     logger->forcedLog(g_level_lookup[level], str, log4cxx::spi::LocationInfo(file, function, line));
#endif
defenzelite-robotics commented 4 months ago

Hello guys i am getting the same error but for ubuntu 24.04 i have checked the version of log4cxx is 1.1.0-1build3: amd64 arm64 armhf ppc64el riscv64 s390x what changes needs to be done ?

erikbeall commented 4 months ago

@defenzelite-robotics I also attempted to build with log4cxx-1.1.0-1build3 and I'm not enough of a user of C++ to have found a solution. For a quick hack you could set the ROSCONSOLE_BACKEND to print or glog in the CMakeLists.txt inside the rosconsole base source directory in your build (e.g. ~/ros_catkin_ws/src/rosconsole/CMakeLists.txt).

However, beware that there are other issues with >Ubuntu20 compatibility further along the chain - this is only the first blocker I ran into (noetic is not fully supported past Ubuntu 20.04). I've run thru about ten other mini blockers trying to get roscore to come up (I have ROS2 in parallel and need ROS1 for already-deployed systems as we gradually migrate) with more blockers still coming (I'll likely just end up using docker for ROS1).

lucasw commented 4 months ago

You might be interested in https://github.com/lucasw/rosconsole/tree/ubuntu_2210 - it's got some necessary patches including the log4cxx fix and some changes of mine for formatting the output differently (those ought to be easy to remove if you made your own branch)- I've been using it fine in 22.04 (using the debian apt installed ros packages with others built from source described here - rosconsole doesn't need to be built from source on 22.04 but I do it for my alterations).

For Ubuntu 24.04 all the base packages need to be built from source but after that it's a similar process. If you try that and have trouble feel free to open an issue in https://github.com/lucasw/ros_from_src/issues (and better to discuss there or maybe https://github.com/ros-o/ros-o/discussions instead of getting more and more off topic in this thread).

erikbeall commented 4 months ago

Thank you Lucas, your fixes for 22 worked on Ubuntu 24 with log4cxx-1.1! For completeness for other users, I cloned lucas's referenced repo in the src subdirectory and checked-out the ubuntu_2210 branch, then built, e.g. for just that package: ~/ros_catkin_ws $ ./src/catkin/bin/catkin_make_isolated --install -DCMAKE_BUILD_TYPE=Release --pkg rosconsole

I'm still working on getting roscore fully working, strace shows a few differences before the hang vs an Ubuntu 20 noetic stock, will report back on your ros_from_source/issues if I get further (and will start from your build instructions).

erikbeall commented 4 months ago

Lucas, your ubuntu2404 branch worked to build a working roscore! I was able to build other components without no major challenges (perception, robot - I skipped diagnostic_common and perception_pcl). Note to others trying to build for Ubuntu 24, use Lucas's ubuntu_2404 branch at https://github.com/lucasw/ros_from_src/.

lucasw commented 4 months ago

I can't promise any stability from any of those forks, and sometimes I force push into them and rewrite commit history- if you have something that works now and you don't want deal with upstream issues my changes could create you should stay on an exact commit (or even make your own fork and branch at that commit for another layer of protection), and try out upstream changes only when you want to.

jspricke commented 4 months ago

@erikbeall you can also use the ROS One repos: https://github.com/ros-o/ those contain all relevant patches and should give you a more stable base.

lucasw commented 4 months ago

All my forks are based off of ros-o obese-devel branches where available, but then I found I had to add some debian patches like https://salsa.debian.org/science-team/ros-rosconsole/-/tree/master/debian/patches as well (for example https://github.com/lucasw/rosconsole/commit/048c41005e0cfd2c796226cc10a3632920f53231 )- but maybe that was only because I was overlaying apt installed debian packages that had all those patches in them, and in Ubuntu 24.04 the debian packages aren't available so that isn't necessary? I haven't tried that (but it's no advantage to me if I could since I want to use the same branches in 22.04 and 24.04 and use pre-built as much as possible).

jspricke commented 4 months ago

@lucasw It could be that we miss some patches in ros-o but Soname patch you linked is surely not needed. Feel free to open bugs in ros-o if you find missing patches.