Move the target libLLVM to llvm-tools-preview

[cuviper]

For running the compiler, we usually only need LLVM from $sysroot/lib,
which rustup will make available with LD_LIBRARY_PATH. We've also been
shipping LLVM in the $target/lib directory, which bloats the download
and installed size. The only times we do need the latter are for the
RPATH of llvm-tools-preview binaries, and for linking rustc-dev
libraries. We'll move it to the llvm-tools-preview component directly,
and rustc-dev will have an implicit dependency on it.

Here are the dist sizes that I got before and after this change:

llvm-tools-1.45.0-dev-x86_64-unknown-linux-gnu.tar.gz     1.3M   24M
llvm-tools-1.45.0-dev-x86_64-unknown-linux-gnu.tar.xz     748K   17M
rustc-1.45.0-dev-x86_64-unknown-linux-gnu.tar.gz          83M    61M
rustc-1.45.0-dev-x86_64-unknown-linux-gnu.tar.xz          56M    41M

The installed size should reduce by exactly one libLLVM.so (~70-80M),
unless you also install llvm-tools, and then it should be identical.

Resolves #70838.

[rust-highfive]

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

Thanks, this looks good to me. Can you check that the llvm tools (e.g. rust-lld) which I believe are dynamically linked to LLVM on Linux also continue to work with this?

Not sure what the best way to do that is though.

r=me presuming that works

[cuviper]

Thanks, this looks good to me. Can you check that the llvm tools (e.g. rust-lld) which I believe are dynamically linked to LLVM on Linux also continue to work with this?

I've giving that a conditional "yes". I tried compiling a wasm project, which implicitly uses rust-lld, and that works fine -- rustup already sets LD_LIBRARY_PATH for rustc itself, so libLLVM is easily found.

The conditional part is that I'm not sure how the other llvm-tools-preview are meant to be used. If you try to directly run them, they won't find LLVM, because they only have the embedded RPATH to $ORIGIN/../lib. But if you run rustup run $CHANNEL path/to/llvm-foo, it will work fine with LD_LIBRARY_PATH.

r=me presuming that works

I'll wait for your opinion on llvm-tools-preview.

[Mark-Simulacrum]

Hm, so I think that we should try to make them work - would it be feasible to set things up such that their RPATH works? I don't recall the details myself.

I think if we can, we should fix them up, but if it's a big hassle I wouldn't be opposed to this mild breakage so long as we don't break the most heavily used case of rustc running rust-lld, which is relied upon on stable I think.

[cuviper]

Ok, I'll explore cmake options to see if we can get a useful RPATH for our layout.

[cuviper]

I can't find a clean way to set RPATH just for the tools. There is CMAKE_INSTALL_RPATH, but we have to manually deal with if apple { "@loader_path/../../.." } else { "$ORIGIN/../../.." }, and that also affects libLLVM itself which we don't want.

What if I just add $target/lib/libLLVM-*.so to llvm-tools-preview, same as rustc-dev? It will make that dist bigger, but I think that's OK for a niche component.

[Mark-Simulacrum]

That's fine by me. We should make sure that installing both doesn't lead to errors though (we can run a try build and check rustup-toolchain-install-master and I can publish a nightly based on the try build to dev-static to test that).

[cuviper]

Ok, added. I'm surprised to see that llvm-tools-preview is included in the combined installer, so now that's back to the original size. At least rustup downloads separate installers, so that case will still benefit from the reduced rustc component.

@bors try

[bors]

⌛ Trying commit 86c29d6 with merge cf4df42b0cc4692d3601ba548c32d0018da14007...

[bors]

💔 Test failed - checks-actions

[cuviper]

OK, that looks like a real failure -- I'll investigate.

[cuviper]

@bors try

[bors]

⌛ Trying commit 90b2c8e with merge 149524b...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 149524b (149524b117be9a1577c242a5f9b1ffd2c89a3479)

[cuviper]

@Mark-Simulacrum so we have a try build -- what did you want to test with this?

[Mark-Simulacrum]

I've triggered a nightly build from this to be published to dev-static, which should do so in the next hour or so. Once that's done I'll likely r+ after checking that rustup install continues to work, both normally and with the added component.

[Mark-Simulacrum]

Tested out the built nightly (RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup toolchain install nightly), and we do indeed run into a problem:

root@44e56aad1805:/# rustup component add rustc-dev --toolchain nightly
info: downloading component 'rustc-dev'
 87.3 MiB /  87.3 MiB (100 %)  39.8 MiB/s in  3s ETA:  0s
info: installing component 'rustc-dev'
 87.3 MiB /  87.3 MiB (100 %)   9.1 MiB/s in  9s ETA:  0s
root@44e56aad1805:/# rustup component add llvm-tools-preview --toolchain nightly
info: downloading component 'llvm-tools-preview'
info: installing component 'llvm-tools-preview'
 18.0 MiB /  18.0 MiB (100 %)   9.6 MiB/s in  1s ETA:  0s
info: rolling back changes
error: failed to install component: 'llvm-tools-preview-x86_64-unknown-linux-gnu', detected conflict: '"lib/rustlib/x86_64-unknown-linux-gnu/lib/libLLVM-9-rust-1.45.0-nightly.so"'
root@44e56aad1805:/#

@kinnison -- I'm thinking that the right thing here would be for rustup to detect that both LLVM's are identical and not report an error, but I guess that's probably hard?

@cuviper I'm not sure what the best way to solve this is. We could temporarily say that the llvm-tools component must be installed together with the rustc-dev component to be usable without LD_LIBRARY_PATH changes, I guess... but that seems not nice.

[kinnison]

With the current model in rustup, trying to deduplicate files like that would be very uncomfortable to do, because we'd need to add reference counting to files etc to deal with component uninstall when files ended up shared. Sorry.

[cuviper]

Would it be possible for rustup to express dependencies? If so, I might put the library in llvm-tools alone and have rustc-dev depend on that.

[Mark-Simulacrum]

Ah, yes, I wasn't considering removal. I can see that being quite hard.

I would be fine with a dependency; if necessary we can also just make it a documented one. I wouldn't personally care too much about breaking rustc-dev consumers given an easy fix (but we shouldn't break llvm-tools-preview consumers if we can avoid it).

[kinnison]

To my knowledge there's no real dependency stuff in the channel manifests right now. I'm not sure if that'd be easier or harder to do in the current model than the other options :( For all that it's useful and clever, rustup is a very simple model under the hood :(

[Mark-Simulacrum]

@cuviper -- sounds like there's not nice support for the things we want in manifests / rustup, so we'll go with the manually installed "just documented" dependency. rustc-dev is nightly-only anyway these days, so I'm not too concerned with breakage -- its users are predominantly miri/clippy devs I imagine.

Can you update this PR to move the libLLVM over to just the llvm-tools-preview component?

If I'm remembering/understanding right that basically means:

• llvm-tools keep working as they have been
• if you want to link against [parts of] rustc-dev, you'll need to install llvm-tools-preview as well (manually)

[cuviper]

OK, lib/rustlib/$target/lib/libLLVM.so is only in llvm-tools now, and I've changed the title and original post to reflect this.

[Mark-Simulacrum]

cc @rust-lang/release as a heads up that we're making rustc-dev incomplete and llvm-tools complete with this patch, as it moves some components around. If people complain about missing libLLVM (probably a linker error) then please ask them to install llvm-tools-preview and see if that resolves their problem.

@bors r+

[bors]

📌 Commit 9c97b3c has been approved by Mark-Simulacrum

[RalfJung]

its users are predominantly miri/clippy devs I imagine.

Indeed so let's ping them so they are aware. :)
Cc @rust-lang/miri @rust-lang/clippy

[Mark-Simulacrum]

Ah, good call, thanks!

To be clear I suspect that neither clippy nor miri may actually depend on the libLLVM linked rustc parts, so you may get away with not installing. But should be easy to install so hopefully not a problem.

[RalfJung]

Miri behaves like normal rustc to compile dependencies in cargo miri, I suspect that will need LLVM.

[cuviper]

To be clear, developers of clippy and miri will probably care about this change, because when you link to compiler libraries, you'll inherit a -lLLVM that needs to be in the link library path, namely lib/rustlib/$target/lib/. Users will just get the compiler's runtime LLVM in the sysroot lib/.

[cuviper]

I just realized that we don't ship llvm-tools-preview on all of the same targets that have rustc-dev. It's just on those that have --enable-full-tools in CI -- which covers the main targets, but not all.

[Mark-Simulacrum]

I'm personally fine with that for now, we can revert (or expand the set of platforms) if people complain. But I'd not expect too much rustc-dev-using development outside the main targets anyway.

[bors]

⌛ Testing commit 9c97b3c with merge c60b675...

[bors]

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing c60b675 to master...

[RalfJung]

Indeed looks like getting Miri to work again is easy: rust-lang/miri#1427

[RalfJung]

This PR made rustc-dev not self-contained any more: #75247.