spack
rust: switch to spack built llvm
Rust currently builds its own llvm within the rust build. This PR switches Rust to use an externally built LLVM from Spack, this reduces duplicate builds in environments with both llvm and Rust and reduces the overall installation size.
This PR notably changes the default variant values of LLVM to disable +clang by default since it's not needed by Rust and greatly increases the installation size and time to build. We should likely only install clang when dependencies / users explicitly ask for it.
In principle, I'm supportive of this. It's been a long, long time since I tried to make this work (think we actually did it for a bit) but it was far too unstable at the time. If this is now practical, and especially if other distros are doing it, I'd be happy to see it work this way.
Some comments on specifics though:
Why all the dylib variant renaming? We can probably do that, but it's going to require a deprecation period (it will break a lot of specs).
I'm also concerned with disabling clang by default. That will also break a lot of envs and specs that otherwise work today. If we want that effect, that should be part of splitting llvm back up into separate packages, with a clang package depending on libllvm and "llvm" probably being a bundle package depending on both.
If this is now practical, and especially if other distros are doing it, I'd be happy to see it work this way. I don't see many other distros building rust from source, but from my local testing over a couple different machines this has seemed pretty stable.
With the integrated LLVM build Rust is now using ~17GB of tmp space which maxes out most people's /tmp and I've gotten reports from multiple users that Rust is a pain for them to build. Using the separate LLVM I'm seeing at max 2-3GB of /tmp usage which I think will really help users.
Why all the dylib variant renaming? We can probably do that, but it's going to require a deprecation period (it will break a lot of specs).
dylib seemed to be an LLVM specific variant naming scheme which differs from the standard shared naming scheme in other packages. Now that we've got variant propagation it makes sense to unify these variants so that specifying +shared is propagated to dependencies as well when that's the intention of the user.
I'm also concerned with disabling clang by default. That will also break a lot of envs and specs that otherwise work today. If we want that effect, that should be part of splitting llvm back up into separate packages, with a clang package depending on libllvm and "llvm" probably being a bundle package depending on both.
I'd be game for splitting LLVM up into separate packages, although I didn't see how to build an installation of clang/flang on top of an existing llvm installation and Johannes Doerfert didn't think that functionality was particularly stable if it still exists.
I agree this could break existing environments when people upgrade to a newer version of the package without +clang although I think including +clang by default is a little more surprising from my perspective since it blows out the size of LLVM.
clang/flang on top of an existing llvm installation
clang has a CLANG_BUILT_STANDALONE CMake option that enables this feature, and it is set to true if the CMake source directory is set to the clang folder.
I've been looking into splitting up the LLVM package and adding other components, such as pstl and bolt. If functionality from #41700 is merged, I believe it would greatly simplify specifying all the dependency versions and conflicts and make breaking up of LLVM much easier.
It’s true that it increases size, but having an llvm with no frontend is a bit unfortunate, it’s much more likely to result in someone building two copies if they “spack install rust” then later “llvm+clang”.
I’ve had the chat with Johannes about the split builds as well, and was the person who originally re-merged the split packages in spack because they were unstable. The difference now is that several distros do a split build by default. Everything gentoo-based at least, Debian-based splits out libc++ and several other parts and I think fedora-based distros do too though I haven’t double checked recently. It’s definitely more stable to use the combined build, but if we have no solution for not having to rebuild an entire llvm to get clang, that’s a bit problematic.
Also, if it’s taking 17GB that means debug symbols are turned on. A full release build shouldn’t be more than ~4 unless configured with Debug or RelWithDebInfo. It might be highest value to figure out why the llvm debug symbols are on and turn them off as a step 1 in the rust package.
Get Outlook for iOShttps://aka.ms/o0ukef
From: Alec Scott @.> Sent: Tuesday, October 22, 2024 10:41:26 AM To: spack/spack @.> Cc: Scogland, Tom @.>; Review requested @.> Subject: Re: [spack/spack] rust: switch to spack built llvm (PR #47136)
If this is now practical, and especially if other distros are doing it, I'd be happy to see it work this way. I don't see many other distros building rust from source, but from my local testing over a couple different machines this has seemed pretty stable.
With the integrated LLVM build Rust is now using ~17GB of tmp space which maxes out most people's /tmp and I've gotten reports from multiple users that Rust is a pain for them to build. Using the separate LLVM I'm seeing at max 2-3GB of /tmp usage which I think will really help users.
Why all the dylib variant renaming? We can probably do that, but it's going to require a deprecation period (it will break a lot of specs).
dylib seemed to be an LLVM specific variant naming scheme which differs from the standard shared naming scheme in other packages. Now that we've got variant propagation it makes sense to unify these variants so that specifying +shared is propagated to dependencies as well when that's the intention of the user.
I'm also concerned with disabling clang by default. That will also break a lot of envs and specs that otherwise work today. If we want that effect, that should be part of splitting llvm back up into separate packages, with a clang package depending on libllvm and "llvm" probably being a bundle package depending on both.
I'd be game for splitting LLVM up into separate packages, although I didn't see how to build an installation of clang/flang on top of an existing llvm installation and Johannes Doerfert didn't think that functionality was particularly stable if it still exists.
I agree this could break existing environments when people upgrade to a newer version of the package without +clang although I think including +clang is a little more surprising from my perspective since it blows out the size of LLVM.
— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/spack/spack/pull/47136*issuecomment-2429880943__;Iw!!G2kpM7uM-TzIFchu!zgJ2lj12mYH6WYgWw1jBv1FRbankOmF9XO974bYeNPYHHUSdebXTRWltL9DMKU89MgG2Jcvluo3I_G0r_oaMsKewtu8$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AAFBFNLVJ3OTISMRRI5XDRTZ42E4NAVCNFSM6AAAAABQMSUDUGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRZHA4DAOJUGM__;!!G2kpM7uM-TzIFchu!zgJ2lj12mYH6WYgWw1jBv1FRbankOmF9XO974bYeNPYHHUSdebXTRWltL9DMKU89MgG2Jcvluo3I_G0r_oaMHCNWMyU$. You are receiving this because your review was requested.Message ID: @.***>
It’s true that it increases size, but having an llvm with no frontend is a bit unfortunate, it’s much more likely to result in someone building two copies if they “spack install rust” then later “llvm+clang”.
At the moment users are already building two versions of llvm one's just hiding in the rust build 😉. Pulling this out into multiple packages at least allows users to now only build one llvm when asking for both in an environment.
Also, if it’s taking 17GB that means debug symbols are turned on. A full release build shouldn’t be more than ~4 unless configured with Debug or RelWithDebInfo. It might be highest value to figure out why the llvm debug symbols are on and turn them off as a step 1 in the rust package.
I might need to check this, but I don't think it's the llvm build that's actually the major offender, rather LLVM plus stage0, stage1, and stage2 rust builds eat up a bunch of space. When we build as two separate packages, the temporary files of LLVM are thrown away by the time we get to rust.
Quite fair, and also we gain an important benefit combining the llvms, the ability to do cross-language LTO across all of c, c++ and rust. Like I said, all for that part as long as it’s reasonably stable.
The space issue could well be the rust stages too, I guess I triggered on the number. 17G is almost exactly the size of a relwithdebinfo llvm+clang of not long ago, made me think that a likely culprit.
Also makes me wonder, might it be worth it to add an option to build the cranelift backend rather than llvm? That should be both much smaller and much faster to build IIRC.
Get Outlook for iOShttps://aka.ms/o0ukef
From: Alec Scott @.> Sent: Tuesday, October 22, 2024 3:13:14 PM To: spack/spack @.> Cc: Scogland, Tom @.>; Review requested @.> Subject: Re: [spack/spack] rust: switch to spack built llvm (PR #47136)
It’s true that it increases size, but having an llvm with no frontend is a bit unfortunate, it’s much more likely to result in someone building two copies if they “spack install rust” then later “llvm+clang”.
At the moment users are already building two versions of llvm one's just hiding in the rust build 😉. Pulling this out into multiple packages at least allows users to now only build one llvm when asking for both in an environment.
Also, if it’s taking 17GB that means debug symbols are turned on. A full release build shouldn’t be more than ~4 unless configured with Debug or RelWithDebInfo. It might be highest value to figure out why the llvm debug symbols are on and turn them off as a step 1 in the rust package.
I might need to check this, but I don't think it's the llvm build that's actually the major offender, rather LLVM plus stage0, stage1, and stage2 rust builds eat up a bunch of space. When we build as two separate packages, the temporary files of LLVM are thrown away by the time we get to rust.
— Reply to this email directly, view it on GitHubhttps://urldefense.us/v3/__https://github.com/spack/spack/pull/47136*issuecomment-2430412822__;Iw!!G2kpM7uM-TzIFchu!yFzPHqvVLFIT5XXdfSiVN162m_cvoj2eU2ESGePPh0m0e6BZrVw5b8oHGeBXaWAQJgZMK4JLgXpUlo1wElfOj5TAnFo$, or unsubscribehttps://urldefense.us/v3/__https://github.com/notifications/unsubscribe-auth/AAFBFNN2WB5XFHJAMC4ELTDZ43EXVAVCNFSM6AAAAABQMSUDUGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZQGQYTEOBSGI__;!!G2kpM7uM-TzIFchu!yFzPHqvVLFIT5XXdfSiVN162m_cvoj2eU2ESGePPh0m0e6BZrVw5b8oHGeBXaWAQJgZMK4JLgXpUlo1wElfODZyIxWM$. You are receiving this because your review was requested.Message ID: @.***>
FWIW, fedora actually builds with the system llvm if the version is sufficient: https://src.fedoraproject.org/rpms/rust/blob/rawhide/f/rust.spec
gentoo has it as a use option: https://github.com/gentoo/gentoo/blob/master/dev-lang/rust/rust-1.82.0.ebuild
and debian does it by default: https://salsa.debian.org/rust-team/rust/-/blob/debian/sid/debian/rules
I'm totally ok with having this as an option, kinda ok with having it as the default with appropriate version checks. It's the other changes that concern me. Recent versions carry very few patches against upstream (to my surprise TBH).
I've rolled back a few of my previous changes after seeing the feedback here.
I still think it's worth enabling the build with the system LLVM instead of the integrated LLVM since the integrated LLVM is actively causing problems for users by using too much space in /tmp. (Though I'm not opposed to adding the integrated LLVM as a variant on rust.) I'm also interested in adding a cranelift backend, and maybe making that the default for users in future, but I'll plan on tackling that in a follow up PR.
I've re-enabled +clang by default in LLVM, although I think it might be better to model clang and flang on top of LLVM as a separate packages like rust in the future so that we can minimize installation sizes for users and maximize composability to reduce builds in CI.
Going to close this for now. I can't get newer Rust versions building in the same way as I had with this before.