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

No problems are shown for non-Cargo projects #15963

Open
dwlsalmeida opened this issue Nov 25, 2023 · 20 comments
Open

No problems are shown for non-Cargo projects #15963

dwlsalmeida opened this issue Nov 25, 2023 · 20 comments
Labels
A-rust-project rust-project.json related issues C-support Category: support questions

Comments

@dwlsalmeida
Copy link

Hi there,

No problems are shown when using RA for non-Cargo projects in VScode. My linkedProjects points to a valid rust-project.json file, and other features are working (hints, code completion, etc).

I tried debugging RA and apparently CargoActor::run expects the command to output some kind of JSON as per the documentation. The problem is that my project is being built by Meson/Ninja, and thus, whatever JSON is expected is simply not produced by these tools.

Is this expected behavior? I was under the impression that the whole point of rust-project.json was to cater to non-Cargo projects and that RA would manage to invoke rustc and then parse whatever compilation errors on its own.

@dwlsalmeida dwlsalmeida added the C-support Category: support questions label Nov 25, 2023
@davidbarsky
Copy link
Contributor

This is expected behavior, yes. rust-project.json is only responsible for providing rust-analyzer an index of the crate graph, it does not tell rust-analyzer how to build your project.

@dwlsalmeida
Copy link
Author

I humbly suggest to clarify this in the docs, I spent quite a bit of time thinking the issue was on my end.

On another note, is there any way I can make this work? Lots of people using Meson, probably, so maybe there's some workaround already in place?

@xclaesse
Copy link

it does not tell rust-analyzer how to build your project.

Could rust-analyser use compile_commands.json for that?

@xclaesse
Copy link

Example of what you get in that file when using Meson to build Mesa rust code:

  {
    "directory": "/home/xclaesse/programmation/mesa/builddir",
    "command": "rustc -C linker=cc --color=always -C debug-assertions=yes -C overflow-checks=no --crate-type rlib -C opt-level=2 -g --crate-name rusticl_opencl_gen --emit dep-info=src/gallium/frontends/rusticl/rusticl_opencl_gen.d --emit link=src/gallium/frontends/rusticl/librusticl_opencl_gen.rlib --out-dir src/gallium/frontends/rusticl/librusticl_opencl_gen.rlib.p -C metadata=b863b06@@rusticl_opencl_gen@sta -Aclippy::all -Aunused_unsafe -Anon_camel_case_types -Anon_snake_case -Anon_upper_case_globals src/gallium/frontends/rusticl/rusticl_opencl_bindings.rs",
    "file": "src/gallium/frontends/rusticl/rusticl_opencl_bindings.rs",
    "output": "src/gallium/frontends/rusticl/librusticl_opencl_gen.rlib"
  },

@dwlsalmeida
Copy link
Author

There is also the issue of whether rustc can itself emit JSON or if this is something done by Cargo

@xclaesse
Copy link

Pretty sure it can: https://doc.rust-lang.org/rustc/json.html, unless that's not what RA needs?

@davidbarsky
Copy link
Contributor

Sorry, was (correctly) yanked away from the computer by my spouse over the holiday.

I humbly suggest to clarify this in the docs, I spent quite a bit of time thinking the issue was on my end.

Probably not a bad idea.

On another note, is there any way I can make this work? Lots of people using Meson, probably, so maybe there's some workaround already in place?

I've never used Meson before, but a decent chunk of my dayjob is supporting using rust-analyzer with buck2. Depending on whether Meson requires you to specify a build target for each command or not (where buck/bazel is an example of the former and Cargo is an example of the latter), you might be able to make use of the rust-analyzer.check.overrideCommand setting (search for it on https://rust-analyzer.github.io/manual.html, as I'm unable to link to a definition item) if you're able to get Meson to emit JSON-structured diagnostics from rustc.

Could rust-analyser use compile_commands.json for that?

@xclaesse I'm assuming that you're referring clangd's compile_commands.json? I don't think it'll take that form specifically, but there might be sufficient demand for something that does a similar thing in rust-analyzer.

Pretty sure it can: https://doc.rust-lang.org/rustc/json.html, unless that's not what RA needs?

Yes, that is the output that rust-analyzer expects from the build system/compiler.

@xclaesse
Copy link

xclaesse commented Nov 28, 2023

@xclaesse I'm assuming that you're referring clangd's compile_commands.json? I don't think it'll take that form specifically, but there might be sufficient demand for something that does a similar thing in rust-analyzer.

Yes, that file is created by Ninja (and thus meson, but also cmake) and is used for example by vscode C/C++ analysers. I think it would be a nice addition for rust-analyser to use the same file, it's somewhat "the standard". My understanding is it should be pretty easy to find the rustc command from that file and just append --error-format=json. Is there any info missing for RA to work that way?

https://clang.llvm.org/docs/JSONCompilationDatabase.html

@xclaesse
Copy link

Seems there is already request to support that db in buck2 as well: facebook/buck2#194.

@Veykril Veykril added the A-rust-project rust-project.json related issues label Nov 28, 2023
@davidbarsky
Copy link
Contributor

Yes, that file is created by Ninja (and thus meson, but also cmake) and is used for example by vscode C/C++ analysers. I think it would be a nice addition for rust-analyser to use the same file, it's somewhat "the standard".

While it is certainly the standard in C++, I don't believe it's the standard. At the risk of coming off as being afflicted with NIH, I think that any build system integration into rust-analyzer would either take the form of collaborating with a companion extension (such as a buck-companion or vscode-meson) or forcing the non-Cargo build system to emit data in a form very similar to what Cargo does 1.

My understanding is it should be pretty easy to find the rustc command from that file and just append --error-format=json. Is there any info missing for RA to work that way?

For Meson? I don't know. If it possible to do a meson compile --error-format=json (where --error-format=json is passed to rustc), then rust-analyzer.check.overrideCommand will just work. The limiting factor, from rust-analyzer's perspective, is that the rust-project.json format deliberately doesn't encode Cargo Target-like data that would allow determining what build target needs to be invoked for a specific crate. Put differently, rust-project.json is lower level than you might otherwise expect.

In the future, Meson could emit a rust-analyzer.toml alongside a rust-project.json that configures a per-directory build override.

Seems there is already request to support that db in buck2 as well: facebook/buck2#194.

For what it's worth, my colleagues already make use of the compilation database in C++ with some FB-internal rules. The request there is to add the same functionality in the open source release, but because C++ toolchains at FB are, uh, complicated, that hasn't happened yet.

Footnotes

  1. I've opened an issue to add this functionality, but I'm not fully convinced that this is the correct approach. At work, we're using https://github.com/rust-lang/rust-analyzer/pull/15476 in tandem with this CLI, but changing how we do compiler diagnostics with buck is, generally speaking, pretty easy.

@xclaesse
Copy link

xclaesse commented Nov 28, 2023

At the risk of coming off as being afflicted with NIH

Nothing against creating new format if it can be justified. However, it's not clear why RA cannot work that way in this case, that seems pretty easy and would give a build system independent solution. Why do you even want to go through meson compile at all when that can be avoided?

The limiting factor, from rust-analyzer's perspective, is that the rust-project.json format deliberately doesn't encode Cargo Target-like data that would allow determining what build target needs to be invoked for a specific crate

That's fine, you don't need to extend rust-project.json format. All you need to do is cross checking with compile_commands.json to find the missing information:

rust-project.json:

"root_module": "/home/xclaesse/programmation/mesa/src/gallium/frontends/rusticl/mesa/lib.rs",

compile_commands.json:

"directory": "/home/xclaesse/programmation/mesa/builddir",
"command": "rustc ...",
"file": "../src/gallium/frontends/rusticl/mesa/lib.rs",

From root_module, you find the command (you need to join "directory" and "file" for matching), append --error-format=json, job done. Get cmake and every other build system support for free.

@xclaesse
Copy link

take the form of collaborating with a companion extension (such as a buck-companion or vscode-meson)

Note that I'm a dev of vscode-meson, that's exactly why I'm trying to see how this could be done. meson itself does not support adding --error-format=json when compiling a target. I guess we could provide some wrapper script that takes both rust-project.json and compile_commands.json files and do the matching. That wrapper could then be used as rust-analyzer.check.overrideCommand. I think that it would be better done in rust-analyser vscode extension though.

@flodiebold
Copy link
Member

We generally don't want to implement a build system into rust-analyzer (using compile commands would still involve figuring out what needs to be rebuilt in what order when things change, etc.). The whole check-on-save functionality is just a stopgap while RA doesn't emit most diagnostics natively yet. A wrapper script called from check.overrideCommand is the way to go to support other build systems with it.
We also don't want to add support for more build systems in the VSCode extension; that should be handled by companion extensions as David described.

@xclaesse
Copy link

using compile commands would still involve figuring out what needs to be rebuilt in what order when things change

That's a valid concern. I think in C/C++ case it just assume that dependencies are up to date, which is reasonable for a check when saving a single file in the editor.

We also don't want to add support for more build systems in the VSCode extension

What I'm suggesting would be a build system agnostic solution, specifically for not having to add support for different build systems.

that should be handled by companion extensions as David described.

I'm not sure what can be done at vscode-meson extension level. Can check.overrideCommand be somehow overriden by a typescript function in my extension? What are arguments passed to that command, the crate root_module?

@xclaesse
Copy link

Another use-case for using compile_commands.json is the Linux kernel, I got told.

@davidbarsky
Copy link
Contributor

That's a valid concern. I think in C/C++ case it just assume that dependencies are up to date, which is reasonable for a check when saving a single file in the editor.

At the risk of overexplaining, this is not sufficient for Rust. All dependencies of a file need to be built first in order to get meaningful diagnostics. That's why I strongly recommend running meson build instead.

meson itself does not support adding --error-format=json when compiling a target.

How difficult would it be to change this behavior in Meson?

@xclaesse
Copy link

xclaesse commented Nov 28, 2023

At the risk of overexplaining, this is not sufficient for Rust. All dependencies of a file need to be built first in order to get meaningful diagnostics.

Which is not different from any language, rust has nothing special here. In C you can also have generated headers as dependency, until you build them you'll get some diagnostic errors. I agree it's not optimal, but it's already a nice first step.

To build dependencies, you would have to first call ninja with the "output" value of compile_commands.json to build it with it's deps. Then you can run the rustc command with the extra --error-format=json to get diagnostic report. But that mean you need a bit of build system specific code. Note that ninja is the backend for both meson and cmake (and more), so that's not entirely specific to meson either.

How difficult would it be to change this behavior in Meson?

That's just not possible at all. Meson generates a ninja file which contains every commands to run and their dependencies. It won't be able to add --error-format=json in rustc commands. Output would also be interleaved with gcc or other languages compilers.

@davidbarsky
Copy link
Contributor

Which is not different from any language, rust has nothing special here. In C you can also have generated headers as dependency, until you build them you'll get some diagnostic errors. I agree it's not optimal, but it's already a nice first step.

If I'm understanding what you're saying correctly, the diagnostics will not be functional or useful to users, requiring them to have a deep understanding of the build system to resolve said issues. In my personal view and experience, that is a non-starter and I strongly recommend that you do not go down this path.

To build dependencies, you would have to first call ninja with the "output" value of compile_commands.json to build it with it's deps. Then you can run the rustc command with the extra --error-format=json to get diagnostic report. But that mean you need a bit of build system specific code. Note that ninja is the backend for both meson and cmake (and more), so that's not entirely specific to meson either.

Is your vision that rust-analyzer would do this on the user's behalf, or...? For what it's worth, when I referred to a companion extension, the thinking was that instead of sending the flycheck parameters to the language server, they would instead be sent to extensions—such as vscode-meson—where the behavior you described above be done. This way, rust-analyzer does not have any awareness of Meson/Ninja/CMake-specific details. As a reference, I've made similar changes to #15830 that you can build on.

That's just not possible at all. Meson generates a ninja file which contains every commands to run and their dependencies. It won't be able to add --error-format=json in rustc commands. Output would also be interleaved with gcc or other languages compilers.

Why would this not be possible? I'm sorry, I'm trying to determine what the specific blocker is on Meson/Ninja. I ask because the diagnostic parsing machinery in rust-analyzer will attempt to read stdout/stderr line by line, but its failure to parse the resulting output doesn't cause crashes.

@Veykril
Copy link
Member

Veykril commented Nov 29, 2023

To add to this in regards to current cargo check running for diagnostics. We might consider in the future to remove that from rust-analyzer, moving it out into a separate extension that can hook into the rust-analyzer extension to fetch the information required to run cargo check. (There is technically no reason for it to be currently part of the server I believe aside for simplicity)

@xclaesse
Copy link

requiring them to have a deep understanding of the build system

Is clicking the build button, or running ninja -C builddir considered deep understanding of the build system? Just saying this has always been the case for C/C++ analysers in vscode and I don't see how Rust would be different. User might also not want vscode to start building for 5h in background to provide RA, they may want to choose when build happens. Running RA on a single crate is different from building the whole dependency tree.

Not saying this is ideal, we certainly can do better with a little bit of build system integration, but I don't see why that would be a showstopper.

Is your vision that rust-analyzer would do this on the user's behalf, or...? For what it's worth, when I referred to a companion extension, the thinking was that instead of sending the flycheck parameters to the language server, they would instead be sent to extensions—such as vscode-meson—where the behavior you described above be done. This way, rust-analyzer does not have any awareness of Meson/Ninja/CMake-specific details. As a reference, I've made similar changes to #15830 that you can build on.

I'm not sure to understand, I'm not expert in vscode extensions. Does that mean that extensions can talk to each other via some API? If RA can tell vscode-meson extension "compile $file and return me the json" then that's something we can definitely work in our side. I still believe it would be better done in RA itself because that would work for other build systems instead of duplicating the work in many extensions, but we can definitely at least experiment first in vscode-meson side.

Why would this not be possible? I'm sorry, I'm trying to determine what the specific blocker is on Meson/Ninja.

Meson generates a ninja file which is similar to a Makefile, a serie of rules and commands to run. Rust targets commands won't have --error-format=json argument and there is no way to tell ninja to add it, that would mean rewriting its build rules completely.

But to be fair, maybe I'm misunderstanding what needs to be done. Correct me if I'm wrong but I think the sequence of events is something like that:

  1. User saves "foo.rs" file.
  2. From info from rust-project.json, RA can determine that the file is part of "mycrate".
  3. RA runs something like "cargo check mycrate".
  4. cargo calls rustc with the extra --error-format=json to rebuild "mycrate" as well as all its dependencies.
  5. RA parses json output from cargo.

When using Meson, or any other build system than Cargo, I think something like that needs to happen:

  1. User saves "foo.rs" file.
  2. From info from rust-project.json, RA can determine that the file is part of "mycrate".
  3. RA runs something like "vscode-meson-ra-wrapper mycrate".
  4. vscode-meson-ra-wrapper tells ninja to build all deps of mycrate, but not mycrate itself
  5. vscode-meson-ra-wrapper lookup the rustc command needed to build mycrate, append --error-format=json and probably other args to skip code generation, etc?
  6. vscode-meson-ra-wrapper output rustc's output.
  7. RA parses json output.

Steps, 4 is indeed build system specific and RA would probably need a command to run in its settings. Step 5 is actually pretty agnostic to the build system if we rely on largely adopted compile_commands.json format, and thus would be better done in RA to avoid code duplication, especially since it involves editing a rustc command line which is much more RA's domain. Also spawning a new command wrapper each time means re-parsing compile_commands.json every time, while that could be cached if done in the extension.

If I misunderstood something, please explain what you think should happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust-project rust-project.json related issues C-support Category: support questions
Projects
None yet
Development

No branches or pull requests

5 participants