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.

Sign up for GitHub

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

Jump to bottom

rustfmt override no longer works as a relative path #15595

Closed
RalfJung opened this issue Sep 11, 2023 · 2 comments · Fixed by #15600
Closed

rustfmt override no longer works as a relative path #15595

RalfJung opened this issue Sep 11, 2023 · 2 comments · Fixed by #15600
Labels
C-bug Category: bug

Comments

@RalfJung
Copy link
Member

RalfJung commented Sep 11, 2023 •
edited
Loading

I'm afraid #15564 has caused new problems... rustfmt doesn't work in the rustc workspace at all any more. The override is

    "rust-analyzer.rustfmt.overrideCommand": [
        "./build/host/rustfmt/bin/rustfmt",
        "--edition=2021",
    ],

and now this is interpreted relative to the directory the file is in, which of course does not work.

[Error - 2:59:01 PM] Request textDocument/formatting failed.
  Message: Failed to spawn cd "/home/r/src/rust/rustc.3/compiler/rustc_middle/src/mir" && "./build/host/rustfmt/bin/rustfmt" "--edition=2021"
  Code: -32603

Probably this could be fixed by having RA do workspace_dir.join(rustfmt_path) so that it doesn't rely on a relative directory any more.

rust-analyzer version: rust-analyzer version: 0.3.1657-standalone
@RalfJung RalfJung added the C-bug Category: bug label Sep 11, 2023
@RalfJung RalfJung mentioned this issue Sep 11, 2023
Merged
@davidbarsky
Copy link
Contributor

Crap, I'm sorry about that. I'll put up a PR and test it against rustc. I'll proceed under the assumption that I'll need to bootstrap rustc?

@RalfJung
Copy link
Member Author

You just need to check out rustc and open it in vscode + RA with the config suggested in the dev guide, and try to format some files.

@davidbarsky davidbarsky mentioned this issue Sep 12, 2023
Merged
@RalfJung RalfJung mentioned this issue Sep 14, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 16, 2023
@bors
Auto merge of rust-lang#115836 - RalfJung:rust_analyzer_settings.json…
96f3a66 
…, r=compiler-errors

update rust_analyzer_settings.json

This works around rust-lang/rust-analyzer#15595, and avoids relying on the unspecified working directory of this command.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 16, 2023
@matthiaskrgr
Rollup merge of rust-lang#115836 - RalfJung:rust_analyzer_settings.js…
edea0d5 
…on, r=compiler-errors

update rust_analyzer_settings.json

This works around rust-lang/rust-analyzer#15595, and avoids relying on the unspecified working directory of this command.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 16, 2023
@rust-timer
Unrolled build for rust-lang#115836
e0b5514 
Rollup merge of rust-lang#115836 - RalfJung:rust_analyzer_settings.json, r=compiler-errors

update rust_analyzer_settings.json

This works around rust-lang/rust-analyzer#15595, and avoids relying on the unspecified working directory of this command.
bors added a commit that referenced this issue Oct 6, 2023
@bors
Auto merge of #15600 - davidbarsky:davidbarsky/broken-rustfmt-in-ra, …
b39875b 
…r=Veykril

fix: ensure `rustfmt` runs when configured with `./`

(Hopefully) resolves #15595. This change kinda approaches canonicalization—which I am not a fan of—but only in service of making `./`-configured commands run correctly.

Longer-term, I feel like this code should be removed once `rustfmt` supports recursive searches of configuration files or interpolation of values like `${workspace_folder}` lands in rust-analyzer.

## Testing

I cloned `rustc`, setup rust-analyzer as suggested in the [`rustc` dev guide](https://rustc-dev-guide.rust-lang.org/building/suggested.html#configuring-rust-analyzer-for-rustc), saved and formatted files in `src/tools/miri` and `compiler`, and saw `rustfmt` (seemingly) correctly.
@bors bors closed this as completed in b1f89a8 Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: ensure rustfmt runs when configured with ./ davidbarsky/rust-analyzer
2 participants
@RalfJung @davidbarsky