Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebuild llvm spuriously less frequently #138333

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 11, 2025

I noticed that x check was rebuilding rustc_llvm basically every time I modified a source file. I tracked this down to the following env variable change:

cargo::core::compiler::fingerprint:     dirty: EnvVarChanged { name: "REAL_LIBRARY_PATH", old_value: Some("/home/jyn/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib"), new_value: None }

The problem was that I had installed rust-analyzer from rustup, not as a standalone tool. As a result, rustup sets
LD_LIBRARY_PATH=$(rustc --print target-libdir) in the environment under the assumption that rust-analyzer needs it to link to rustc_private crates. This is not in fact the case; RA does not link to rustc_private. But rustup does not know this. Ideally we would make rustup smarter, but that takes a while because rustup has infrequent releases.

In the meantime, as a workaround, be a little more selective about when we forward LD_LIBRARY_PATH. See the new comment for more details.

Note that there are still cases when we will spuriously rebuild; in particular when setting llvm-config = /path/to/llvm-config and using rust-analyzer installed from rustup. but both of those are individually rare, and the combination seems very unlikely to me.

@rustbot
Copy link
Collaborator

rustbot commented Mar 11, 2025

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 11, 2025
@rust-log-analyzer

This comment has been minimized.

I noticed that `x check` was rebuilding rustc_llvm basically every time I modified
a source file. I tracked this down to the following env variable change:
```
cargo::core::compiler::fingerprint:     dirty: EnvVarChanged { name: "REAL_LIBRARY_PATH", old_value: Some("/home/jyn/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib"), new_value: None }
```

The problem was that I had installed rust-analyzer from rustup, not as a
standalone tool. As a result, rustup sets
`LD_LIBRARY_PATH=$(rustc --print target-libdir)` in the environment
under the assumption that rust-analyzer needs it to link to
rustc_private crates. This is not in fact the case; RA does not link to
rustc_private. But rustup does not know this. Ideally we would make
rustup smarter, but that takes a while because rustup has infrequent
releases.

In the meantime, as a workaround, be a little more selective about when we forward
LD_LIBRARY_PATH. See the new comment for more details.
@jyn514 jyn514 force-pushed the spurious-llvm-rebuilds branch from a191eb0 to 2a8c87b Compare March 11, 2025 03:13
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 11, 2025

📌 Commit 2a8c87b has been approved by onur-ozkan

It is now in the queue for this repository.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
…nur-ozkan

Rebuild llvm spuriously less frequently

I noticed that `x check` was rebuilding rustc_llvm basically every time I modified a source file. I tracked this down to the following env variable change:
```
cargo::core::compiler::fingerprint:     dirty: EnvVarChanged { name: "REAL_LIBRARY_PATH", old_value: Some("/home/jyn/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib"), new_value: None }
```

The problem was that I had installed rust-analyzer from rustup, not as a standalone tool. As a result, rustup sets
`LD_LIBRARY_PATH=$(rustc --print target-libdir)` in the environment under the assumption that rust-analyzer needs it to link to rustc_private crates. This is not in fact the case; RA does not link to rustc_private. But rustup does not know this. Ideally we would make rustup smarter, but that takes a while because rustup has infrequent releases.

In the meantime, as a workaround, be a little more selective about when we forward LD_LIBRARY_PATH. See the new comment for more details.

Note that there are still cases when we will spuriously rebuild; in particular when setting `llvm-config = /path/to/llvm-config` and using rust-analyzer installed from rustup. but both of those are individually rare, and the combination seems very unlikely to me.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
…nur-ozkan

Rebuild llvm spuriously less frequently

I noticed that `x check` was rebuilding rustc_llvm basically every time I modified a source file. I tracked this down to the following env variable change:
```
cargo::core::compiler::fingerprint:     dirty: EnvVarChanged { name: "REAL_LIBRARY_PATH", old_value: Some("/home/jyn/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib"), new_value: None }
```

The problem was that I had installed rust-analyzer from rustup, not as a standalone tool. As a result, rustup sets
`LD_LIBRARY_PATH=$(rustc --print target-libdir)` in the environment under the assumption that rust-analyzer needs it to link to rustc_private crates. This is not in fact the case; RA does not link to rustc_private. But rustup does not know this. Ideally we would make rustup smarter, but that takes a while because rustup has infrequent releases.

In the meantime, as a workaround, be a little more selective about when we forward LD_LIBRARY_PATH. See the new comment for more details.

Note that there are still cases when we will spuriously rebuild; in particular when setting `llvm-config = /path/to/llvm-config` and using rust-analyzer installed from rustup. but both of those are individually rare, and the combination seems very unlikely to me.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants