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

Support GCC 11 #285

Closed
Blutkoete opened this issue May 25, 2021 · 3 comments
Closed

Support GCC 11 #285

Blutkoete opened this issue May 25, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@Blutkoete
Copy link
Contributor

Blutkoete commented May 25, 2021

Is your feature request related to a problem? Please describe.
Arch Linux recently switched to GCC 11 and a minor patch is required to compile eCAL with it. The patch only adds two includes to two files (see "Header dependency changes" at https://gcc.gnu.org/gcc-11/porting_to.html for more information) that were implicit before and need to be explicit with GCC 11. As this does not interfere with building on officially supported platforms, I'd like to ask if it is possible to add this patch.

Describe the solution you'd like
The patch is applied to the official eCAL repository.

Describe alternatives you've considered
I have no problem to apply this patch during the build process of the AUR package each time which clearly is an alternative; but applying the patch to the official code is convenient for me and saves you a minor headache later when an officially supported platform moves to GCC 11.

Additional context
I also have a patch changing one line to support Protobuf >= 3.15.6, but that breaks compiling against lower versions of Protobuf and so I'll keep applying it in the AUR package (it's just one line that changes, so no worries for the future).

Patch

Index: samples/cpp/multiple/multiple_snd/src/multiple_snd.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/samples/cpp/multiple/multiple_snd/src/multiple_snd.cpp b/samples/cpp/multiple/multiple_snd/src/multiple_snd.cpp
--- a/samples/cpp/multiple/multiple_snd/src/multiple_snd.cpp	(revision b160392e47800319a498b2ff30c85cb418bb794c)
+++ b/samples/cpp/multiple/multiple_snd/src/multiple_snd.cpp	(date 1621537793427)
@@ -25,6 +25,7 @@
 #include <atomic>
 #include <chrono>
 #include <thread>
+#include <memory>
 
 #define PAYLOAD_SIZE      1024
 #define PRINT_LOG            0
Index: app/rec/rec_addon_core/src/time_limited_queue.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/app/rec/rec_addon_core/src/time_limited_queue.h b/app/rec/rec_addon_core/src/time_limited_queue.h
--- a/app/rec/rec_addon_core/src/time_limited_queue.h	(revision b160392e47800319a498b2ff30c85cb418bb794c)
+++ b/app/rec/rec_addon_core/src/time_limited_queue.h	(date 1621537489591)
@@ -25,6 +25,7 @@
 #include <chrono>
 #include <thread>
 #include <functional>
+#include <condition_variable>
 
 template <class T>
 class TimeLimtedQueue : public std::enable_shared_from_this<TimeLimtedQueue<T>>
@FlorianReimold
Copy link
Member

Hey, thanks for the patch! Of course we want to have a fix like that in our eCAL repository. There are rumours that in a few centuries even Ubuntu will support gcc11, so it's nice to be prepared for that 😉
Do you want to create a PR, or shall I manually add the patch?

And btw, I am also interested in the protobuf 3.15.6 fix. A while back I prepared eCAL for protobuf 3.15.3, but we decided to not switch to it in the forseeable future, as it broke compatibility with earlier protobuf Versions due to the claimed pb namespace (-> issue).

@FlorianReimold FlorianReimold added the enhancement New feature or request label May 26, 2021
@Blutkoete
Copy link
Contributor Author

Blutkoete commented May 26, 2021

I think I'll create PRs this weekend (no need to hurry for this) and create one branch per compatbility patch so you can pick what you want once it is either necessary or not interfering with the supported OS builds.

After reading through the issue you linked, I think I did not really create a patch to build with protobuf >= 3.15.6, but one to build against 3.16.0 - I jumped to conclusions because I looked at the protobuf repository to identify when the change that failed the build with 3.16.0 was introduced and it was with 3.15.6, but it looks like they solved the pb namespace problem with 3.16.0, so my patch is probably actually for "building with protobuf >= 3.16.0", which is just a one-line change.

I never had issues with GCC 10 by the way and all warnings that I get currently get are deprecation warnings from my newer Qt versions, so kudos to you all for doing a great job in keeping eCAL portable & upgradeable (especially upgradeability is a feature painfully often missing in the automotive context).

@FlorianReimold
Copy link
Member

FlorianReimold commented May 27, 2021

Protobuf removed the namespace claming with some later 3.15.x version again. So actually our Projects (and eCAL) should be upgradable to 3.16 or higher. The only plattform we manually decide the protobuf version for is Windows, all other plattforms bring their own specific protobuf version, anyways.

Yesterday I have noticed a compile issue with our macOS build caused by protobuf. As we are using homebrew to install protobuf and homebrew just forcefully upgrades you to the latest and greatest version, the macOS build is great for detecting issues with the latest protobuf. Later that day however the macOS build magically fixed itself. At the moment (2021-05-27 08:45 UTC+2), homebrew installs protobuf 3.17.1. I don't know which version it was when the build failed, but it may have been something different. But it looks like protobuf 3.17.1 is working just fine.

Failed build:
https://github.com/continental/ecal/runs/2674341994?check_suite_focus=true

All macOS builds, latest work again without changing anything:
https://github.com/continental/ecal/actions/workflows/build-macos.yml

BTW, thanks for the kind words 😊. We are trying hard to keep eCAL compatible with a wide variety of compiler versions and operating systems. Since we have official Ubuntu PPAs we even support non-LTS Ubuntus; at the moment those are Ubuntu 20.10 and 21.04. Therefore the range of gcc versions is 5.4 to 10.3, along with msvc 2015 - 2019 for Windows and Clang for macOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants