-
Notifications
You must be signed in to change notification settings - Fork 888
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
Use rustfmt given by RUSTFMT env var #4419
Conversation
I'm not opposed to this, but is there any additional context you could share around the use case? Curious if it's a case of wanting to run There would also be some serious compatibility issues in the event someone tried to use v1 of one and v2 of the other |
Sure thing -- my work codebase is a big monorepo (>2 million lines of first-party Rust code) in which:
In terms of incompatibility between 1.x and 2.x, we are happy to attach an explicit |
That's really helpful info, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@topecongiro - will hold off on merging for a bit in case you have any objections, but seems a reasonable use case to me that we should support. If enough users end up leveraging and getting confused/hitting issues (for example the 1.x vs. 2.x) then we could address with a note to the docs and/or issue template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
We should add documentation about the RUSTFMT
environment variable to our README.md and the cargo's documentation. I will create an issue to track these, and merge this PR first.
I'm going to create a new branch to include the next rustc-ap bump in v1.4.22, and will pull these changes in so that it gets included with the next rustfmt update that hits nightly. |
Document RUSTFMT environment variable This PR documents the `RUSTFMT` enviorment variable, as specified in [rust-lang/rustfmt/4426](rust-lang/rustfmt#4426) and [rust-lang/rustfmt/4419](rust-lang/rustfmt#4419).
RUSTFMT=1 changes the binary used by cargo fmt - see rust-lang/rustfmt#4419
RUSTFMT=1 changes the binary used by cargo fmt - see rust-lang/rustfmt#4419 Signed-off-by: Richard Whitehouse <richard.whitehouse@metaswitch.com>
backported in #4453 |
This matches how Cargo uses the rustc and rustdoc specified by RUSTC and RUSTDOC environment variables (see https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-reads).
Trying to do this via PATH instead does not work because Cargo always prepends its own bin directory to PATH before calling any subcommand, and Rustup places its own rustfmt executable there.
$ PATH=/path/to/custom:$PATH cargo fmt # does not call /path/to/custom/rustfmt