rust-lang / rust-analyzer

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

rust-analyzer proc macro support causes project rebuilds #13011

Closed RalfJung closed 2 years ago

RalfJung commented 2 years ago

See https://github.com/rust-lang/miri/issues/2406 for details, but the summary is: with a fully loaded build cache, when I start vscode, let RA do its start-up indexing, and then quite vsocde, now if I build my project again, there are crates that it needs to rebuild. The first crate being rebuilt is libc.

The only happens if I have proc macros and build scripts enabled. Setting "rust-analyzer.cargo.buildScripts.useRustcWrapper": false changes nothing. However, setting

    "rust-analyzer.cargo.buildScripts.enable": false,
    "rust-analyzer.procMacro.enable": false,

makes the problem disappear.

Veykril commented 2 years ago

All r-a does in your case should be running ./miri check --message-format=json though ... I'm not sure how that would invalidate your builds 🤔 (the only other thing that happens is that we set the working directory of executing that command to the workspace root, but I assume that should be the case for you in general)

RalfJung commented 2 years ago

When I keep vscode open and just save a file to make it run ./miri check, that doesn't cause the problem.

So it's something that only happens during vscode startup that is different from the check-on-save build.

RalfJung commented 2 years ago

So I suspect it has to do with the invocation for proc-macros. I am not sure how to debug what exactly it does in that invocation though. I made ./miri dump the environment, but strangely, it gets called only twice when vsocde loads -- once for each of the linkedProjects. I expected a third invocation for proc macros. In fact I have changed rust-analyzer.buildScripts.overrideCommand to a command that will fail (exit code 1), and RA does not complain. How can I tell whether rust-analyzer.buildScripts.overrideCommand is actually taking effect?

Veykril commented 2 years ago

Setting RA_LOG to rust_analyzer::reload=info should give some relevant build-script/proc-macro building logging (including the executed command)

RalfJung commented 2 years ago

Not sure what I am looking for, but I don't see it in these logs

[INFO rust_analyzer::reload] will fetch workspaces cause=startup
[INFO rust_analyzer::reload] did fetch workspaces [Ok(Cargo { root: Some("miri"), n_packages: 78, sysroot: true, n_rustc_compiler_crates: 197, n_rustc_cfg: 68, n_cfg_overrides: 1, toolchain: Some(Version { major: 1, minor: 65, patch: 0, pre: Prerelease("nightly") }) }), Ok(Cargo { root: Some("cargo-miri"), n_packages: 53, sysroot: true, n_rustc_compiler_crates: 197, n_rustc_cfg: 68, n_cfg_overrides: 1, toolchain: Some(Version { major: 1, minor: 65, patch: 0, pre: Prerelease("nightly") }) })]
[INFO rust_analyzer::reload] will switch workspaces cause=fetched workspace
[INFO rust_analyzer::reload] Spawning proc-macro servers
[INFO rust_analyzer::reload] Using proc-macro server at /home/r/.rustup/toolchains/miri/libexec/rust-analyzer-proc-macro-srv args=[]
[INFO rust_analyzer::reload] Using proc-macro server at /home/r/.rustup/toolchains/miri/libexec/rust-analyzer-proc-macro-srv args=[]
[INFO rust_analyzer::reload] did switch workspaces
[INFO rust_analyzer::reload] will fetch workspaces cause=workspace vfs file change: /home/r/src/rust/miri/src/lib.rs
[INFO rust_analyzer::reload] did fetch workspaces [Ok(Cargo { root: Some("miri"), n_packages: 78, sysroot: true, n_rustc_compiler_crates: 197, n_rustc_cfg: 68, n_cfg_overrides: 1, toolchain: Some(Version { major: 1, minor: 65, patch: 0, pre: Prerelease("nightly") }) }), Ok(Cargo { root: Some("cargo-miri"), n_packages: 53, sysroot: true, n_rustc_compiler_crates: 197, n_rustc_cfg: 68, n_cfg_overrides: 1, toolchain: Some(Version { major: 1, minor: 65, patch: 0, pre: Prerelease("nightly") }) })]
[INFO rust_analyzer::reload] will switch workspaces cause=fetched workspace
[INFO rust_analyzer::reload] build scripts do not match the version of the active workspace
[INFO rust_analyzer::reload] will fetch workspaces cause=workspace vfs file change: /home/r/src/rust/miri/test_dependencies/src/main.rs
[INFO rust_analyzer::reload] did fetch workspaces [Ok(Cargo { root: Some("miri"), n_packages: 78, sysroot: true, n_rustc_compiler_crates: 197, n_rustc_cfg: 68, n_cfg_overrides: 1, toolchain: Some(Version { major: 1, minor: 65, patch: 0, pre: Prerelease("nightly") }) }), Ok(Cargo { root: Some("cargo-miri"), n_packages: 53, sysroot: true, n_rustc_compiler_crates: 197, n_rustc_cfg: 68, n_cfg_overrides: 1, toolchain: Some(Version { major: 1, minor: 65, patch: 0, pre: Prerelease("nightly") }) })]
[INFO rust_analyzer::reload] will switch workspaces cause=fetched workspace
[INFO rust_analyzer::reload] build scripts do not match the version of the active workspace
[INFO rust_analyzer::reload] will fetch build data cause=workspace updated
[INFO rust_analyzer::reload] will switch workspaces cause=fetched build data
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/target/debug/deps/libserde_derive-9e551b1e5775ff31.so: ["Serialize", "Deserialize"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/target/debug/deps/libserde_derive-9e551b1e5775ff31.so: ["Serialize", "Deserialize"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/cargo-miri/target/debug/deps/libenum_iterator_derive-6ca7fcc5b753f77e.so: ["IntoEnumIterator"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/cargo-miri/target/debug/deps/libgetset-0c1f0ca0d50ffbe1.so: ["Getters", "CopyGetters", "MutGetters", "Setters"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/cargo-miri/target/debug/deps/libproc_macro_error_attr-8a3132bdad857a2d.so: ["proc_macro_error"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/cargo-miri/target/debug/deps/librustversion-e67a322b869b9b50.so: ["stable", "beta", "nightly", "since", "before", "not", "any", "all", "attr", "cfg"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/cargo-miri/target/debug/deps/libserde_derive-a7cae654cc0e49ec.so: ["Serialize", "Deserialize"]
[INFO rust_analyzer::reload] Loaded proc-macros for /home/r/src/rust/miri/cargo-miri/target/debug/deps/libthiserror_impl-d328180265862f40.so: ["Error"]
[INFO rust_analyzer::reload] did switch workspaces
eminence commented 2 years ago

I have also seen this problem, but I hadn't tied the problem to proc-macros or RA's proc-macro support (in my head, I thought it was related to different feature sets being used between RA and cargo, but I have no data at all to support that idea). The symptoms for me are something like this:

  1. I run my code, triggering a build of whatever is out-of-date
  2. I iterate on the code, relying heavily on RA's "check-on-save" feature
  3. I run my code again and notice that some (but not all) of my dependencies are rebuilt when they shouldn't be.

The problem doesn't always happen for me (in fact it's fairly rare). But when I see it again, I'll try to make the config changes Ralf suggested to see if it problem also goes away.

Veykril commented 2 years ago

Heh my bad, the relevant logging for build scripts is in project_model::build_scripts (the one I linked is usually only proc-macro stuff .. mixed them up), so the env filter should be project_model::build_scripts=info.

I'll see if i can reproduce this on miri myself so we don't have to debug this by proxy Actually that's not gonna work since miri is a bash script and I'm on windows (without having wsl setup or anything).

Veykril commented 2 years ago

So ye would be good to set RA_LOG to project_model::build_scripts=info and see if the build-command fits. Also iirc CARGO_LOG=cargo::core::compiler::fingerprint=trace makes cargo cargo show re-builds reasons (but I don't know the specifics here).

RalfJung commented 2 years ago

Yeah I know the cargo flag but find those logs extremely hard to interpret...

RalfJung commented 2 years ago

Yeah that doesn't look good.

[INFO project_model::build_scripts] Running build scripts: "cargo" "check" "--quiet" "--workspace" "--message-format=json" "--all-targets"

In the client logs it does show

  buildScripts: {
    overrideCommand: [ './miri', 'check-fail', '--message-format=json' ]
  }

so the setting definitely made its way somewhere, but then got lost further down?

(./miri check-fail will always fail, I wanted to see whether it actually uses that command.)

Having a rebuild after a direct cargo invocation is expected; ./miri sets some env vars that will affect the build. That's why I configured vsocde to use ./miri in the first place. :)

Veykril commented 2 years ago

Oh, I see the issue, you need to set rust-analyzer.cargo.buildScripts.overrideCommand, judging from the linked issue which shows your config you are setting rust-analyzer.buildScripts.overrideCommand though.

VSCode doesn't lint unknown configurations for some reason (it only tells you if you hover the unknown key which is just absolutely useless). I'd love to implement a warning message server side for incorrect keys but that isn't so easy unfortunately with our current system... (at least it looked like that last time I checked)

Veykril commented 2 years ago

@eminence your issue doesn't seem build script related, but checkOnSave related, this usually happens if the environment r-a runs in differs from the environment you use to manually build (env-vars for example)

RalfJung commented 2 years ago

Oh d'oh, yes that fixes the problem. Sorry for the noise! Now I wonder where I copy-pasted that from... https://github.com/rust-lang/miri/pull/2491 fixes our config. Thanks for the help!

(Not that I understand why buildScripts is below cargo but checkOnSave is not... :shrug: )