Remove RUSTC_WRAPPER hack

[Veykril]

Closes #12973

[matklad]

It feels like a PR/commit which could include a fair amount of context in the body :P

Absolutely not every commit should have a great commit message, but this probably shold

[Veykril]

Will add more context in a bit (doing some more timings testing right now)

[jplatte]

So I've just disabled rust-analyzer.cargo.buildScripts.useRustcWrapper to work around the anyhow issues (which apparently are still there on 1.0.61?), and that makes even more build scripts fail, or at least that's what the popup says.

When I look at the language server output, it seems like it's complaining about the crate I'm working on right now, which of course isn't always in a buildable state, but this is a binary crate without a build.rs so I'm quite puzzled. In any case, just wanted to let this be known, I'll try to see whether this reproduces in other workspaces / what could be wrong here.

[lnicola]

I've seen some weird build errors without the proxy which got fixed after a cargo clean, but you might be right.

[Veykril]

Ye, so I tested this on a random crate I had lying around https://github.com/djeedai/bevy_hanabi, current master takes ~20s to fully load metadata with build script set ups. With this PR its ~30s. And as jplatte mentioned, the problem is if a crate in the workspace is not in a buildable state, we will pop up an error even if build scripts work fine...

So I guess we have to stick to the wrapper still until cargo gives us an alternative

[jplatte]

Yeah, not using the rustc wrapper is a pretty bad experience right now. Just opened up another workspace with currently-non-buildable proc-macro code and the same error popped up, which I'm pretty sure was not the case before.

edit: No, actually this always happens when opening that workspace, regardless of the setting :(

[lnicola]

@jplatte you should probably test on a nightly which includes #12984. That might help a little, but probably not as much as the wrapper.

[jplatte]

Yeah I'm gonna use the wrapper again. It only breaks anyhow, and only on non-nightly versions.

[matklad]

current master takes ~20s to fully load metadata with build script set ups. With this PR its ~30s.

I'd say that 30% slowdown here is an acceptable cost to pay here to avoid questionable hacks.

For error reporting, I think we could add some filtering and just ignore errors from everything which is not compiled for $host?

[lnicola]

It not always 30%, if you disable check on save, it can take arbitrarily longer depending on how large the project is, right?

[matklad]

It not always 30%, if you disable check on save, it can take arbitrarily longer depending on how large the project is, right?

Right: I think 2x slowdown would be OK, 10x would be too much. For a typical project, I would probably expect something closer to 2x.

And in theory rust-analyzer should work OK while proc-macros are not loaded, it's not like you are completely blocked.

🤔 I wonder, can we maybe detect, by reading cargo messages, when all proc-macros have already compiled and kill the cargo thence? Probably not.

[flodiebold]

We could just disable it by default, instead of immediately removing it?

[lnicola]

We could, but the only crate which doesn't build correctly under the wrapper happens to be quite popular, so most users would have to keep it disabled.

[Veykril]

It not always 30%, if you disable check on save, it can take arbitrarily longer depending on how large the project is, right?

Note I tested this with checkOnSave disabled on a project that had no build artifacts yet, but yes the slowdown will scale with project size, though bevy as a dependency is already pretty large i think.

[veber-alex]

which apparently are still there on 1.0.61?

I can't reproduce the issue with anyhow 1.0.61 on either stable or nightly.

[bors]

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

[lnicola]

I think this hasn't been a problem lately, and ideally we'll get a caego option (not that there's been much interest on the cargo side). So it makes sense to close this.

[Veykril]

Ye let's close this, so far no one of the cargo team has replied to the RFC unfortunately so thsi will probably be a while until we can remove this.