C++ compilers: Be sane with standard libraries #85189

Ericson2314 · 2020-04-14T02:25:05Z

Motivation for this change

This is long overdue, @LnL7 Time for you to say "I told you so!" :).

Things done

Ericson2314 · 2020-04-14T02:28:53Z

Seeing that this is such a massive rebuild, on I am all ears on how we can do more nix and less bash to make the inevitable follow-up work easier.

Maybe we could purely concat lists for all those *flags file and write once? The building detection the wrapper script does currently I am not too pleased with.

pkgs/development/compilers/llvm/7/default.nix

matthewbauer · 2020-04-14T03:04:17Z

pkgs/top-level/all-packages.nix

@@ -8181,14 +8180,6 @@ in
    stripped = false;
  }));

-  libstdcxxHook = makeSetupHook


We may need a migration or warning for this.

It may not be actively used though

aliases.nix should be enough.

I am not sure what should be aliased?

Not aliased but we need a throw statement in aliases.nix that generates a warning stating that this file has been removed.

Ah! sounds good.

pkgs/build-support/cc-wrapper/default.nix

matthewbauer · 2020-04-14T03:12:49Z

pkgs/build-support/cc-wrapper/default.nix

@@ -115,7 +109,7 @@ stdenv.mkDerivation {
    # Binutils, and Apple's "cctools"; "bintools" as an attempt to find an
    # unused middle-ground name that evokes both.
    inherit bintools;
-    inherit libc nativeTools nativeLibc nativePrefix isGNU isClang default_cxx_stdlib_compile;


This may be used outside by wrappers outside of nixpkgs. might be worth keeping, but just as meta-information.

It might be, but I think that's bad :)

There's no good way to use libraries that want these c flags without some meta-info like this. For instance bindgen in rust or anything that uses libclang. If we're going to be removing it, we should at least have a replacement.

see #26474 for why this was exported

Right I would rather have a more general way to get flags and just this one in particular. The default part (i.e. miy get overriden) is no good too.

matthewbauer · 2020-04-14T03:19:21Z

pkgs/applications/networking/browsers/firefox/common.nix

@@ -178,9 +178,9 @@ stdenv.mkDerivation ({

    BINDGEN_CFLAGS="$(< ${stdenv.cc}/nix-support/libc-cflags) \


Maybe we should just expose all of these flags in the cc-wrapper ? Something like stdenv.cc.cflags_compile and stdenv.cc.ld_flags

that is the pre-substituted values since we don't know the actual values at evaluation

It would be nice to uses more Nix so we did know the values. (e.g. have a nix boolean and then assert at run-time it was correct.)

I think that requires Nix-level setup-hooks to work correctly. I think that might be too ambitious for now.

I'm thinking something like /nix-support/get-cflags.sh which does all of this for you.

Well getting all the -I flags for other packaged requires that, but not just the std library stuff I think.

Right these should just need the basic c/c++ headers and library paths.

pkgs/os-specific/linux/firmware/fwupdate/default.nix

matthewbauer · 2020-04-14T20:05:02Z

I think we can also remove some of the old hacks like in https://github.com/NixOS/nixpkgs/pull/26709/files too

LnL7 · 2020-04-14T20:30:44Z

Does that change here? I would expect this to still only include it for clang++. Come to think of it, wouldn't CC=$CXX be a nicer fix for those cases or does that not work?

pkgs/build-support/cc-wrapper/cc-wrapper.sh

LnL7 · 2020-04-14T20:35:32Z

pkgs/build-support/cc-wrapper/default.nix

@@ -115,7 +109,7 @@ stdenv.mkDerivation {
    # Binutils, and Apple's "cctools"; "bintools" as an attempt to find an
    # unused middle-ground name that evokes both.
    inherit bintools;
-    inherit libc nativeTools nativeLibc nativePrefix isGNU isClang default_cxx_stdlib_compile;
+    inherit libc nativeTools nativeLibc nativePrefix isGNU isClang;


Would it make sense to expose the libc++ of the compiler, similar to libc?

Yeah, though null for libstdc++ is rather unfortunate. Oh well, all the more reason to get on splitting up the GCC derivation.

Perhaps for now cc.libcxx = gcc would make sense? Maybe that could also solve cc ? gcc.

I'm on the fence about this---I do want to get rid of cc.gcc but it's a bit disingenous unless we can somehow filter the other libraries out. Maybe we can leave that for a follow-up PR?

#91293 got rid of cc.gcc here.

pkgs/build-support/cc-wrapper/add-flags.sh

matthewbauer · 2020-04-14T21:53:15Z

Does that change here? I would expect this to still only include it for clang++. Come to think of it, wouldn't CC=$CXX be a nicer fix for those cases or does that not work?

Yeah, you're right - I was forgetting what the issue was there. We still need CXX to be used.

Ericson2314 · 2020-04-20T05:23:34Z

OK with the last push I have both stdenvs building, so now we can clean up in earnest.

Mic92 · 2020-04-20T09:13:39Z

We also have some tools that consume the c/c++ standard includes: cquery, clang-tools and ccls

Mic92 · 2020-06-22T06:59:58Z

We also have some tools that consume the c/c++ standard includes: cquery, clang-tools and ccls

As it turns out those are not affected by this change. I removed cquery last week. Clangd and CCLS only take the libcxx include path as input. I remember to use default_cxx_stdlib_compile for youcompletme at some point, but this was more of a hack and using something like ${llvmPackages.libcxx}/include/c++/v1 would be cleaner.

Ericson2314 · 2020-06-22T14:38:28Z

OK, it evals! I don't think we need any more back-compat stuff ATM, but happy to add things in requested.

NIX_CXXSTDLIB_COMPILE has been removed in NixOS#85189 and NixOS#85189 (comment) remommends using the files in ${clang}/nix-support to find the correct flags to pass to libclang.

ofborg bot added the 6.topic: vim label Apr 14, 2020

TravisWhitaker reviewed Apr 14, 2020

View reviewed changes

pkgs/development/compilers/llvm/7/default.nix Show resolved Hide resolved

Ericson2314 force-pushed the cxx-wrapper-debt branch 3 times, most recently from 25260c6 to 881d8cc Compare April 14, 2020 02:56

matthewbauer reviewed Apr 14, 2020

View reviewed changes

pkgs/build-support/cc-wrapper/default.nix Outdated Show resolved Hide resolved

matthewbauer reviewed Apr 14, 2020

View reviewed changes

pkgs/build-support/cc-wrapper/default.nix Outdated Show resolved Hide resolved

matthewbauer reviewed Apr 14, 2020

View reviewed changes

pkgs/os-specific/linux/firmware/fwupdate/default.nix Outdated Show resolved Hide resolved

Ericson2314 force-pushed the cxx-wrapper-debt branch 2 times, most recently from 9427d9c to de07048 Compare April 14, 2020 15:17

matthewbauer requested review from Mic92 and 7c6f434c April 14, 2020 19:22

LnL7 reviewed Apr 14, 2020

View reviewed changes

Ericson2314 mentioned this pull request Apr 17, 2020

treewide: Get rid of -dumpmachine in favor of static info #85468

Merged

10 tasks

Ericson2314 force-pushed the cxx-wrapper-debt branch 2 times, most recently from a7808ca to 995989a Compare April 19, 2020 01:25

ofborg bot added the 6.topic: stdenv Standard environment label Apr 19, 2020

Ericson2314 force-pushed the cxx-wrapper-debt branch from 995989a to 604b60d Compare April 19, 2020 04:01

Ericson2314 changed the title ~~C/C++ compilers: Be sane with standard libraries~~ C++ compilers: Be sane with standard libraries Apr 19, 2020

Ericson2314 force-pushed the cxx-wrapper-debt branch from 604b60d to a385085 Compare June 14, 2020 21:44

Ericson2314 changed the base branch from master to staging June 14, 2020 21:44

ofborg bot added the 8.has: module (update) label Jun 22, 2020

emscripten: Don't always clain to have a GCC

e00d4d6

Ericson2314 force-pushed the cxx-wrapper-debt branch 2 times, most recently from eba5f1f to 9e62a88 Compare June 22, 2020 04:21

Ericson2314 marked this pull request as ready for review June 22, 2020 04:22

Ericson2314 requested a review from jonringer as a code owner June 22, 2020 04:22

C++ Compilers: Systematize handling of standard libraries

f3f7612

Ericson2314 force-pushed the cxx-wrapper-debt branch from 9e62a88 to f3f7612 Compare June 22, 2020 04:24

Ericson2314 merged commit fa54dd3 into NixOS:staging Jun 22, 2020

Ericson2314 deleted the cxx-wrapper-debt branch June 22, 2020 14:38

vcunat mentioned this pull request Jun 29, 2020

nss: 3.52.1 -> 3.54 #91746

Merged

4 tasks

alyssais mentioned this pull request Jun 30, 2020

rustc: 1.43.0 -> 1.44.1 #89576

Merged

10 tasks

DieGoldeneEnte mentioned this pull request Jul 2, 2020

[wait for staging] remove libstdcxxHook, since it isn't needed anymore nixos-rocm/nixos-rocm#45

Merged

symphorien mentioned this pull request Feb 3, 2022

rust-bindgen: fix finding c++ stdlib #158041

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C++ compilers: Be sane with standard libraries #85189

C++ compilers: Be sane with standard libraries #85189

Ericson2314 commented Apr 14, 2020 •

edited

Ericson2314 commented Apr 14, 2020

matthewbauer Apr 14, 2020

Mic92 Apr 15, 2020

Ericson2314 Jun 14, 2020

Mic92 Jun 15, 2020 •

edited

Ericson2314 Jun 21, 2020

matthewbauer Apr 14, 2020

Ericson2314 Apr 14, 2020

matthewbauer Apr 14, 2020 •

edited

matthewbauer Apr 14, 2020

Ericson2314 Apr 14, 2020

matthewbauer Apr 14, 2020

matthewbauer Apr 14, 2020 •

edited

Ericson2314 Apr 14, 2020

matthewbauer Apr 14, 2020

matthewbauer Apr 14, 2020

Ericson2314 Apr 14, 2020

matthewbauer Apr 14, 2020

matthewbauer commented Apr 14, 2020

LnL7 commented Apr 14, 2020

LnL7 Apr 14, 2020

Ericson2314 Apr 15, 2020

LnL7 Apr 15, 2020 •

edited

Ericson2314 Apr 20, 2020

Ericson2314 Jun 22, 2020

matthewbauer commented Apr 14, 2020

Ericson2314 commented Apr 20, 2020

Mic92 commented Apr 20, 2020

Mic92 commented Jun 22, 2020

Ericson2314 commented Jun 22, 2020

		@@ -178,9 +178,9 @@ stdenv.mkDerivation ({

		BINDGEN_CFLAGS="$(< ${stdenv.cc}/nix-support/libc-cflags) \

C++ compilers: Be sane with standard libraries #85189

C++ compilers: Be sane with standard libraries #85189

Conversation

Ericson2314 commented Apr 14, 2020 • edited

Motivation for this change

Things done

Ericson2314 commented Apr 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Jun 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer Apr 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer Apr 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer commented Apr 14, 2020

LnL7 commented Apr 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LnL7 Apr 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewbauer commented Apr 14, 2020

Ericson2314 commented Apr 20, 2020

Mic92 commented Apr 20, 2020

Mic92 commented Jun 22, 2020

Ericson2314 commented Jun 22, 2020

Ericson2314 commented Apr 14, 2020 •

edited

Mic92 Jun 15, 2020 •

edited

matthewbauer Apr 14, 2020 •

edited

matthewbauer Apr 14, 2020 •

edited

LnL7 Apr 15, 2020 •

edited