`rust-project check` cannot emit compile errors, only warnings, when used with system_rust_toolchain

[cormacrelf]

Observed:

$ rust-project check path/to/file.rs

# (no output)

Failure path:

1. A rust library can have an error in it. Imagine path/to/file.rs contains a syntax error.
2. rust-project check path/to/file.rs executes prelude//rust/rust-analyzer/check.bxl:check with some additional command line args including "-c=rust.failure_filter=true" "-c=rust.incremental=true"
3. The file is resolved to a bunch of targets that include it using the owner() cquery
4. Those targets are built with the diag.json subtarget, e.g. //path/to:lib[diag.json]
5. The failure_filter machinery is not engaged at all for system_rust_toolchain. The legacy config settings from step 2 are not read anywhere. The rust rules try to read toolchain_info.failure_filter but it is always false because it is never given a value anywhere, you can't even set it manually on system_rust_toolchain.
6. Those builds fail because of the syntax error in file.rs
7. No artifacts are found for those builds because the builds weren't prevented from failing
8. The bxl can't see the diag.json artifacts
9. Empty output.

Is the solution to create create config_settings for these and use them for the system_rust_toolchain defaults?

[davidbarsky]

Ugh, I should've documented this a little better, but the path you pass to rust-project should be an absolute path. If you're not doing that, this will not work. This was implemented in service of rust-lang/rust-analyzer#15381, an approach I'm... not fully confident in.

*The failure_filter machinery is not engaged at all for system_rust_toolchain. The legacy config settings from step 2 are not read anywhere. The rust rules try to read toolchain_info.failure_filter but it is always false because it is never given a value anywhere, you can't even set it manually on system_rust_toolchain.

That doesn't seem great, but I haven't looked too closely at this detail. Also, why do you consider those settings to be legacy?

[cormacrelf]

Lol, I made an almost identical patch to rust analyzer to try it out. It does go!

Re the absolute paths, yeah I had figured that out, just didn't write them that way in the issue for whatever reason. Rust analyzer gives you absolute paths anyway, but it did need a doc on the command line probably.

I worked out that if you patch prelude to apply failure_filter = True to the RustToolchainInfo provider, then rust-project works. So that proved the configs weren't being applied otherwise. Grepping the buck2 codebase for failure_filter also showed this.

I referred to those configs as legacy because they're documented under the legacy path in the docs https://buck2.build/docs/legacy/files-and-directories/dot-buckconfig/. I'm sure I also read somewhere, probably an issue on here, that much of the buckconfig is deprecated in favour of using platforms and constraints. I would guess they are still in use internally because so much code was using them from buck v1. Edit, as of half an hour ago: https://buck2.build/docs/rfcs/drafts/cfg-modifiers/api/

It would certainly be possible to configure the toolchain using a constraint for failure_filter. You would probably need the constraint to be applied in both the target and exec platforms so as to compile proc macros with failure filter as well (once the new plugins work lands for those and they become exec deps).

[davidbarsky]

Lol, I made an almost identical patch to rust analyzer to try it out. It does go!

Re the absolute paths, yeah I had figured that out, just didn't write them that way in the issue for whatever reason. Rust analyzer gives you absolute paths anyway, but it did need a doc on the command line probably.

Heh, yeah. I could've sworn I deliberately hid the check subcommand on rust-project, but it seems like I didn't, which is why I didn't really document it. That's my bad—I didn't really want to spend effort documenting a thing that I expected to change over time but you are not wrong that it should be now that it's (accidentally) not hidden.

I worked out that if you patch prelude to apply failure_filter = True to the RustToolchainInfo provider, then rust-project works. So that proved the configs weren't being applied otherwise. Grepping the buck2 codebase for failure_filter also showed this.

Gotcha, that's useful to know.

It would certainly be possible to configure the toolchain using a constraint for failure_filter. You would probably need the constraint to be applied in both the target and exec platforms so as to compile proc macros with failure filter as well (once the new plugins work lands for those and they become exec deps).

I didn't consider the interactions there, but I don't think you're wrong. I think the main issue with -C=failure-filter is that it doubles the number of actions in the action graph, but a colleague has explained to me in the past as to how we can get rid of this doubling. We might tackle this in the next 3 months or so.