rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.19k stars 1.59k forks source link

cargo.targetDir seems to ignore $CARGO_TARGET_DIR #18296

Open kanashimia opened 2 days ago

kanashimia commented 2 days ago

rust-analyzer seems to ignore $CARGO_TARGET_DIR at least partly when cargo.targetDir is set. I'm pretty sure that this is a regression and it worked before. With it now I get stuff like this in my git directory:

› echo */target/
example/target/ engine/target/ synth/target/ physics/target/ render/target/ die-hard-macros/target/ die-hard/target/

I will try to find the offending version / commit, but it will take some time.

rust-analyzer version: 1.83.0-nightly (eb4e234 2024-10-09)

rustc version: 1.83.0-nightly (eb4e23467 2024-10-09)

editor or extension: helix

relevant settings:

[language-server.rust-analyzer.config]
cargo.targetDir = true
CARGO_TARGET_DIR=/home/kanashimia/.local/state/cargo
CARGO_HOME=/home/kanashimia/.local/state/cargo
snspinn commented 1 day ago

Having the same issue in Zed.

Veykril commented 1 day ago

The docs say that setting that config to true will use a subdir

Set to true to use a subdirectory of the existing target directory or set to a path relative to the workspace to use that path.

So I am not sure why you would expect us to honor cargos environment variable there, it never used that env var afaik. If you want r-a to use that env you should unset that config

kanashimia commented 1 day ago

it never used that env var afaik

Yeah, looking at the commits and testing some previous versions I don't think that environment variable was used at all, I don't know what was that.

So I am not sure why you would expect us to honor cargos environment variable there

In the description you quoted: "subdirectory of the existing target directory", if existing target directory is located in a different place then rust-analyzer should make subdirectory in it, it makes most sense.

If you want r-a to use that env you should unset that config

I use it because I don't want for cargo run to be blocked by rust-analyzer doing whatever it does for eternity, having a separate target solves that at the cost of extra compilation time.

https://github.com/rust-lang/rust-analyzer/blob/418c1365eccf20c9261b6948a6e637f789224af9/crates/rust-analyzer/src/config.rs#L2029-L2038

Seems quite easy to fix, I will make a patch.

Also it seems you can't use absolute paths here for some reason, so I can't even type it out manually, needs to be fixed too. I found that was broken by this commit: https://github.com/rust-lang/rust-analyzer/commit/c8fdcea85c273c386d1024815b955af6edae3fa2 I guess it was changed because option description mentions subdirectories explicity, again why?

Veykril commented 1 day ago

In the description you quoted: "subdirectory of the existing target directory", if existing target directory is located in a different place then rust-analyzer should make subdirectory in it, it makes most sense.

Ah right, that's a good catch, we do hardcode it as a subdir of target right now

Veykril commented 1 day ago

Also it seems you can't use absolute paths here for some reason, so I can't even type it out manually, needs to be fixed too. I found that was broken by this commit: https://github.com/rust-lang/rust-analyzer/commit/c8fdcea85c273c386d1024815b955af6edae3fa2 I guess it was changed because option description mentions subdirectories explicity, again why?

Judging from the diff, absolute paths were rejected as the easy way out, would need to check the call site as to whether that would cause issues or not.