Update abseil to the latest LTS. #1725
Conversation
bstaletic
force-pushed
the
abseil-update
branch
from
66fac7a to
d852511
Compare
Instead of reinventing FetchContent_MakeAvailable (badly), we can just use the CMake provided utility. It is introduced in cmake 3.14, which also matches our minim required cmake version.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1725 +/- ##
=======================================
Coverage 95.47% 95.47%
=======================================
Files 83 83
Lines 8177 8177
Branches 163 163
=======================================
Hits 7807 7807
Misses 320 320
Partials 50 50 |
There was a problem hiding this comment.
What happens if we enable that flag? Our build is c++17 so abseil should just use that and STFU
Reviewable status: 1 of 2 LGTMs obtained
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
|
An eyes-only review shows it has the patch necessary for the OpenBSD/FreeBSD fix. I'll do full build of it on my OpenBSD machine in a little bit and confirm it works, but I fully expect it will! |
bstaletic
force-pushed
the
abseil-update
branch
from
d852511 to
4d1accc
Compare
The update to the latest LTS fixes compilation on some BSD systems. The new option before making abseil available has to do with dependencies propagating their needs for C++ standard up to the projects that depend on them. If a library needs C++14, then the application depending on that library needs at least C++14, but can opt for newer standards too.
There was a problem hiding this comment.
What happens if we enable that flag? Our build is c++17 so abseil should just use that and STFU
Tried it, it's fine. In general, a dependency saying "needs C++14" means "needs C++14 or newer". Plus abseil knows about C++17 and switches to it after turning the option on.
-- Performing Test ABSL_INTERNAL_AT_LEAST_CXX17
-- Performing Test ABSL_INTERNAL_AT_LEAST_CXX17 - Success
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
|
Thanks for sending a PR! |
|
Thanks for sending a PR! |
This should be enough to fix ycm-core/YouCompleteMe#4212
@Jaywalker, would you mind double checking my choice of abseil commit?
@puremourning Abseil's cmake is warning about this:
https://github.com/abseil/abseil-cpp/blob/lts_2023_08_02/CMakeLists.txt#L73-L78
We should figure out how to set that.
This change is