llvmPackages_16: init #223282
llvmPackages_16: init #223282
Conversation
|
The babel commit is on staging, so a rebase should get it in |
There was a problem hiding this comment.
Hm, I need to check why.
This is still being worked on, you can see what we are doing in the Compilers Team matrix. We will get to this eventually, no worries. |
I fucked up my previous PR, but I will pull in the next time I try. |
|
Thanks for the PR @RaitoBezarius! :) IMO we should definitely cc the Would be great if we could get this into a mergable state ASAP to test
Definitely out of scope for this PR! :D
This confuses me. What previous PR? You should be able to just rebase and force push without having to open a new PR. PS: I like my old approach of having a dedicated commit to copy the files (e.g., llvmPackages_13: Copy from llvmPackages_git / #132643 / #162104) so that the other commits clearly show the relevant changes but IIRC that hasn't been done since then (and it also has its drawbacks so feel free to use a different approach). |
Let's try to do this :). With @alyssais and @rrbutani we are trying to reduce the differences between llvmPackages_15 and llvmPackages_git.
I created a previous PR which almost mass pinged a lot of people. :)
Got it. :) |
RaitoBezarius
force-pushed
the
llvmPackages_16
branch
from
b1b8939
to
b903ebb
Compare
RaitoBezarius
force-pushed
the
llvmPackages_16
branch
2 times, most recently
from
28c7966
to
54abe0f
Compare
ofborg
bot
requested review from
alyssais,
sternenseemann,
primeos,
lovek323,
Ericson2314 and
dtzWill
I think it's just because the diff --git a/pkgs/development/compilers/llvm/16/default.nix b/pkgs/development/compilers/llvm/16/default.nix
index 17074a37b2b..a7b6c18982e 100644
--- a/pkgs/development/compilers/llvm/16/default.nix
+++ b/pkgs/development/compilers/llvm/16/default.nix
@@ -102,10 +102,11 @@ in let
tools = lib.makeExtensible (tools: let
callPackage = newScope (tools // { inherit stdenv cmake ninja libxml2 python3 release_version version monorepoSrc buildLlvmTools; });
+ major = lib.versions.major release_version;
mkExtraBuildCommands0 = cc: ''
rsrc="$out/resource-root"
mkdir "$rsrc"
- ln -s "${cc.lib}/lib/clang/${release_version}/include" "$rsrc"
+ ln -s "${cc.lib}/lib/clang/${major}/include" "$rsrc"
echo "-resource-dir=$rsrc" >> $out/nix-support/cc-cflags
'';
mkExtraBuildCommands = cc: mkExtraBuildCommands0 cc + ''But I'm not sure whether that's actually the right solution. |
Yup, this is exactly right; this is a documented change in LLVM16. |
RaitoBezarius
force-pushed
the
llvmPackages_16
branch
from
54abe0f
to
2c627d9
Compare
|
Thank you everyone for your help, with this, LLVM16 is compiling fine on x86_64-linux. For aarch64-darwin, I see 2 test failures: |
|
@primeos With this, we have a working LLVM16 for x86_64-linux if you want to run tests for Chromium M113. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/2090 |
|
@RaitoBezarius thanks for the heads-up (and sorry for the delay - I was sick). I can built |
The idea was to have some common "helper" function to be involved once we finished ensuring that llvmPackages_15 and llvmPackages_git have almost zero delta in patches, etc. I think we are almost there for the delta in patches. llvmPackages_git was bumped to llvmPackages_15. |
Kept separate from 2c627d9 to skip CI for this trivial change.
Chromium M113 got released yesterday and contains important security fixes. |
Obviously, for security measures, I definitely understand (and that's why I left it this way.), we wanted to race you before. So, it's definitely okay IMHO. |
|
When trying to update emscripten to use LLVM 16 rather than a patched LLVM 15 (see #229718), the build fails with the following error (and succeeds on 15) on darwin-aarch64 at least: |
|
Based on emscripten-core/emscripten#18302, it may be that libc++ may have some kind of mismatch. Hopefully there is a way to get that to align for emscripten alone. |
|
I also hit this error today for a C codebase using atomics. The header that I expect to be included in my toolchain is located at |
|
@primeos @RaitoBezarius thanks for your efforts! Is this planned to be backported to 22.11 or do you aim for 23.05 already? |
|
@RaitoBezarius thanks for understanding :) And of course thanks for this awesome PR that saved me a lot of work! <3 I wanted to write another comment last week but it looks like I somehow forgot to... :o At least I already mentioned the hard deadline in #223282 (comment). But I also wanted to mention that it would be best to merge this as it is anyway since I need to backport LLVM 16 to 22.11 for Chromium (which would likely be way too risky with that common "helper" function). And merging this as it is shouldn't interfere with that common "helper" function as it basically only adds new files and we can simply delete all of https://hydra.nixos.org/eval/1794582?filter=llvmPackages_16&compare=1794561&full= looks really good (no failures - I only tested Chromium and expected that some LLVM packages might still fail) so I'll try to give backporting a try today (if someone else wants to handle it or help that's of course welcome as well - it should just be as simple as possible / a straightforward backport to get it merged ASAP).
Yes, I already mentioned it in #223282 (comment) ;) |
|
I had a bit of time just now so I already got around to preparing a backport: #230327 |
|
@nedwill |
|
@willcohen Thanks for letting me know. I was having some trouble getting one of the documentation tests to pass but was able to work around it with a derivation that skipped checking the tests. Now that this is merged the binary package the original derivation is working great for me. |
rrbutani
added
the
6.topic: llvm/clang
Description of changes
For aarch64-darwin: some old workarounds should be re-introduced.
For other archs: help me!
DO NOT MERGE : this is for review and further work only, as @rrbutani is working on introducing a common function between LLVM versions to reduce the duplication.
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/)