NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.06k stars 14.1k forks source link

llvmPackages stdenv broken on master and unstable, can't find C++ headers? #28223

Closed dtzWill closed 7 years ago

dtzWill commented 7 years ago

Issue description

The clang stdenv (e.g., llvmPackages_4.stdenv) on recent unstable channel no longer seems able to find the C++ headers when used as the stdenv for a build.

Steps to reproduce

$ NIX_PATH=unstable=https://d3g5gsiof5omrk.cloudfront.net/nixos/unstable/nixos-17.09pre112691.abdb58e407/nixexprs.tar.xz nix-build -E "with import <unstable> {}; lzip.override { stdenv = llvmPackages_4.stdenv; }"

yields

...
build flags: SHELL=/nix/store/8v2glga3sg78wj5fmxqvf63vdxrjgy6h-bash-4.4-p12/bin/bash
c++ -DNDEBUG -O3 -c -o arg_parser.o arg_parser.cc
arg_parser.cc:20:10: fatal error: 'cstring' file not found
#include <cstring>
         ^~~~~~~~~
1 error generated.
make: *** [Makefile:50: arg_parser.o] Error 1

Curiously the libcxxStdenv does work:

NIX_PATH=unstable=https://d3g5gsiof5omrk.cloudfront.net/nixos/unstable/nixos-17.09pre112691.abdb58e407/nixexprs.tar.xz nix-build -E "with import <unstable> {}; lzip.override { stdenv = llvmPackages_4.libcxxStdenv; }"

and just for sanity, it does work on 17.03 and usually works otherwise O:)

NIX_PATH=nixpkgs=https://nixos.org/channels/nixos-17.03/nixexprs.tar.xz nix-build -E "with import <nixpkgs> {}; lzip.override { inherit (llvmPackages_4) stdenv; }"

Technical details

copumpkin commented 7 years ago

cc @Ericson2314 and @LnL7, since they were tracking down a similar problem in the Darwin clang stdenv recently.

LnL7 commented 7 years ago

This only happens on linux, it looks like the -isystem flags from gcc used to leak into the clang stdenv somehow.

--- release-17.03   2017-08-13 12:48:00.000000000 +0200
+++ master      2017-08-13 12:48:07.000000000 +0200
@@ -1,22 +1,17 @@
 original flags to /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-4.0.0/bin/clang++:
   -o
   foo
   /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-hello.cc
-extraAfter flags to /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-4.0.0/bin/clang++:
+extra flags after to /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-4.0.0/bin/clang++:
   -B/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-4.0.0/lib
   -B/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-glibc-2.25/lib/
   -idirafter
   /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-glibc-2.25-dev/include
   -idirafter
-  /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-4.0.0/lib/gcc/*/*/include-fixed
-  -B/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-wrapper-4.0.0/bin/
-  -isystem
-  /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-gcc-5.4.0/include/c++/5.4.0
-  -isystem
-  /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-gcc-5.4.0/include/c++/5.4.0/x86_64-unknown-linux-gnu
+  -B/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-clang-wrapper-4.0.0/bin/
   -O2
   -D_FORTIFY_SOURCE=2
   -fstack-protector-strong
LnL7 commented 7 years ago

This works, we could fix the clang stdenv to include those by default. Unless there's a reason to use the gcc headers, but using libcxx makes more sense I would think.

(lzip.override { stdenv = llvmPackages_4.stdenv; }).overrideDerivation (drv: {
  buildInputs = drv.buildInputs ++ [ llvmPackages_4.libcxx llvmPackages_4.libcxxabi ];
})
globin commented 7 years ago

cc @Ericson2314

dtzWill commented 7 years ago

(lzip.override { stdenv = llvmPackages_4.stdenv; }).overrideDerivation (drv: { buildInputs = drv.buildInputs ++ [ llvmPackages_4.libcxx llvmPackages_4.libcxxabi ]; })

Is this significantly different than using libcxxStdenv?

IMO it's an important use case, using clang w/gcc's libstdc++, it's the most "compatible" with Linux things and is the default way to use clang (for now, anyway).

Ericson2314 commented 7 years ago

This have something to do with default_cxx_stdlib_compile?

LnL7 commented 7 years ago

From the name I wouldn't expect llvmPackages.stdenv to still use the gcc libstc++, but the use case makes sense. The condition for default_cxx_stdlib_compile looks good, not sure why it's not working.

Ericson2314 commented 7 years ago

@LnL7 noticed that the logic would only apply if that variable was undefined, not empty. But add-flags.sh now ensures that it is defined (and empty if not otherwise defined). The best solution would be to handle the default in add-flags so there's no empty fallback at any point.

Ericson2314 commented 7 years ago

I'll do this for 17.09

teh commented 7 years ago

python27Packages.vowpalwabbit appears to be a victim of this issue as well

Mic92 commented 7 years ago

NIX_@infixSalt@_CXXSTDLIB_COMPILE seems to be defined before. I currently test the following patch:

 if [[ "$isCpp" = 1 ]]; then
     if [[ "$cppInclude" = 1 ]]; then
-        NIX_@infixSalt@_CFLAGS_COMPILE+=" ${NIX_@infixSalt@_CXXSTDLIB_COMPILE-@default_cxx_stdlib_compile@}"
+        NIX_@infixSalt@_CFLAGS_COMPILE+=" @default_cxx_stdlib_compile@"
     fi
     NIX_@infixSalt@_CFLAGS_LINK+=" $NIX_@infixSalt@_CXXSTDLIB_LINK"
 fi

this is not a good fix though, but if this the root cause, I can come up with a better patch.

LnL7 commented 7 years ago

@Mic92 That would break llvmPackages.libcxxStdenv or make it behave incorrectly.

Mic92 commented 7 years ago

@LnL7 i know

Zitrax commented 6 years ago

Is this supposed to work now? At least I am having the same problem on unstable(18.03); g++ works but clang fails and does not find the headers. I might have missed something though, rather new to nix.

sifmelcara commented 6 years ago

@Zitrax Maybe you need clang++ instead of clang? Edit: nevermind, I can also reproduce your issue

sifmelcara commented 6 years ago

After some testing, I found that using clang++ without nix-shell will cause the c++ header not found error. (I guess this is not a bug?) To successfully compile a simple .cpp file, I need nix-shell -p clang --run "clang++ *.cpp"

LnL7 commented 6 years ago

@Zitrax See https://github.com/NixOS/nixpkgs/issues/30670#issuecomment-360300282.

Zitrax commented 6 years ago

Indeed it works in nix-shell, thanks.

copumpkin commented 6 years ago

There's probably a bigger issue here of people wanting our tools to work normally outside of Nix builds and shells, but that's a much bigger endeavor.

Ericson2314 commented 6 years ago

Yeah we'd need like a clangWithPackages, which would easiest to accomplish if we did less dependency stuff in bash.