Add clang64 toolchain to pkgsCross#450796
Conversation
lib/systems/examples.nix
Outdated
|
|
||
| # LLVM-based mingw-w64 for ARM | ||
| # LLVM-based urct64s | ||
| clang64 = { |
There was a problem hiding this comment.
I feel like this needs a better name so people don't end up confusing it as being similar to pkgsLLVM.
There was a problem hiding this comment.
clang64 is the official name of the toolchain.
There was a problem hiding this comment.
I still think it's too unambiguously
There was a problem hiding this comment.
Clang 64 is a name that only makes since within the MSYS2 family of toolchains, where MinGW is a baseline assumption.
I would call it ucrt64-llvm.
There was a problem hiding this comment.
Good suggestion, or even msys2-clang64 is fine. Just something which indicates to people that it is a part of the MSYS2 family of toolchains or wouldn't confuse people who don't read documentation or comments.
There was a problem hiding this comment.
I don't like putting the toolchain between the libc and arch. I would prefer the more important things (libc and arch) go first, and the less important thing (toolchain) go second.
There was a problem hiding this comment.
Even the existing ucrt64 name I don't like, because it falsely assumes the 64 has an unambiguous interpretation as x86_64, when this is not the case, and the ucrtAarch64 option right below proves it!
I would like to see both names cleaned up, ideally.
There was a problem hiding this comment.
Could that be another PR, though? Just so I don't break shit.
There was a problem hiding this comment.
Sure. It can be a separate PR, For this PR, I just want to be clear that ucrt64 is not good precedent --- it's a bad name we probably should not have ever gone with.
There was a problem hiding this comment.
lib/systems/examples.nix has quite a bit of bad naming, I think it's best to not create more names which can cause confusion.
|
@RossComputerGuy looks good? Also, sorry for the pressure but when can I expect this on unstable? My work depends on this. |
|
Hey @umbrageodotus, if you need it urgently, I recommend that you maintain your own fork of Nixpkgs that holds it. We're often fast, but adding whole package sets needs a lot more discussion than a quick LGTM and ship it. |
|
@philiptaron Then I won't have caching - if that's what I wanted I could've just used an override afaik... |
|
This was done before with the introduction of ucrt64 and that was OK. This is that but with LLVM. |
|
What's the urgency? |
|
Using it to build rust code. The rust code needs -Clinker-plugin-lto which doesn't work with the gnu ld. |
|
OK, and why is this urgent? And why is this a nixpkgs wide urgency? |
|
It's not a nixpkgs wide urgency. That doesn't mean it shouldn't have been there SO much sooner. |
|
And it's urgent because of my job. |
|
It definitely does, to be honest, and you, my friend, need to lay off the gas. No one is going to add an entire new platform definition to nixpkgs overnight because one person's work requires it, and this change won't even achieve what you think it does to begin with. |
|
That's not a nixpkgs urgency |
|
I understand that and apologize. Talked with my boss and he was comprehensive about it. Carry on, no pressure anymore. |
|
Before the naming discussion devolves into fisticuffs, I would like to remind everyone, and OP in particular, that adding this alias will not, in fact, make Hydra build anything. |
|
@rosd hmm...? Howd I get hydra to build & cache it? |
|
You'd do that by adding it to |
|
Okay. |
|
@Ericson2314 all is left to do is fix the ci errors on this pr. |
|
@Eveeifyeve yes, but I'm waiting for Alyssa first. The important thing to do here is decide what we want, e.g. how much CI time to we want to devote to MinGW via release-cross.nix. once we decide, fixing the CI errors is trivial and very quick. |
|
I'm interested in what Alyssa thinks, too. |
alyssais
left a comment
There was a problem hiding this comment.
Makes sense to me. Happy to defer to the experts on naming.
| cross-mingw-msvcrt-x86_64 = mapTestOnCross systems.examples.mingw-msvcrt-x86_64 windowsCommon; | ||
| cross-mingw-ucrt-x86_64 = mapTestOnCross systems.examples.mingw-ucrt-x86_64 windowsCommon; | ||
| cross-mingw-ucrt-x86_64-llvm = mapTestOnCross systems.examples.mingw-ucrt-x86_64-llvm windowsCommon; | ||
| cross-mingw-ucrt-aarch64 = mapTestOnCross systems.examples.mingw-ucrt-aarch64 windowsCommon; |
There was a problem hiding this comment.
Do we really need all of these on Hydra?
There was a problem hiding this comment.
Yes it would have helped us on the journey of creating this pr and catching issues earlier eg. this pr. This is also will be required for building bash needed for the stdenv for windows migrating it to powershell = hundreds of thousands of packages not working.
There was a problem hiding this comment.
These are currently very small jobsets, so I am hoping that it should be fine.
If we ever cross-compile more stuff at scale, I might only do (UCRT-LLVM) * (x86_64, Aarch64). How does that sound?
(I am hoping we'll eventually do more native compilation at scale, and we fund-raise around this, and have more or fewer package sets accordingly.)
Co-authored-by: Eveeifyeve <88671402+Eveeifyeve@users.noreply.github.com>
|
Added the TODO. Anything else, anyone? |
|
I would say I want @alyssais to re-review this again. |
|
Successfully created backport PR for |
|
So I will be able to cross compile nix? |
|
Not any more than you were able before this. |
Added support for the clang64 toolchain which should be cached. Clang32 should also be added and
ucrtAarch64should be renamed to fit the scheme, but those are future PRs - as I need this in nixpkgs unstable URGENTLY.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.