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

Custom rustfmt commands cannot use relative executable paths #4486

Closed
ecstatic-morse opened this issue May 16, 2020 · 7 comments
Closed

Custom rustfmt commands cannot use relative executable paths #4486

ecstatic-morse opened this issue May 16, 2020 · 7 comments
Labels
E-easy S-actionable Someone could pick this issue up and work on it right now

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented May 16, 2020

While trying to make rust-analyzer use the bootstrap version of rustfmt inside rustc, @jonas-schievink discovered that using a relative path to rustfmt won't work:

Ah, looks like it adjusts the working dir dynamically, so it can't find the command with a relative path

https://github.com/rust-analyzer/rust-analyzer/blob/d51c1f62178c383363a2d95e865131d9a7b969d0/crates/rust-analyzer/src/main_loop/handlers.rs#L651-L655

As a workaround, you can specify an absolute path, but I was thinking we could canonicalize the first "shell word" if it looks like a path (e.g., ./rustfmt or build/rustfmt but not rustfmt2). This would require some sort of library that can do shell word-splitting, however. Perhaps there is a better way?

The which crate would be helpful for part of this.

@RalfJung
Copy link
Member

Curiously, relative paths do work for rust-analyzer.checkOnSave.overrideCommand, so it is somewhat surprising that they do not work for other overrides.

@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented May 16, 2020

I assume it's because checkOnSave doesn't change the current_dir depending on the file like rustfmt does. I haven't checked though.

@oxalica
Copy link
Contributor

oxalica commented May 19, 2020

According to the doc, current_dir may effect the resolution of relative program name. It's better to canonicalize the relative command path first.

But why do we need to set current_dir for rustfmt here?

@matklad
Copy link
Member

matklad commented May 19, 2020

But why do we need to set current_dir for rustfmt here?

Not sure there's a super-grand design here, but we do want rustfmt to pick up rustfmt.toml. Hm, is tihs right at all though? Like if I run rustfmt foo/bar/barz/qux.toml, will it pick the rustfmt.toml at foo/bar/rustfmt.toml? I think it should, so this current_dir patching could be removed....

Separately, I feel that semantics of relative paths in config should be independent of the cwd of the language server.

@matklad matklad added the E-easy label Jul 12, 2020
@lnicola lnicola added the S-actionable Someone could pick this issue up and work on it right now label Jan 27, 2021
@lnicola
Copy link
Member

lnicola commented Jan 27, 2021

@ecstatic-morse Can you test this again? The chdir was removed at some point and added back in #7371, but only for the default (non-overridden) case.

(Marking S-actionable, if you look at this in the future and the OP hasn't answered, ping me so we can close it.)

@RalfJung
Copy link
Member

I now set the rustfmt command to ./build/x86_64-unknown-linux-gnu/stage0/bin/rustfmt and that seems to work here. :)

@lnicola lnicola closed this as completed Jan 27, 2021
@mrchantey
Copy link

Is it the expected behavior that this vs code setting should not work?

{
"rust-analyzer.rustfmt.extraArgs": [
  "--config-path",
  "./config",
  ]
}

If so, what is the workaround for a file in ./config/rustfmt.toml? At the moment I'm using an absolute path in workspace settings but thats not portable:

{
"rust-analyzer.rustfmt.extraArgs": [
  "--config-path",
  "C:/Users/Example/Project/config",
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

6 participants