-
Notifications
You must be signed in to change notification settings - Fork 440
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
Integrate with RLS #95
Comments
If there's not a way better than #71, I am happy to polish it up.
I've been using intellij-rust and don't think it's worth trying to fix all
of the things downstream of Cargo.toml`s
…On Fri, Jun 8, 2018, 16:46 Damien Martin-Guillerez ***@***.***> wrote:
I am working with VSCode and while it is super nice with Rust & Cargo, RLS
bugs out with Bazel. I am actually contemplating shaving that yak further
and modifying RLS and VSCode RLS to support generic build system but I have
resisted the temptation so far so I am creating a bug here so it doesn't
get lost.
Might be a workaround to use #71
<#71>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#95>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACLjeOebODU7ZdkWWQWmR0tbdQi1s8uVks5t6uKugaJpZM4Ug7gB>
.
|
Ok so I think #71 and bazel-watcher is going to make an acceptable solution. Not as nice as getting RLS to understand other build system (although seeing RLS code, that shouldn't be too hard) but it seems to be the more straightforward version. I'll leave some more comment about the prototype on #71. |
I digged a bit more in RLS source code and they basically runs cargo to get the list of rustc call it perform (see https://github.com/rust-lang-nursery/rls/blob/master/src/build/cargo.rs#L332). This should be quite easy to adapt to Bazel except that the current rust files create a symlink tree that is not exposed. This could be done by simply moving the setup_cmd in a separate command that would output a directory artifact. So the rustc execution would all be a single command. I actually don't follow why we need to do all that set-up, why can't we point rustc to the deps directly? I reviewed that original code but I was knowing nothing of rust at that time. |
Ok there is one reason we might want to put all deps in one dir is to avoid explosion in the command line size but there is a standard way to fix that is by using params file. It seems like rustc does not support it at the moment but I am pretty sure it wouldn't be hard to upstream that. |
I'll probably tackle this during my next flight (this week). My current plan is to use bazel aquary to generate a build plan and pass it to RLS. RLS support getting a JSON build plan (see cargo build --build-plan) from a custom command (specified as part of RLS config here: https://github.com/rust-lang-nursery/rls/blob/master/src/config.rs#L165). |
@damienmg why is this approach preferable to generating the |
This approach is simpler than generating the toml file (that we don't have at the moment) and support generated file. The build by RLS would be driven by Bazel. |
Perhaps integrating with rust-analyzer would be easier at the moment. rust-analyzer is part of a larger rls-2.0 effort and already has some build system agnostic functionality. |
So what's the recommended way now to have RLS in VSCode works with Bazel? |
Anybody got anything that works OK? |
I'm interested as well. |
I was thinking about messing around with aspects to generate the config that rust-analyzer expects. I know some compilation database setups really emphasize not having to do a rebuild but I think that makes the most sense. |
What do you mean about avoiding a rebuild? The aspect running would re-do a minimal portion of loading the build graph, and should cache very well. If you really wanted to, you could generate the Cargo.toml and whatever other metadata as part of the various crate rules themselves. |
Is this still necessary after #505? I thought RLS was being replaced by rust_analyzer |
I believe the plan is for rust-analyzer to become RLS 2.0. rust-analyzer is already in a better spot than the original RLS, generally it is recommended over the original RLS. Considering many people are already moving away from the original RLS, I think #505 makes this issue much less relevant. |
I'm gonna close this for now. Happy to reopen it if someone feels there's still value in doing this. |
I am working with VSCode and while it is super nice with Rust & Cargo, RLS bugs out with Bazel. I am actually contemplating shaving that yak further and modifying RLS and VSCode RLS to support generic build system but I have resisted the temptation so far so I am creating a bug here so it doesn't get lost.
Might be a workaround to use #71.
The text was updated successfully, but these errors were encountered: