Skip to content

Fix path resolution of rustfmt in cargo-fmt #6566

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

Merged
merged 2 commits into from
Jun 5, 2025

Conversation

onur-ozkan
Copy link
Member

Instead of resolving rustfmt path from RUSTFMT or PATH envs, use the one located in the same directory as cargo-fmt. So that cargo-fmt uses the expected version of rustfmt to avoid accidental use of unrelated versions.

This shouldn't change anything on the current behavior but it should make it easier to test the bootstrapped cargo-fmt in rust-lang/rust, since it currently always points to the globally installed rustfmt.

Moved from rust-lang/rust#141100

@ytmimi
Copy link
Contributor

ytmimi commented May 31, 2025

I feel like using RUSTFMT is fine. How is the bootstrapped cargo-fmt being tested in r-l/rust?

@onur-ozkan
Copy link
Member Author

Well, people can't build cargo-fmt and test it in the bootstrapped sysroot without RUSTFMT env variable, which is not obvious (most people don't have any idea about this implementation detail).

Does this PR bring any limitation/problem on cargo-fmt?

@ytmimi
Copy link
Contributor

ytmimi commented Jun 2, 2025

I don't think I'd want to remove the RUSTFMT environment variable, but if it's not set then I think looking for the rustfmt executable in the same directory as cargo-fmt makes sense.

@onur-ozkan
Copy link
Member Author

Yeah, that's reasonable. I will do that tomorrow.

@onur-ozkan
Copy link
Member Author

Yeah, that's reasonable. I will do that tomorrow.

It should be ready now.

Instead of resolving `rustfmt` path from RUSTFMT or PATH envs, use the one
located in the same directory as `cargo-fmt`. So that `cargo-fmt` uses the
expected version of `rustfmt` to avoid accidental use of unrelated versions.

Signed-off-by: onur-ozkan <[email protected]>
Signed-off-by: onur-ozkan <[email protected]>
@ytmimi ytmimi merged commit 1443bba into rust-lang:master Jun 5, 2025
26 checks passed
@ytmimi ytmimi added release-notes Needs an associated changelog entry and removed pr-not-reviewed labels Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Needs an associated changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants