Include `lld` in `rust-dev` package #91229

Aaron1011 · 2021-11-25T17:47:17Z

Fixes #88941

This will allow using download-ci-llvm while still having LLD
available.

rust-highfive · 2021-11-25T17:47:20Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

Aaron1011 · 2021-11-25T20:23:57Z

@bors try

bors · 2021-11-25T20:24:05Z

⌛ Trying commit 9afc013df9f25d16be3198d0d0271f0afcecb9b4 with merge e774ca25315475b9480cbb24892ba8aaf626f583...

bors · 2021-11-25T21:54:19Z

💔 Test failed - checks-actions

jyn514 · 2021-11-26T13:23:13Z

src/bootstrap/dist.rs

@@ -2082,6 +2082,7 @@ impl Step for RustDev {
            "llvm-bcanalyzer",
            "llvm-cov",
            "llvm-dwp",
+            "lld",


Hmm, I was going to suggest adding ensure(native::Lld) somewhere, but I don't see any ensure calls in this function. Maybe this step is assuming it's run from some other step? But it seems better to just call ensure anyway for all the tools it lists here.

I'm not very familiar with this code - I think it would be better to add in ensure calls in a separate PR.

Aaron1011 · 2021-11-26T16:36:17Z

@bors try @rust-timer queue

rust-timer · 2021-11-26T16:36:19Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-11-26T16:36:25Z

⌛ Trying commit 41b5b3808f251e856387093f354575aaa7eea59f with merge 668b61ea6d1ba3681ab71c90dd4a6d7ceb3ef175...

Aaron1011 · 2021-11-26T16:36:37Z

Oops, I didn't mean to queue the timer...

bors · 2021-11-26T18:09:54Z

☀️ Try build successful - checks-actions
Build commit: 668b61ea6d1ba3681ab71c90dd4a6d7ceb3ef175 (668b61ea6d1ba3681ab71c90dd4a6d7ceb3ef175)

rust-timer · 2021-11-26T18:09:56Z

Queued 668b61ea6d1ba3681ab71c90dd4a6d7ceb3ef175 with parent 454cc5f, future comparison URL.

rust-timer · 2021-11-26T22:34:01Z

Finished benchmarking commit (668b61ea6d1ba3681ab71c90dd4a6d7ceb3ef175): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Mark-Simulacrum · 2021-12-02T16:43:26Z

I suspect this will fail on CI builders that do build rust-dev but don't enable LLD (and so the file won't be present). We probably will want to optionally populate lld into this directory if it's available if that's the case.

In any case, I'm a little uncertain that this is the right path -- we actually already have rust-lld available in the downloaded packages for rustbuild, at least on x86_64-unknown-linux-gnu (build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/bin/rust-lld). I suspect that adding another copy is probably not the right path to making LLD more available if it's missing on other targets from a similar location...

(Edit: Sorry for a potentially misleading comment on the issue.)

Aaron1011 · 2021-12-04T21:01:42Z

@Mark-Simulacrum Isn't the downloaded stage0 rust-lld potentially different from the LLD we build from LLVM? If we've bumped the LLVM submodule sometime after performing a stage0 bump, then the stage0 lld will be built from a different LLVM source tree, right?

Mark-Simulacrum · 2021-12-04T21:26:45Z

That's true - do we really need a absolutely recent lld though? Generally speaking Rust users will not be using the most recent lld, so I'm not sure rustc compilation requires it.

I'm not strictly opposed to doing this, but I think it would be good to better understand the motivation for why we need an even more recent lld than beta.

Aaron1011 · 2021-12-04T21:33:52Z

The motivation is to have a set of working LLVM tools when using download-ci-llvm. When building a project that expects lld to be available, it would probably fine to use the slightly older stage0 lld. However, I think it would be better to make download-ci-llvm match the results of a local LLVM + lld build, instead of doing something slightly different.

Mark-Simulacrum · 2021-12-04T21:43:01Z

Hm. Yes, that seems true. It's worth noting that in my experience LLD is really quite fast to build, so it may be possible to just have rustbuild directly build it if it's asked for, against the downloaded LLVM artifacts. But that doesn't seem particularly better than just downloading a fresh copy, so it seems OK to package it up with the artifacts we provide in rust-dev, though (I think) nothing will currently reach for it directly, so it's only for manual usage?

Anyway, I'm fine with this PR (r=me) though as mentioned, I'd expect it to fail with missing LLD on platforms we're not currently building lld for -- so @bors rollup=iffy

Fixes rust-lang#88941 This will allow using `download-ci-llvm` while still having LLD available.

Aaron1011 · 2021-12-04T22:01:04Z

@Mark-Simulacrum I modified the PR to only add LLD to the tarball if it exists.

bjorn3 · 2021-12-04T22:03:20Z

It's worth noting that in my experience LLD is really quite fast to build, so it may be possible to just have rustbuild directly build it if it's asked for, against the downloaded LLVM artifacts.

That still requires cloning llvm-project, which takes a while and uses a lot of disk space.

Mark-Simulacrum · 2021-12-04T22:33:54Z

@Mark-Simulacrum I modified the PR to only add LLD to the tarball if it exists.

Hm, OK -- I guess I was expecting us to rather try to build it more frequently (by eagerly requesting via ensure(native::Lld ...) for example, or --enable-lld on ~all dist builders.

In particular if the goal is to ensure it's reliably available, the current PR will likely make it available for just

x86_64-unknown-linux-musl
x86_64-unknown-linux-gnu

as best as I can tell based on cursory CI inspection, which seems pretty limited.

But r=me if that seems OK to you.

Aaron1011 · 2021-12-04T22:50:22Z

Since the main motivation is local rustc development, I think it's fine to just support a few platforms for now, and add in others as the need arises.

@bors r=Mark-Simulacrum

bors · 2021-12-04T22:50:23Z

📌 Commit 7826c57 has been approved by Mark-Simulacrum

bors · 2021-12-05T06:47:01Z

⌛ Testing commit 7826c57 with merge 6c189bc...

bors · 2021-12-05T09:48:33Z

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 6c189bc to master...

rust-timer · 2021-12-05T11:19:09Z

Finished benchmarking commit (6c189bc): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

rust-highfive assigned Mark-Simulacrum Nov 25, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 25, 2021

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2021

jyn514 added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Nov 26, 2021

jyn514 reviewed Nov 26, 2021

View reviewed changes

Aaron1011 force-pushed the dist-lld branch from 9afc013 to 41b5b38 Compare November 26, 2021 16:35

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 26, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 26, 2021

Aaron1011 removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 27, 2021

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2021

Include lld in rust-dev package

7826c57

Fixes rust-lang#88941 This will allow using `download-ci-llvm` while still having LLD available.

Aaron1011 force-pushed the dist-lld branch from 41b5b38 to 7826c57 Compare December 4, 2021 22:00

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 5, 2021

bors merged commit 6c189bc into rust-lang:master Dec 5, 2021

rustbot added this to the 1.59.0 milestone Dec 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include `lld` in `rust-dev` package #91229

Include `lld` in `rust-dev` package #91229

Aaron1011 commented Nov 25, 2021

rust-highfive commented Nov 25, 2021

Aaron1011 commented Nov 25, 2021

bors commented Nov 25, 2021

This comment has been minimized.

bors commented Nov 25, 2021

jyn514 Nov 26, 2021

Aaron1011 Nov 27, 2021

Aaron1011 commented Nov 26, 2021

rust-timer commented Nov 26, 2021

bors commented Nov 26, 2021

Aaron1011 commented Nov 26, 2021

bors commented Nov 26, 2021

rust-timer commented Nov 26, 2021

rust-timer commented Nov 26, 2021

Mark-Simulacrum commented Dec 2, 2021 •

edited

Loading

Aaron1011 commented Dec 4, 2021

Mark-Simulacrum commented Dec 4, 2021

Aaron1011 commented Dec 4, 2021 •

edited

Loading

Mark-Simulacrum commented Dec 4, 2021

Aaron1011 commented Dec 4, 2021

bjorn3 commented Dec 4, 2021

Mark-Simulacrum commented Dec 4, 2021 •

edited

Loading

Aaron1011 commented Dec 4, 2021

bors commented Dec 4, 2021

bors commented Dec 5, 2021

bors commented Dec 5, 2021

rust-timer commented Dec 5, 2021

Include lld in rust-dev package #91229

Include lld in rust-dev package #91229

Conversation

Aaron1011 commented Nov 25, 2021

rust-highfive commented Nov 25, 2021

Aaron1011 commented Nov 25, 2021

bors commented Nov 25, 2021

This comment has been minimized.

bors commented Nov 25, 2021

jyn514 Nov 26, 2021

Choose a reason for hiding this comment

Aaron1011 Nov 27, 2021

Choose a reason for hiding this comment

Aaron1011 commented Nov 26, 2021

rust-timer commented Nov 26, 2021

bors commented Nov 26, 2021

Aaron1011 commented Nov 26, 2021

bors commented Nov 26, 2021

rust-timer commented Nov 26, 2021

rust-timer commented Nov 26, 2021

Mark-Simulacrum commented Dec 2, 2021 • edited Loading

Aaron1011 commented Dec 4, 2021

Mark-Simulacrum commented Dec 4, 2021

Aaron1011 commented Dec 4, 2021 • edited Loading

Mark-Simulacrum commented Dec 4, 2021

Aaron1011 commented Dec 4, 2021

bjorn3 commented Dec 4, 2021

Mark-Simulacrum commented Dec 4, 2021 • edited Loading

Aaron1011 commented Dec 4, 2021

bors commented Dec 4, 2021

bors commented Dec 5, 2021

bors commented Dec 5, 2021

rust-timer commented Dec 5, 2021

Include `lld` in `rust-dev` package #91229

Include `lld` in `rust-dev` package #91229

Mark-Simulacrum commented Dec 2, 2021 •

edited

Loading

Aaron1011 commented Dec 4, 2021 •

edited

Loading

Mark-Simulacrum commented Dec 4, 2021 •

edited

Loading