treewide: define default C&C++ versions for compilers#255979
treewide: define default C&C++ versions for compilers#255979tobim wants to merge 1 commit intoNixOS:masterfrom
Conversation
The variables `defaultCStandard` and `defaultCXXStandard` make the defaults of the `-std` option known to Nix. This knowledge can be used to decide whether that option must be passed explictly for packages with specific standard requirements. Usage is demonstrated in `protobuf` and `grpc`.
| ] ++ lib.optionals (stdenv.cc.cc.defaultCxxStandard < 14) [ | ||
| "-DCMAKE_CXX_STANDARD=14" |
There was a problem hiding this comment.
| ] ++ lib.optionals (stdenv.cc.cc.defaultCxxStandard < 14) [ | |
| "-DCMAKE_CXX_STANDARD=14" | |
| ] ++ lib.optionals (stdenv.cc.cc.defaultCxxStandard < 14) [ | |
| "-DCMAKE_CXX_STANDARD=14" |
What??
How can stdenv.cc.cc.defaultCxxStandard be less than 14 but -DCMAKE_CXX_STANDARD be 14?
There was a problem hiding this comment.
The idea here is to pass -DCMAKE_CXX_STANDARD only if the compiler in use defaults to an older language version. C++14 is the minimum required to build protobuf.
There was a problem hiding this comment.
Oh, I understood. The default C++ standard is not the same as the supported C++ standard.
However it looks too superfluous.
"If the default C++ standard of this compiler is not 14, then force-set the C++ standard to 14".
Why not force-set the C++ standard to 14 and ignore the compiler's default?
There was a problem hiding this comment.
If the default is newer than 14 some of the dependencies are not API compatible (cough, abseil, cough), so that approach doesn't work.
There was a problem hiding this comment.
We should write a comment to explain that so that the next guy who looks as this doesn't go through the same process.
something like
# protobuf needs at least c++14 to be compiled
# if the default is higher don't override it as abseil for example won't be api compatible
| passthru = { | ||
| inherit libllvm; | ||
| isClang = true; | ||
| defaultCStandard = 11; |
There was a problem hiding this comment.
Can this be treated as int?
There was a problem hiding this comment.
I'm not sure what you mean?
In case you are thinking of gnu11 vs c++11: That almost never makes a difference. Allowing arithmetic on this value is what makes it useful.
There was a problem hiding this comment.
Understandable.
The problem to me is because e.g C20 comes after C99 however 20<99.
There was a problem hiding this comment.
That is a very astute observation. It seems I'll need to change to interface to a defaultCxxStandardless = YY: ... function. Bummer.
There was a problem hiding this comment.
This is another reason why it's better to detect "is the compiler going to use a standard equal to or later than XYZ" like autoconf does -- no need to encode the entire list of standards in nix, along with their partial order.
tobim
requested review from
Artturin and
risicle
and removed request for
matthewbauer
ofborg
bot
added
10.rebuild-darwin: 0
|
This should fix some of the troubles we had with onnx, protobuf and abseil. |
|
btw, just for information, but the default c++ version on darwin is 11 |
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
There are no examples of defaultCStandard being used, so it should not be included.
Moreover, the C++ standard in use is not a property of the compiler. It is a property of the compiler as well as the flags passed to the compiler. This includes lots of things, like NIX_CFLAGS_COMPILE and overlays. Putting these values in passthru attributes will lead to incorrect results and headaches in the future.
If a codebase needs a particular version of the C++ standard, it can specify that need with -std=.
In the unlikely case that a codebase needs to know the standard used by the compiler without changing it, that can be detected by invoking the compiler. In fact, autoconf does precisely this. Detecting these values with compilation attempts is the right way to do this. It will pick up any extra flags added by NIX_CFLAGS_COMPILE or even ordinary CFLAGS environment variables.
Detect, don't define.
This whole issue looks like a CMake problem IMHO.
It looks like these project should be using this CMake feature. Try patching the cmake file; if it works, send it upstream. |
wegank
added
2.status: stale
4 participants
Description of changes
The variables
defaultCStandardanddefaultCXXStandardmake the defaults of the-stdoption known to Nix. This knowledge can be used to decide whether that option must be passed explictly for packages with specific standard requirements.Usage is demonstrated in
protobufandgrpc.This should not cause any rebuilds.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)