Cleanup and harmonize Carla server build to use Unreal sysroot #7151
Conversation
Ensure that actually only the unreal sysroot is deployed for all C as well C++ compilations including dependencies. That ensures the server is NOT anymore linked against the system glibc (which is in Ubuntu22.04 incompatible with the Unreal one). Harmonized naming split into client/server (instead of libcpp and libstdcpp) to ensure nothing mixed up (and there were things mixed up before!) Refactor C# Unreal build using CarlaRules base class to provide common functionality. Fixing windows build without ROS2 and added intitial windows build with ROS2 Minors: - Update osm2odr commit - Fix windows install scripts and forward arguments for building libcarla - Add some helper scripts to run under Linux
…ros2_against_unreal_dependencies
…ros2_against_unreal_dependencies
Separate installation of clang/llvm in Ubuntu is not required anymore since the UE4-clang/llvm is used for compilation of ALL dependencies to ensure the exact required sysroot is used.
LibCarla/source/carla/Exception.cpp
Outdated
| void throw_exception<std::exception>(const std::exception &); | ||
| } | ||
|
|
||
| #endif |
There was a problem hiding this comment.
EOF is missing in this file
There was a problem hiding this comment.
fixed by calling Util/Formatting/codeformat.py and placing .clang-format to LibCarla folder
LibCarla/source/carla/Exception.cpp
Outdated
| @@ -14,14 +14,14 @@ | |||
| #ifdef BOOST_NO_EXCEPTIONS | |||
|
|
|||
| namespace boost { | |||
|
|
|||
There was a problem hiding this comment.
fixed by calling Util/Formatting/codeformat.py and placing .clang-format to LibCarla folder
LibCarla/source/carla/Exception.cpp
Outdated
| void throw_exception(const std::exception &e) { | ||
| carla::throw_exception(e); | ||
| } | ||
|
|
||
| void throw_exception( | ||
| const std::exception &e, | ||
| boost::source_location const & loc) { | ||
| boost::source_location const & ) { |
There was a problem hiding this comment.
done and added (void)loc as the second option to get rid of the compiler warning on this.
| } | ||
|
|
||
| template void throw_exception<std::length_error>(const std::length_error &); | ||
| template void throw_exception<asio::service_already_exists>(const asio::service_already_exists &); |
There was a problem hiding this comment.
Done. also the other exception functions in the other file
|
I have added couple comments on your code, additionally could you merge dev branch? |
Docs/build_linux.md
Outdated
| sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/lib/llvm-10/bin/clang++ 180 && | ||
| sudo update-alternatives --install /usr/bin/clang clang /usr/lib/llvm-10/bin/clang 180 && | ||
| sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-7 180 | ||
| sudo apt-get install build-essential cmake ninja-build libvulkan1 python-is-python3 python3 python3-dev python3-pip libpng-dev libtiff5-dev libjpeg-dev tzdata sed curl unzip autoconf libtool rsync libxml2-dev git git-lfs |
There was a problem hiding this comment.
g++ is required to build modules like ROS2 and pytorch.
Why do you think we should remove it from package installation?
There was a problem hiding this comment.
only the additional g++-7 is removed (as not required) and the corresponding update alternatives call
build-essentials should bring already the default system g++ with it.
There was a problem hiding this comment.
Good catch about the build-essential package!
I have addicional worries about this change:
If we define the g++ version as system default one, this implies CARLA could be possibly build with huge number of g++ versions, as many users are building CARLA and we don't have any info about what distro are the using. On my understanding, define g++ version as system default adds the next risks:
- Sometimes code built with different g++ versions my conclude on build errors or new defects.
- Is not clear for CARLA contributors what is the minimum requiered version of g++ to mantain build backward compatibility when creating PRs.
- I'm not sure if ROS2 and pytorch libraries have some restriction about what versions of the comipler to use.
- It's pracmatic (usable) for the contributors to have a default version of g++ that is tested by us and we ensure is working. Otherwise, they could potentially have to deal with non desired build errors and this will take extra time for them. Anyway, I think we should provide a mechanism to the contributors to build with different versions of g++, but we also should provide a default stable g++ version for the contributors who don't expect to lose time dealing by unestable CARLA build sysmtem.
What are your thoughts about it?
There was a problem hiding this comment.
I see, yes your fears make sense.
I would then approach from that side. Standard supported linux system is Ubuntu. Then I would go with the active LTS versions and support their standard g++ versions. Taking their EOL dates as supporting dates might be a bit too much as Ubuntu 14.04 goes EOL right now. So taking their "End of Standard Support" as basis might be more meaningful:
Ubuntu 22.04 LTS: June 2027, gcc-11
Ubuntu 20.04 LTS: April 2025, gcc-9
Therefore, I'd either define gcc-9 or gcc-11 as gold standard for now.
On pytorch, I don't know since I have to admit that I never worked so far with it on my own. On ROS2 the respective Ubuntu default compiler for sure works.
But if CARLA supports both active Ubuntu systems (and maybe have a build job for both of them in the background to ensure), then you don't need to install a specific version here. One should then rewrite the docu in the sense that the two active Ubuntu systems are supported. And if someone else with nother Linux-Distro wants to compile CALRA he/she should install either gcc-9 or gcc-11 (at the current point in time for sure ;-)
Still, then you don't need to change the build instructions for 22.04 and 20.04, and why not making the live harder for the restistent guys who are still using 18.04 or older; If they do use such old systems they should be aware of that and be able to handle to install other gcc versions on their own if they run into problems. Or they should just upgrade their system -- which is from system security perspective in each case adviceable.
The supported clang version is in each case clear, since it's the Unreal one... which will also change soon when swithing to Unreal 5.xx engine.
So you see: you might want to extend the notes in the docu around that topic a bit and "degrade" also 18.04 to a not adviced Linux -Distro, since it has some years "on the back" (don't know if one says that also in UK like that ... it's some German slang ;-)
Please discuss with the team and adapt the commit as you decide on your own. You should have the rights to do so, I believe.
Ah, so now it's Easter in Germany: Happy searching of Easter Eggs.
There was a problem hiding this comment.
Happy Easter!
We also celebrate Easter here at Barcelona :)
…ros2_against_unreal_dependencies
Copied Util/Formatting/clang-format to LibCarla/.clang-format and formatted the Exception files (at least) to get rid of formatting errors.
|
In terms of code formatting, the .clang-format can be placed in the respective folders with different content (so the Unreal part can get one with Unreal formatting style). |
| @@ -69,12 +75,6 @@ if not exist "%GT_BUILD_DIR%" ( | |||
| cd "%GT_BUILD_DIR%" | |||
| echo %FILE_N% Generating build... | |||
|
|
|||
| echo.%GENERATOR% | findstr /C:"Visual Studio" >nul && ( | |||
| set PLATFORM=-A x64 | |||
There was a problem hiding this comment.
I share some discusion where is mentioned that buiding without -A x64 flag casues build error in some environments:
#7237
I'm not sure about why this build errors are happening. I'm worried about removing this code could introduce more build errors like the described in the shared discussion. Coudl your share your thoughts about it?
There was a problem hiding this comment.
The point is that I had build errors on my windows machine on this. Otherwise I wouldn't have changed it. But if you look more detailed onto the diff, I've just moved the code in the batch-file some lines upwards to line 40 that it is more or less identical to the other files.
But I could imagine also what the problems of others where. If you look into line 19 I've added the GENERATOR variable to this (and other scripts) with empty definitions per default. So in the batch file the GENERATOR variable wasn't defined at all if nothing is passed by the command line options. Then the original code in line 72 does somehow fail.
Therefore, I've tried to follow the "copy exactly" formula and adapted all (most?) of the batch files to
- define the GENERATOR somewhere at top BEFORE command argument parsing
- put the (full) PLATFORM definition lines relatively close to the end of the coomand line parsing.
You see this pattern in the below batch files, too. That's then part of the "harmonizing" work of this PR.
|
Hey Bernd! When launching a make clean I receive this error: Util/BuildTools/Linux.mk:163: warning: overriding recipe for target 'downloadplugins' Then when I do a make launch, this issue appears: FAILED: LibCarla/cmake/test/CMakeFiles/libcarla_test_server_release.dir///source/test/common/test_buffer.cpp.o include <gtest/gtest.h> |
…ros2_against_unreal_dependencies
- Move gcc-7 to gcc-13 since that's the default compiler on Ubuntu 24.4LTS. - Fix make clean - Fix make launch (without ROS2 args) - Fixed wrong syntax in Linux.mk - Fix missing ROS2 include when compiled without ROS - Remove Util/Running folder again - Remove LibCarla/.clang-format file
|
I wasn't able to reproduce the gtest.h issue, even uninstalling my ros2-humble packages including gtest headers. Nevertheless I've found other compilation issues, which I fixed ;-) |
commit def46b8edc34e470caea445423add114eedf6585
Author: Bernd Gassmann <bernd.gassmann@intel.com>
Date: Thu Apr 18 16:27:08 2024 +0200
Fix proj and asio build
checkout the proj 7.2 branch with required compilation fixes on newer
systems
let asio only install the data instead of compiling the tests/examples
which we don't want/need
berndgassmann
force-pushed
the
berndgassmann/compile_ros2_against_unreal_dependencies
branch
from
b78bd19
to
940271a
Compare
Ensure that actually only the unreal sysroot is deployed for all C as
well C++ compilations including dependencies. That ensures the server is
NOT anymore linked against the system glibc (which is in Ubuntu22.04
incompatible with the Unreal one).
Harmonized naming split into client/server (instead of libcpp and
libstdcpp) to ensure nothing mixed up (and there were things mixed up
before!)
Refactor C# Unreal build using CarlaRules base class to provide common
functionality. Fixing windows build without ROS2 and added intitial
windows build with ROS2
Minors:
libcarla
Description
Fixes #
Where has this been tested?
Ubunut 22.04, Ros2 humble, python 3.10
Windows 11, ahhh, no idea what python version there was installed, assume python3.10, but python is in each case irrelevant for this commit as it's mainly affecting the server build. Let's see if the CI builds under Windows (still) correctly
Possible Drawbacks