Skip to content

make rust-analyzer use a dedicated build directory #141839

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

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

Conversation

tshepang
Copy link
Member

inspired by #132794

@rustbot
Copy link
Collaborator

rustbot commented May 31, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2025
@tshepang tshepang changed the title make rust-analyzer settings use dedicated directory make rust-analyzer use a dedicated directory May 31, 2025
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label May 31, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Jun 1, 2025

I feel like this was discussed #general > ✔ Setup multiple editors for work on standard lib? @ 💬 but I don't remember why we used a top-level build-rust-analyzer/ as opposed to nesting inside of build/... @Zalathar do you happen to remember why? (It's been a while)

@jieyouxu
Copy link
Member

jieyouxu commented Jun 1, 2025

Oh I think it just happens to be an option that works.

@Zalathar
Copy link
Contributor

Zalathar commented Jun 1, 2025

It has unintuitive interactions with x clean and caching, for example. IIRC x clean won’t delete your download cache, but it will delete your nested RA download cache, because it’s just indiscriminately deleting a subdirectory it doesn’t know about.

If people choose to inflict those risks on themselves, fine, but I don’t think a nested RA build directory is a responsible default.

@jieyouxu
Copy link
Member

jieyouxu commented Jun 1, 2025

Oh right. I think that's the caveat I was vaguely recalling...

@tshepang
Copy link
Member Author

tshepang commented Jun 1, 2025

would it be good to get x clean not delete what it does not recognize

@mati865
Copy link
Member

mati865 commented Jun 2, 2025

Or even just avoid deleting build/rust-analyzer.

@Zalathar
Copy link
Contributor

Zalathar commented Jun 2, 2025

would it be good to get x clean not delete what it does not recognize

Bootstrap “owns” the build directory, so I think it's reasonable for x clean to delete anything it doesn't know about, on the assumption that it's junk left over from an earlier version of bootstrap.

Or even just avoid deleting build/rust-analyzer.

IIRC, one of the motivations people commonly cite for using build/rust-analyzer is that they want x clean to also clean their RA build directory. (And it does accomplish that, as a consequence of deleting build/rust-analyzer entirely.)

@Zalathar
Copy link
Contributor

Zalathar commented Jun 2, 2025

Anyhow, I strongly agree that our default IDE configs really should be using a separate build dir (e.g. build-rust-analyzer).

@tshepang
Copy link
Member Author

I recently found that x clean does not in fact remove build/rust-analyzer

@Mark-Simulacrum
Copy link
Member

Could you file a compiler MCP for this? I'd like to see some more eyes on this before we move in this direction.

@tshepang tshepang changed the title make rust-analyzer use a dedicated directory make rust-analyzer use a dedicated build directory Jun 23, 2025
@lolbinarycat
Copy link
Contributor

This won't actually work reliably due to a bug in rustic: emacs-rustic/rustic#105

@Mark-Simulacrum
Copy link
Member

That seems specific to emacs?

r=me on the change itself once MCP is approved.

@lolbinarycat
Copy link
Contributor

Ah, right, this will still improve things for other editors (this PR was linked to me as "changing the default for emacs"), so there's no point blocking it on emacs being fixed.

@tshepang
Copy link
Member Author

tshepang commented Jul 3, 2025

the MCP is now accepted and the pr updated to match

This avoids rust-analyzer having to wait for a build lock due to ./x
running other commands (and the other way around).
@tshepang
Copy link
Member Author

tshepang commented Jul 3, 2025

made the move to build-rust-analyzer/ more comprehensive... it did not cover rustfmt and procmacro settings

should be ready now @Mark-Simulacrum

@fee1-dead
Copy link
Member

I'm not sure this is actually the right approach. Doing so makes it so that when rust-analyzer runs x check and we try to run x test or something similar makes rustc run parallel. This will then just lead to a lot more resource usage and potentially freezes and other stuff.

A better approach might be to have some shim binary (or bootstrap option) that oversees rust-analyzer processes and aborts any work in progress when it detects an actual user-run command is waiting to acquire the lock.

@lolbinarycat
Copy link
Contributor

@fee1-dead I'm not sure that fixes the other issue this fixes, which is cache corruption, which was happening to me really often, causing changes to source code to not update the behavior of the final binary.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 13, 2025

📌 Commit fdd2352 has been approved by Mark-Simulacrum

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 Jul 13, 2025
@bors
Copy link
Collaborator

bors commented Jul 13, 2025

☔ The latest upstream changes (presumably #143888) made this pull request unmergeable. Please resolve the merge conflicts.

@bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

10 participants