rust-lang / rust-mode

Emacs configuration for Rust
Apache License 2.0
1.09k stars 176 forks source link

Running rust-check, rust-compile & friends are not workspace aware #456

Closed yanchith closed 1 year ago

yanchith commented 1 year ago

Hi folks!

I have a mild annoyance with rust-mode.rust-check and friends run the cargo command in the crate root, but if the crate root is a member of a workspace, that is not taken into account in any way.

To work around this in workspace projects, I run rust check and friends from the project root, so the entire workspace gets built.

I can imagine workspace auto-detection is not what you'd always want, but it would be great if there were workspace-aware alternatives to the functions that run cargo, like rust-check-workspace, the only difference being them cd-ing up to the workspace directory before invoking cargo.

I'd happily make a PR for this if maintainers agree.

yanchith commented 1 year ago

Did some local experimentation and it seems like it is enough to locate the correct rust project here: https://github.com/rust-lang/rust-mode/blob/master/rust-cargo.el#L47 by adding a --workspace flag.

Note that it is not enough to add the workspace flag to the invocations of cargo itself (e.g. cargo check --workspace), because while that would check the entire workspace, the file paths in the compilation buffer would be rooted at the workspace member, not the entire workspace, and navigating to the site of the error would not be possible.

brotzeit commented 1 year ago

Thanks for the bug report. Can you please show me an example Cargo.toml.

yanchith commented 1 year ago

I am not sure this is a bug report. More like a feature request.

One Cargo.toml is not sufficient to reproduce the problem, but here is a repo with the reproduction: https://github.com/yanchith/rust-mode-repro

The repo has just one source file: ./repro_app/src/main.rs. It has a type error just so we have an error to show for rust-check.

brotzeit commented 1 year ago

If you run rust-check from main.rs, the compilation buffer runs and shows the errors, but the error paths, when concatenated to the path of the compilation buffer, do not produce valid links for Emacs to follow, meaning you can not directly visit the error sites when pressing Enter or clicking.

I see. Let me think about it. Could be tricky to get this working.

brotzeit commented 1 year ago

Or do you already have a solution ?

yanchith commented 1 year ago

I literally just replaced https://github.com/rust-lang/rust-mode/blob/master/rust-cargo.el#L47 with (let ((ret (call-process rust-cargo-bin nil t nil "locate-project" "--workspace"))) and cargo seems to do the correct thing even for non-workspace projects, but there may be edge cases I haven't thought about.

brotzeit commented 1 year ago

Oh, I didn't realize this is missing in rust-mode. Would be great if you could open a pr and replace the line with https://github.com/brotzeit/rustic/blob/master/rustic.el#L82. Somebody seems to have fixed it for rustic some time ago.

yanchith commented 1 year ago

Opened a PR.

I think running cargo locate-project --workspace is a good-enough default for most, but some people would maybe like to compile just the one crate (which is what was happening until now), in which case we might need to split the surface-level commands analogously to this:

I am personally happy with this small change as is.

brotzeit commented 1 year ago

Thanks. We had a discussion about this some time ago and introduced rustic-compile-directory-method for rustic. If you are interested in doing something similar for rust-mode, I would merge it. I think it's better to not introduce separate commands for each cargo command.