llvmPackages.libc: init at 20.0.0-unstable-2024-12-08#363449
llvmPackages.libc: init at 20.0.0-unstable-2024-12-08#363449RossComputerGuy merged 3 commits intoNixOS:masterfrom
Conversation
github-actions
bot
added
the
6.topic: llvm/clang
nix-owners
bot
requested review from
Ericson2314,
alyssais,
dtzWill,
lovek323,
rrbutani and
sternenseemann
RossComputerGuy
force-pushed
the
feat/llvm-libc
branch
from
91c39aa to
44e5254
Compare
github-actions
bot
added
10.rebuild-darwin: 0
RossComputerGuy
force-pushed
the
feat/llvm-libc
branch
from
44e5254 to
c16e82d
Compare
RossComputerGuy
force-pushed
the
feat/llvm-libc
branch
from
c16e82d to
50de490
Compare
|
The non-full variant doesn't seem to be used? |
|
As of this time, the overlay build isn't a dependency for anything. |
|
What is "the overlay build"? I don't see it explained anywhere here. |
|
The overlay build is the non full build of LLVM libc. |
|
What's the point of it? |
|
I asked the LLVM libc team on their Discord server:
|
|
What does "kinda borked" mean? |
|
The full build has some problems still needing to be worked out. There's some odd C++ errors preventing compiling. Overlay builds work. |
|
So then why add two attributes, one that works and one that doesn't? Why not just one libc attribute that builds as much as we can? Is this actually useful for anything yet? If not, maybe we should just wait until we can at least build it properly. |
|
I was recommended to have both as an option since it'll allow for flexibility if things are broken. I'm going to be working on fixing the full build soon, just haven't gotten time. I've got some PR's merged upstream and some other might've flown under my radar so it could be working. |
|
For flexibility, wouldn't a single attribute with an option we could flip be better? Otherwise, we'd have to change everything that referred to the attribute all at once if we wanted to go from full to overlay. |
|
My thinking was to probably have 3, two are the actual builds and one is an alias. I think other parts of LLVM does this already in nixpkgs. |
|
But if the full build is working, why would we ever need the alias? Why does it need an attribute too? |
ofborg
bot
added
the
2.status: merge conflict
|
We already have to set a boolean anyways for full builds. The dependencies are different too. Providing both makes it easier to test if both work and for people to experiment with. |
|
If full is building, why would I want to test with overlay? |
|
I was meaning people who write software using nix could try it out. I know some people who would want to try it out. |
|
Yes, but is there a reason they'd specifically want to test out the non-full version, even if full is available and building? |
RossComputerGuy
force-pushed
the
feat/llvm-libc
branch
from
ec43413 to
f1645ab
Compare
github-actions
bot
added
10.rebuild-darwin: 1-10
ofborg
bot
added
8.has: package (new)
lib/systems/default.nix
Outdated
There was a problem hiding this comment.
OOC does it make sense to ever use LLVM libc with GCC?
There was a problem hiding this comment.
Hard to say based on whether GCC supports the naked attribute for specific targets. I was thinking the LLVM abi which means to use the libc that it'd make sense to use the entire toolchain. That's what the team upstream thinks as well.
There was a problem hiding this comment.
Certainly we'd want $arch-unknown-linux-llvm to use both an LLVM toolchain and libc, but I'm not sure about pkgsLLVMLibc — I think it's a bit surprising that it works differently from pkgsMusl and replaces the toolchain as well. Having it only change the libc would make it more composable (with #303849), and still allow getting to an LLVM toolchain+libc combination with pkgsLLVM.pkgsLLVMLibc.
There was a problem hiding this comment.
Yeah, I could drop the LLVM toolchain part once the composable PR lands. It won't work properly without it.
There was a problem hiding this comment.
Sounds good, except maybe then we should just wait for that PR to add this attr, so we don't make a breaking change to it later? It'll still be possible to use LLVM libc by calling Nixpkgs with the appropriate triple.
There was a problem hiding this comment.
I figured out pkgsLLVM.pkgsLLVMLibc actually doesn't require the composable PR so it should eval right.
RossComputerGuy
force-pushed
the
feat/llvm-libc
branch
from
f1645ab to
357295d
Compare
|
Eval failure |
RossComputerGuy
force-pushed
the
feat/llvm-libc
branch
from
357295d to
41b1402
Compare
github-actions
bot
removed
the
10.rebuild-darwin: 1
|
Eval should be fixed now |
wegank
added
12.approvals: 1
|
Will likely merge this tomorrow, libunwind and libcxx don't quite work but there's possible fixes. The LLVM libc monthly meeting is tomorrow so I'm hoping to figure out how we can get those building. |
|
This might have broken the r-ryantm-updates bot? nix-community/infra#1676 |
|
cc @infinisil shouldn't this have been caught by CI? |
|
I was thinking the same thing now that you mention it, I'm not sure why CI didn't catch it. |
|
Might be a case of nix-community/infra#1180 Edit: Nope |
| else if name == "relibc" then targetPackages.relibc or relibc | ||
| else if name == "llvm" then | ||
| # Use llvmPackages_git until LLVM 20 is the default. | ||
| targetPackages.llvmPackages_git.libc or llvmPackages_git.libc |
There was a problem hiding this comment.
I would have expected CI to catch this, but it seems to have broken in the transition away from OfBorg for evaluation. llvmPackages_git is defined in aliases.nix, with the intention that things in Nixpkgs should not be relying on it — it's only there for external users. This causes breakage when trying to do a full Nixpkgs evaluation (a context in which aliases basically have to be disabled or you get removed package throws etc.).
LLVM 20 is coming out soon, so I guess we can fix it then, but going forward until CI is fixed we need to try to remember that llvmPackages_git is not supposed to be used inside Nixpkgs.
6 participants
Things done
Adds LLVM libc, we've got the overlay building but full is still kinda borked. We're only going to support from >= LLVM 20 because it requires a lot of patching in previous versions. We're hoping the inclusion of LLVM libc at the point it is right now could help further its development and we can figure out how to package it better early on.
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.