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

rust-analyzer.check.workspace = false is ignored on server startup #18141

Closed
BaxHugh opened this issue Sep 18, 2024 · 36 comments · Fixed by #18738
Closed

rust-analyzer.check.workspace = false is ignored on server startup #18141

BaxHugh opened this issue Sep 18, 2024 · 36 comments · Fixed by #18738
Labels
C-bug Category: bug

Comments

@BaxHugh
Copy link

BaxHugh commented Sep 18, 2024

"When rust-analyzer.check.workspace" : false is set I expect that cargo check should only be run on the rust crate which I have open in vs code. However, rust-analyzer runs cargo check across all workspace members [[edit] when the server is first started. After which saving a file shows that check is only run on the crate which is opened.]
This is a problem as I am working in a mono repo which has many member crates: when I'm working in one, where VS Code is only open at the member crate's manifest directory, the rust-analyzer is slow [[edit] to start] as it is unnecessarily checking all the workspace crates.

I have reproduced this problem in a simple toy workspace.

rust-analyzer version: (eg. output of "rust-analyzer: Show RA Version" command, accessible in VSCode via Ctrl/⌘+Shift+P)
rust-analyzer 0.3.2112-standalone

rustc version: (eg. output of rustc -V)
rustc 1.77.0 (aedd173a2 2024-03-17)

editor or extension: (eg. VSCode, Vim, Emacs, etc. For VSCode users, specify your extension version; for users of other editors, provide the distribution if applicable)
VSCode version 1.93.1 38c31bc77e0dd6ae88a4e9cc93428cc27a56ba40 x64
rust-analyzer extension: v0.3.2112

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME)
in .vscode/settings.json for the workspace (user level settings.json file removed)
"rust-analyzer.check.workspace": false,

code example to reproduce:
To reproduce, please see the zipped demo project: ra-workspace-issue-demo.zip

This has the structure

./
├── bar
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
├── Cargo.lock
├── Cargo.toml
└── foo
    ├── Cargo.toml
    └── src
        └── lib.rs

i.e. a Cargo workspace with two workspace.member crates which do not depend on each other.
In both lib.rs files, I have called a non existent function such that cargo check reports an error.

If I open, say, the foo crate: code ./foo in the problems panel, I can clearly see that rust-analyzer has reported problems in the foo crate which is not open in VS Code. (similarly I can confirm by introducing a build.rs file with panics, and the rust-analyzer output shows the error when it has built the crate it should not be running check on).

Image
Screenshot showing that problems from both the foo and bar crate are being reported, despite only foo being open

[edit]
Image
Screenshot showing that after a save, only problems from the open crate are reported

@BaxHugh BaxHugh added the C-bug Category: bug label Sep 18, 2024
@BaxHugh BaxHugh changed the title rust-analyzer.check.workspace = false does not work rust-analyzer.check.workspace = false does not work in VS Code Sep 18, 2024
@ChayimFriedman2
Copy link
Contributor

I cannot reproduce. Initially rust-analyzer indeed checks all packages, but when editing and saving it checks only one.

@BaxHugh
Copy link
Author

BaxHugh commented Sep 18, 2024

Thanks for the feedback @ChayimFriedman2. When you say initially rust-analyzer checks all packages. Do you agree that, that is not the desired behaviour?
check.workspace = false specifies that the --workspace flag is not passed, but instead -p=crate is. If the -p argument is passed all along, then cargo check should never be run on anything but the single crate which is opened. That is how I understand it.

@BaxHugh
Copy link
Author

BaxHugh commented Sep 19, 2024

@ChayimFriedman2 I can confirm that I see the same behaviour as you. The screenshot showing both problems is after first restarting the RA server. Once I save a file, the problems listed reduce to just the crate that is open.

I'll update the original post to reflect this.

However, RA checking the whole workspace on startup is still a problem for me, as there are a lot of crates and deps in the workspace, and starting the server takes a long time, after opening the project in VSCode.

@BaxHugh BaxHugh changed the title rust-analyzer.check.workspace = false does not work in VS Code rust-analyzer.check.workspace = false is ignored on server startup Sep 19, 2024
@alibektas
Copy link
Member

I would recommend using rust-analyzer.linkedProjects

@afroozeh
Copy link

Recently, I'm also observing issues with rust-analyzer.check.workspace" : false. It just checks the current crate and all the dependent crates on it. In very deep crates, it takes quite some time to check things on each save. Before, it was just changing the current crate. Has something changed recently in the behavior of this setting?

@alibektas
Copy link
Member

@afroozeh Yes, we did it exactly like you describe. See #17912

In very deep crates, it takes quite some time to check things on each save

If this wasn't the case before, then it indicates that we did something wrong.

@afroozeh
Copy link

afroozeh commented Oct 10, 2024

@alibektas I can confirm that it was not the case before, editing/save in that deep crate was immediate when rust-analyzer.check.workspace was false. Now, it seems like true or false value is meaningless because you're just checking all the dependent crates. And it has really degraded my developer experience using RustRover RustAnalyzer: each save takes about 5-10 seconds... Before I was invoking cargo check for the whole workspace on demand. I think, it would be good to have the option with these values instead of just true and false: "all", "dependent_crates", "current_crate"

Update: RustRover was a typo here, I meant RustAnalyzer.

@alibektas
Copy link
Member

You may want to make sure that you have your rust-analyzer up-to-date. There was previously an issue with which crates we ran our flycheck on but #18197 hopefully resolved. If your rust-analyzer is indeed a more recent version, I will check if there are other cases where this issue persists but I highly doubt that.

@afroozeh
Copy link

@alibektas my RustAnalyzer is up to date. if you're now running cargo check on the current crate and all dependent crates, it's going to be slow for live editing for very deep crates in the dependency graph. I want an option that runs cargo check just for a single crate, i.e., cargo check -p <current_crate>. Again to double check, Is this the case when rust-analyzer.check.workspace is false? Or false means check the current crate and all dependent crates?

@alibektas
Copy link
Member

alibektas commented Oct 10, 2024

@alibektas my RustAnalyzer is up to date. if you're now running cargo check on the current crate and all dependent crates, it's going to be slow for live editing for very deep crates in the dependency graph. I want an option that runs cargo check just for a single crate, i.e., cargo check -p <current_crate>.

Let me get back to you with this in a few days. I am rewriting most of the cargo check logic.

Again to double check, Is this the case when rust-analyzer.check.workspace is false? Or false means check the current crate and all dependent crates?

true : Add --workspace to cargo check
false : Add -p <package_name>

And pardon my ignorance but I thought that RustRover uses IntellijRust or is it now possible to use both of them?

@davidbarsky
Copy link
Contributor

And pardon my ignorance but I thought that RustRover uses IntellijRust or is it now possible to use both of them?

Not without some active work: while it is possible to configure a classic, non-Fleet JetBrains IDE to use an arbitrary LSP server, I'm not sure why folks would want to point RustRover at rust-analyzer.

@afroozeh
Copy link

afroozeh commented Oct 11, 2024

I'm sorry, I meant RustAnalyzer, there is no RustRover, it was a typo 😅 (I updated the comment to avoid confusing for other readers)

@afroozeh
Copy link

Let me get back to you with this in a few days. I am rewriting most of the cargo check logic.

alibektas Thank you. Please also make the semantic of the config also clear. As I said, it seems like when the flag is false, it checks the current crate and all dependent ones.

@afroozeh
Copy link

afroozeh commented Nov 6, 2024

@alibektas is there any update on this issue?

@alibektas
Copy link
Member

@afroozeh hey sorry for the late reply. Honestly, I totally forgot that I wanted to resolve this issue but now the team lead is away and I definitely want to check with him first before I do it because something doesn't seem right. Sorry for the inconvenience! I made a separate issue you can check it out the updates

@TheColorRed
Copy link

TheColorRed commented Nov 26, 2024

I am not sure if this is related, but I have the following in my settings.json:

{
  "rust-analyzer.check.workspace": false
}

When I save a file, in a large project (e.g. packages/package-a/main.rs), it takes a few minutes to run the check.

However, if I run the following in a terminal for that same project, it takes one second or less:

cargo check -p package-a

@ChayimFriedman2
Copy link
Contributor

Did you run the commands one after the other?

@TheColorRed
Copy link

TheColorRed commented Nov 26, 2024

I did it in different tests:

  1. Save same file two times in a row (both times ran 1+ minutes)
  2. Run save (ran 1+ minutes) then run in terminal (ran under 1 second)
  3. Run save in the terminal two times in a row (both ran under 1 second)

So, for me, no matter how many times I run it via file save it takes lots of time as if it is running caro check --workspace instead of cargo check -p ...

@afroozeh
Copy link

The current "rust-analyzer.check.workspace": false is broken in the sense that it does not do cargo check -p. To me it seems like it checks the current package and all dependent ones, and that's why it takes a long time in a large project.
This was working fine a couple of months ago, so something has changed. @alibektas Is there any plan to fix this? You created a new issue but that seems to be just there without any activity.

@alibektas
Copy link
Member

alibektas commented Nov 26, 2024

I am waiting for the project lead to be online again which will happen either tomorrow or the day after. After that it will be resolved in no time. Once again terribly sorry for the inconvenience.

@ChayimFriedman2
Copy link
Contributor

Ah it's probably #18197, you don't have a target set (like the default). @alibektas I get a headache every time I try to think if we should have && or ||, but are you sure && is the correct call?

@alibektas
Copy link
Member

Ah it's probably #18197, you don't have a target set (like the default). @alibektas I get a headache every time I try to think if we should have && or ||, but are you sure && is the correct call?

I was sure back then can't say I am sure now. Let me have a look again.

@afroozeh
Copy link

Thank you! appreciate your work on this project!

Ah it's probably #18197, you don't have a target set (like the default).

How can I set a target and check if it works with that?

@ChayimFriedman2
Copy link
Contributor

@afroozeh Save a bin file (or test (integration)/example/bench).

@afroozeh
Copy link

They are all [lib] targets already, so not sure if that's related.

@ChayimFriedman2
Copy link
Contributor

@afroozeh Yeah, [lib] is where this doesn't work. Add a [bin] target and save it.

@ChayimFriedman2
Copy link
Contributor

Given that this is not the same problem as OP's, I opened a new issue about this: #18562, please use it for discussions.

@afroozeh
Copy link

@ChayimFriedman2 I can't just add [bin] there... it's a large project with a lot of inner lib crates that depend on each other. I guess you already know what's the problem. It should not check the dependent crates, or you should introduce a separate config to just check the current crate.

@ChayimFriedman2
Copy link
Contributor

@afroozeh Please discuss this in the new issue.

@mhnap
Copy link

mhnap commented Dec 20, 2024

The current "rust-analyzer.check.workspace": false is broken in the sense that it does not do cargo check -p. To me it seems like it checks the current package and all dependent ones, and that's why it takes a long time in a large project. This was working fine a couple of months ago, so something has changed. @alibektas Is there any plan to fix this? You created a new issue but that seems to be just there without any activity.

I still have the same issue on v0.3.2220. @afroozeh @TheColorRed Maybe someone also tried to update to the latest release and check this again?

@afroozeh
Copy link

afroozeh commented Dec 20, 2024

@mhnap I'm on the latest version, and still the same behavior, checking all dependent crates. Each file save takes 16-25 seconds for me...

@mhnap
Copy link

mhnap commented Dec 20, 2024

Recently, I'm also observing issues with rust-analyzer.check.workspace" : false. It just checks the current crate and all the dependent crates on it. In very deep crates, it takes quite some time to check things on each save. Before, it was just changing the current crate. Has something changed recently in the behavior of this setting?

@afroozeh Do you remember the version you used when it was working as expected?

@afroozeh
Copy link

@mhnap Not exactly, but it was some times in September 2024...

@mhnap
Copy link

mhnap commented Dec 20, 2024

Not exactly, but it was some times in September 2024...

Hmm, I've switched 4 months back, but I still have the same problem (it checks the current crate and all the dependent crates on it). Or maybe I have wrong config 🤔

	"rust-analyzer.cargo.allTargets": false,
	"rust-analyzer.check.allTargets": false,
	"rust-analyzer.check.workspace": false,
	"rust-analyzer.checkOnSave": false,

@afroozeh
Copy link

I remember a time that when "rust-analyzer.check.workspace": false, was set, saving a file was instant... I don't think our repo has grown that much larger that it's due to that...

@afroozeh
Copy link

@Veykril I think the issue is not resolved. It still seems to me that cargo clippy checks the current crate and all dependent ones when "rust-analyzer.check.workspace": false. I'm on 0.3.2228 which is released today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants