-
Notifications
You must be signed in to change notification settings - Fork 17
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
Update to use latest protobuf #436
Conversation
Fixes build with a more recent version of the protobuf library (24.4) by replacing old function calls with their Abseil (20230125) equivalents.
Thanks for improving on this. I have to remember to go back and fix the workflows on my PR :') I might offer that the reason only c++17 works is that you may be linking to a system abseil library compiled with C++17 flags (the linked library is not cross-C++ runtime ABI compatible). That's why I also needed C++17 on my machine to get this to compile, but I opted not to include that in the PR. |
Fails CI (which should at least pass ccl and sbcl |
CI fixed (maybe). They weren't installing absl in the setup steps.
That makes sense. I'm using the current version of protobuf that's in the Arch Linux package repository, so I didn't compile it myself. |
CCL and SBCL are not yet passing CI |
So, the issue is the ubuntu repository used in ubuntu-latest doesn't have absl_absl_log in libabsl-dev. |
https://abseil.io/docs/cpp/tools/cmake-installs Installing is slow, not quite sure what to do about that, Note: This did work. |
work picked up - unfortunately too distracted to give this as much time as I wanted nicely, this is superseded by #438 |
This is based on the work in #434 by @Partmedia
This PR adds the missing
include
statements as well as a few tweaks to the Makefile to get everything working.Changes to the Makefile
-std=c++17
instead of-std=c++14
. It wasn't working on my machine with 14pkg-config
to setup linking for absl and protobufmake