-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
llvm 5: split out compiler-rt and remove libcxxabi dep #42048
llvm 5: split out compiler-rt and remove libcxxabi dep #42048
Conversation
d23354c
to
417601c
Compare
buildInputs = stdenv.lib.optional stdenv.hostPlatform.isDarwin libcxxabi; | ||
|
||
configureFlags = [ | ||
"-DCOMPILER_RT_DEFAULT_TARGET_ONLY=ON" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why default target only, just curious?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO one of the big advantages of building compilers and their run-times separately is that you can libraries per-target. This makes it economical to support indefinite variations of targets (especially for e.g. embedded) without wasting time rebuilding the compiler itself.
ln -s "${targetLlvmLibraries.compiler-rt.out}/lib" "$rsrc/lib" | ||
echo "-resource-dir=$rsrc" >> $out/nix-support/cc-cflags | ||
'' + stdenv.lib.optionalString stdenv.targetPlatform.isLinux '' | ||
echo "--gcc-toolchain=${tools.clang-unwrapped.gcc}" >> $out/nix-support/cc-cflags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa when'd we start doing this?? haha guess I haven't been paying enough attention!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build-time equivalent was done before, polluting clang. i changed this to this when working on the first round of compiler-rt stuff with @Ralith.
inherit (llvmPackages_5) llvm; | ||
# The .override that was here before had the side affect of removing | ||
# the hacked-in "man" output. | ||
clang-unwrapped = builtins.removeAttrs llvmPackages_5.clang-unwrapped [ "man" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the manpages are in a separate drv now, so we can probably get rid of the extra output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this was causing much more building. Let's investigate this after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ericson2314 Looks like this breaks the nixpkgs tarball: https://hydra.nixos.org/build/76089043
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed on master with #42174 I'm pretty sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
ln -s "${targetLlvmLibraries.compiler-rt.out}/lib" "$rsrc/lib" | ||
echo "-resource-dir=$rsrc" >> $out/nix-support/cc-cflags | ||
'' + stdenv.lib.optionalString stdenv.targetPlatform.isLinux '' | ||
echo "--gcc-toolchain=${tools.clang-unwrapped.gcc}" >> $out/nix-support/cc-cflags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get rid of the gcc
attribute? It only works on linux and I think it's pretty confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh but there's some funky cc-wrapper, switch, and emscripten stuff about this too. Nasty all around, but this is big enough already.
"-DCOMPILER_RT_DEFAULT_TARGET_ONLY=ON" | ||
]; | ||
|
||
outputs = [ "dev" "out" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is out not the default here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. No reason. Good catch.
f42dab0
to
a6dafe1
Compare
We already did them on non-mass-rebuild llvm 6. Also, this allows simplifying the stdenv booting. We were missing the libcxxabi dep in compile-rt in llvm 6, so fixed that too.
a6dafe1
to
6e7e22d
Compare
Motivation for this change
We already did them on non-mass-rebuild llvm 6. Also, this allows simplifying the stdenv booting.
We were missing the libcxxabi dep in compile-rt in llvm 6, so fixed that too.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)CC @Ralith