abseil-cpp
abseil-cpp copied to clipboard
abseil
CMake Targets
Here's a non-exhaustive list of Abseil CMake public targets:
Could the list be completed?
https://github.com/abseil/abseil-cpp/tree/master/CMake
set(CMAKE_CXX_FLAGS "-std=c++11 -stdlib=libc++ ${CMAKE_CXX_FLAGS}")
AFAIK that's not the proper way to select c++11.
Hey there! Thanks for the comments.
We will definitely update the readme. I just got back from vacation and am triaging all the CMake related issues and once they're all together I'll update all the information.
The correct way to set the language level for abseil is to set CMAKE_CXX_STANDARD to 11, 14, or 17. We use it here and then here and here.
Is it OK if Abseil is build with 11 and the program itself is build with 17?
I haven't tried it, but that's extremely likely to go badly because the underlying types for absl::string_view are different in C++11 and C++17 compilation modes (https://github.com/abseil/abseil-cpp/blob/master/absl/strings/string_view.h#L33).
Can you give us a little more background on why you'd like to compile Abseil as C++11 if the rest of your program is C++17? This isn't something we've heard of people wanting to do before. If we understand the problem better, we may be able to offer alternatives or work on a fix if the problem is on our end.
It's actually related to https://github.com/abseil/abseil-cpp/issues/260
If it's a bad idea and the compiler defaults to C++14 (GCC 6) and Abseil defaults to C++11 then something is wrong.
Abseil uses the value of CMAKE_CXX_STANDARD since this is the cmake way of controlling C++ language level for all targets. How are you specifying C++17 in your build?
set(CMAKE_CXX_FLAGS -std=c++17) which is wrong. ;)
But what if c++17 became the default in a future GCC, like c++14 already is? CMAKE_CXX_STANDARD might not be set in that case.
Abseil supports compilers for 5 years, which includes a lot of versions of GCC for which C++14 is not the default. If Abseil changed the default C++ version, then users of those old but supported GCC versions who rely on the default would have their builds break when they pull in the latest version of Abseil.
I don't think there's a good answer here. It's really important that users specify what version of C++ they're using. I can see an argument that not specifying a version should be an error instead of a warning, though.
The correct way to set the language level for abseil is to
set CMAKE_CXX_STANDARDto 11, 14, or 17.
If I may make a comment on this: In my experience this is the absolutely wrong approach for a library. Especially if your API changes depending on the language level, then the used standard is something that you should simply adopt from the project abseil is used from.
Otherwise there are two possibilities:
- Either the library and the application happen to use the same c++ version and doing a
set CMAKE_CXX_STANDARDin the library cmake file is redundant, - or both are using a different version and everything is blowing up in your face.
set CMAKE_CXX_STANDARD belongs either in the top level cmake file of the Application (if the libraries are added via add_subdirectory( ... ) ) or even better in the toolchain file that is used to build all the applications and libraries or (e.g. for testing and ci) in the command line command that invokes cmake( cmake -DCMAKE_CXX_STANDARD=17 ..).
The only thing a library cmake file should do is a target_compile_features(mylib PUBLIC cxx_std_11) to state the minimum language version it needs.
If I may make a comment on this: In my experience this is the absolutely wrong approach for a library. Especially if your API changes depending on the language level, then the used standard is something that you should simply adopt from the project abseil is used from.
How do you propose to do this besides the standard mechanism in CMake?
Otherwise there are two possibilities:
Either the library and the application happen to use the same c++ version and doing a set CMAKE_CXX_STANDARD in the library cmake file is redundant, or both are using a different version and everything is blowing up in your face. set CMAKE_CXX_STANDARD belongs either in the top level cmake file of the Application (if the libraries are added via add_subdirectory( ... ) ) or even better in the toolchain file that is used to build all the applications and libraries or (e.g. for testing and ci) in the command line command that invokes cmake( cmake -DCMAKE_CXX_STANDARD=17 ..).
We don't set CMAKE_CXX_STANDARD, we only read it. The relvant code is here In fact our test scripts do what you suggested, using -DCMAKE_CXX_STANDARD in the command line.
The only thing a library cmake file should do is a target_compile_features(mylib PUBLIC cxx_std_11) to state the minimum language version it needs.
But this is a problem in a library like abseil which changes depending on language level. We need to know what language level our dependents are using, and so we must use the tool given to us in CMake to detect C++ language level.
We don't set CMAKE_CXX_STANDARD, we only read it.
That might not be right. If it's unset, you default to C++11 which might not be what the compiler defaults to.
@JonathanDCohen: Sorry, my mistake. I read this as "the correct way is for abseil to use set(CMAKE_CXX_STANDARD". I shouldn't post when I'm tired.
My broader point stands though: you should not set an explicit cmake standard for individual targets./libraries
How do you propose to do this besides the standard mechanism in CMake?
Just do nothing. If the parent project / toolchain file sets CMAKE_CXX_STANDARD, then your targets will automatically adopt it. If your parent manually adds -std=c++11 to the global CXX flags (it shouldn't but...) your targets will adopt them. And if the parent just uses the compiler default, then so will abseil.
The only thing a library cmake file should do is a target_compile_features(mylib PUBLIC cxx_std_11) to state the minimum language version it needs.
But this is a problem in a library like abseil which changes depending on language level.
cxx_std_11 only sets the minimum required level to 11. If your parent sets CMAKE_CXX_STANDARD to 14 or the compilers default is c++14, abseil will still be compiled with c++14. Also, as a public property, this propagates to your users, so even if they use a compiler that doesn't default to c++11 or later (gcc4.9) and they don't manually set the standard level in one way or another, your users will automatically be compiled with c++11 (EDIT: And we can argue whether that is actually desirable or not).
The only situation, where this unfortunately doesn't work is if another library uses e.g. target_compile_features(mylib PUBLIC cxx_std_17), you are using target_compile_features(abseil PUBLIC cxx_std_11) and all your parent does is target_link_libraries(myexec mylib abseil) (wihtout specifying a global language level). In that case, abseil will be compiled with c++11, but myexec with c++17. Unfortunately, I don't think cmake offers any solution to this problem.
We need to know what language level our dependents are using, and so we must use the tool given to us in CMake to detect C++ language level.
Do you actually have to know this anywhere in the cmake scripts or does it just have to be consistent? The less individual libraries meddle around with compile flags the easier it is to compile everything with a consistent set of flags, because that is what cmake does by default automatically.
so even if they use a compiler that doesn't default to c++11 or later (gcc4.9)
IIRC the default has never been C++11. GCC 6 defaults to C++14, before that it was C++03 (or 98).
The only situation, where this unfortunately doesn't work is if another library uses e.g.
target_compile_features(mylib PUBLIC cxx_std_17), you are usingtarget_compile_features(abseil PUBLIC cxx_std_11)and all your parent does istarget_link_libraries(myexec mylib abseil)(wihtout specifying a global language level). In that case, abseil will be compiled with c++11, but myexec with c++17. Unfortunately, I don't think cmake offers any solution to this problem.
Won't the requirement of mylib bubble up and cause everything to use C++17?
Won't the requirement of mylib bubble up and cause everything to use C++17?
It will bubble up to myexec, but not down to abseil, which is why I'm still on the fence, if libraries should not even use the target_compile_features(mylib PUBLIC cxx_std_XX) mechanism and just error out if the parent doesn't specify a sufficient language standard.
But if myexec sets it directly it trickles down to abseil, isn't that a bit weird?
But if myexec sets it directly it trickles down to abseil, isn't that a bit weird?
No it doesn't.
Only if you set the language standard globally in the parent cmake file (E.g. via CMAKE_CXX_STANDARD) the setting will trickle down.
Sounds like a design bug in cmake.
I certainly don't like the situation, but I'm not sure what a better solution would look like.
A single library can be used from mutliple different other targets and not all of them might be compiled as part of same cmake project (or compiled by cmake at all), so it makes sense that "requirements" trickle up the dependency chain, but you can't realistically enforce that they are also propageted down, as that would lead to conflicts and/or you'd have to travel back in time.
If modes can't be mixed you should have separate library builds for each mode.. like you have separate builds for amd64 and arm and x86.
Or you shouldn't have libraries at all and build everything as part of the executable (like I think Google does).
True, and cmake absolutely allows for that usage pattern. As I said: don't mess with compiler flags in library cmake files and (almost) everything is going to be fine. Instead define your toolchain parameters (compiler, c++ standard version, standard library, debug/release flags) in a toolchain file and use that toolchain to build all your libraries consistently.
The remaining problems I encounter are
- Libraries that can't be build with cmake (e.g. boost)
- Libraries that try to be overly clever and use autodetection, options and "configuration targets" to influence how they are build (almost all libraries out there)
- Developers that misuse a system's package manager like apt for c++ dependency management.
Pesonally I think the fundamental problem is actullay that c++ libraries like to scatter #ifdefs all around their interface. With the current compilation model that relies on textual inclusion and the package management story being what it is, that is just not viable.
A very good talk on that topic https://www.youtube.com/watch?v=sBP17HQAQjk
Sometimes I wonder if it wouldn't be a better approach, if headers got installed in a preprocessed manner, so there is no chance they get used in different ways in different translation units.
So the offending bit of code is really right here .
Do you actually have to know this anywhere in the cmake scripts or does it just have to be consistent?
It has to be consistent everywhere abseil is used. The issue raised aboe with usage requirements is why we didn't use them. We could just error out of CMAKE_CXX_STANDARD is 98 and else just use whatever cmake gives us I suppose. Do we have an idea of what the surface area is for breaking people if we make this change? We wouldn't be able to ship a tool to fix projects as it would depend on them setting CMAKE_CXX_STANDARD either in the cmake files or command line invocations.
Or you shouldn't have libraries at all and build everything as part of the executable (like I think Google does).
This is the current recommended way to use abseil, via add_subdirectory(). We are in the middle of writing out the install() rules to install Abseil headers, but we will only support using installed headers with an LTS version of abseil in order to prevent ODR violations due to multiple different versions of abseil being used. We are still investigating how to support Abseil as a shared library, as there are parts of what we do which won't work in one.
So the offending bit of code is really right here .
And in particular here: https://github.com/abseil/abseil-cpp/blob/7bd8f36c741c7cbe311611d7981bf38ba04c6fef/CMake/AbseilHelpers.cmake#L210
Do we have an idea of what the surface area is for breaking people if we make this change?
I think the projects that could get broken by this (and which aren't broken at the moment) are the ones that
- Specify the language level not via
CMAKE_CXX_STANDARDbut e.g. rely on defaults and/or usage requirements (otherwise the fallback path isn't exercised) - Have targets consuming abseil that are compiled in c++11 (otherwise abseil's default would be wrong anyway)
- Use a conpiler that has a c++11 mode, but defaults to something different (otherwise the abseil will be compiled with the same c++ version as before)
- I think 3 is very common among the users of abseil (iirc all supported versions of gcc and clang).
- I could imagine that 2 is happening quite often via the
target_compile_features(mytarget PUBLIC cxx_std_XX)while using an "older" compiler. - I have no Idea how common 1 is.
The issue raised aboe with usage requirements is why we didn't use them.
If I'm not mistaken, adding a usage requirement would result in zero beakage (all consumers of abseil are already compiled with at least c++11 and so is abseil itself), which is why I suggested it, but I agree that it comes with its own bag of problems and might not be the right mechanism for abseil.
Btw.: You can specify the reuqired compile features as private, so it doesn't propagate to your consumers if that makes you feel better.
Coming back around. It seems like the answer is from the thread -- we can set target_compile_features(mylib PUBLIC cxx_std_11) to at least force direct abseil users to have C++11 set, and then read from CMAKE_CXX_STANDARD to get the actual level, erroring out if it's unset.
This is a potential breaking change for Abseil users, and due to the unpredictable nature of how this global could propagate, I want to be very careful making the change, as we can't provide a fix for people's CMake builds as we promise in our developer guidelines.
What do we think if this idea -- provide a flag -DABSL_NO_DEFAULT_STD_VALUE for people to try to make sure their code builds, and then after some amount of time (6 months? Probably at a particular LTS release) remove the flag
What do we think if this idea -- provide a flag -DABSL_NO_DEFAULT_STD_VALUE for people to try to make sure their code builds,
This will result in very limited testing..
and then read from CMAKE_CXX_STANDARD to get the actual level, erroring out if it's unset.
Why is this necessary?
Just to clarify my post above: Removing the whole machinery from the cmake file without adding the requirement should only break projects that satisfy all 3 conditions I've listed at the same time.
If you remove the existing machinery and add the cxx_std_11 requirement, I believe this should break no one (although as titus demonstrated so nicety in his cppcon talk - even whitespace changes can break someone somewhere).
Unfortunately, I'm not sure how to best validate this claim.
This is a potential breaking change for Abseil users, and due to the unpredictable nature of how this global could propagate,
As I said, if you make the requirement private it doesn't propagate at all.
I did some experimenting, with compilers which default both to c++98 and to c++14. It seems that for pretty much all cases using a private requirement should work for us. The experiments are in https://github.com/JonathanDCohen/CMakeVersionExample.
The specific problem mentioned by @MikeGitb I think is a non-issue, because I don't see how all of those conditions can co-exist. If the compiler defaults to C++14, but has a target which specifies via usage requirements to use C++11, and then we change Abseil to use C++11 via usage requirement, it will be the same effect, that Abseil is build with C++11. On the other hand if the client library specifies CMAKE_CXX_STANDARD=11, then Abseil was already being built with C++11 anyways, so we're fine.
So all in all it seems like this is actually just a refactor and shouldn't change behavior at all, but will give us a little cleaner code. I'll write up this change tomorrow when I get back to work :)
We have hit a roadblock -- the cxx_std_11 compile feature is only available starting in cmake 3.8. See https://cmake.org/cmake/help/v3.8/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html vs https://cmake.org/cmake/help/v3.7/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html.
I recently pushed a change to remove the use of list(FILTER becuase it only appears in cmake 3.6, and we found that cmake 3.5 is the version bundled with, for example, ubuntu 14 LTS and ubuntu 16 LTS, so we can't reasonably introduce a cmake 3.8 feature.
I think it's time to reach out to our users and see what versions of cmake everyone is using. It is pretty easy to keep cmake up to date as I've found, and so far the decisions for where to draw the line on cmake versions has been pretty ad-hoc. For now I'm going to put a hold on this until we can set a more firm version requirement.
I did some experimenting, with compilers which default both to c++98 and to c++14. It seems that for pretty much all cases using a private requirement should work for us. The experiments are in https://github.com/JonathanDCohen/CMakeVersionExample.
So the 'private' requirement upgrades but also downgrades the level the client is build with, if the client is using defaults? What's private about that? The downgrading seems problematic.
It will not downgrade, but if the project is compiled with a compiler that defaults to 14, abseil will also be compiled with 14 it just makes sure you don't get compiled with something older than c++11.
It is private, because it doesn't influence the user of the library.
knowing that we have:
https://github.com/abseil/abseil-cpp/blob/b312c3cb53a0aad75a85ac2bf57c4a614fbd48d4/absl/copts/AbseilConfigureCopts.cmake#L38-L45
todo: we should verify CMAKE_CXX_STANDARD >= 11, if user set 03
An other way would be to change the code here: https://github.com/abseil/abseil-cpp/blob/b312c3cb53a0aad75a85ac2bf57c4a614fbd48d4/CMake/AbseilHelpers.cmake#L130-L132 and here https://github.com/abseil/abseil-cpp/blob/b312c3cb53a0aad75a85ac2bf57c4a614fbd48d4/CMake/AbseilHelpers.cmake#L217-L218 by:
# INTERFACE libraries can't have the CXX_STANDARD property set
# For cxx_std_11 see https://cmake.org/cmake/help/v3.8/release/3.8.html#c-c
if(${CMAKE_VERSION} VERSION_GREATER_EQUAL 3.8.2)
target_compile_features(${_NAME} PRIVATE cxx_std_11)
else()
set_property(TARGET ${_NAME} PROPERTY CXX_STANDARD ${ABSL_CXX_STANDARD})
set_property(TARGET ${_NAME} PROPERTY CXX_STANDARD_REQUIRED ON)
endif()
@JonathanDCohen you could update the CL 235732173 accordingly ?
