-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
rubyPackages.iconv, v8: fix build with clang 16 #265307
Conversation
Fix incompatible function pointer conversion errors.
Setting to draft while I troubleshoot an issue I encountered while doing a rebuild. |
Marking ready after successfully rebuilding locally. |
This was taken from NixOS#264091 to use in the interim before that PR lands (sometime after the release of 23.11). It allows different versions of clang to link the same libc++, allowing dependencies to be linked when they are built with a different version of clang than the stdenv.
The version of v8 in nixpkgs fails to build due to `-Wenum-constexpr-conversion` errors. Since this will be made a hard error in later versions of clang, use clang 15 to build v8.
18b192d
to
2c4e2d8
Compare
@wegank I added the adapter and updated the PR to use it. |
in | ||
overrideCC stdenv (stdenv.cc.override { | ||
inherit libcxx; | ||
extraPackages = [ cxxabi pkgs.pkgsTargetTarget."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt ]; |
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.
Does pkgs.pkgsTargetTarget
work for you for any cross-cases? I tried pkgsLLVM.asciidoc-full
and it fails the eval as:
nix build --no-link -f. pkgsLLVM.asciidoc-full
error:
error: attribute 'llvmPackages_13' missing
at pkgs/stdenv/adapters.nix:86:32:
85| inherit libcxx;
86| extraPackages = [ cxxabi pkgs.pkgsTargetTarget."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt ];
| ^
87| });
It looks like pkgsTargetTarget
is always empty for cross-compilers. On the contrast the following seems to work:
--- a/pkgs/stdenv/adapters.nix
+++ b/pkgs/stdenv/adapters.nix
@@ -83,7 +83,10 @@ rec {
in
overrideCC stdenv (stdenv.cc.override {
inherit libcxx;
- extraPackages = [ cxxabi pkgs.pkgsTargetTarget."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt ];
+ extraPackages = [
+ cxxabi
+ pkgs.buildPackages.targetPackages."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt
+ ];
});
# Override the setup script of stdenv. Useful for testing new
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.
Proposed the change as #279463
Description of changes
Second batch of fixes for staging-next #263535.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)