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 is not run if rustup exists, but nightly toonchain/rustfmt-nightly is not installed #1184

Closed
Kobata opened this issue Dec 9, 2017 · 1 comment · Fixed by #1204
Closed

Rustfmt is not run if rustup exists, but nightly toonchain/rustfmt-nightly is not installed #1184

Kobata opened this issue Dec 9, 2017 · 1 comment · Fixed by #1204
Labels
A-rustfmt enhancement help wanted

Comments

@Kobata
Copy link
Contributor

Kobata commented Dec 9, 2017

This code will attempt to run rustup run nightly rustfmt in any situation that rustup is available, without fallback to the non-nightly version in the event it fails, for instance due to there not being an installed nightly toolchain.

It appears to have been introduced by #1042

Obviously, there are some known issues with 'old' versions of rustfmt, so prioritizing nightly when possible isn't that bad an idea, however currently there is no fallback of any sort short of preventing bindgen from finding any rustup binary.
@Kobata
Copy link
Contributor Author

Kobata commented Dec 25, 2017

rust-lang/rustup#1294 added a 'proxy' executable for the default path that will (once bundled rustfmt hits a channel) run the appropriate version for the default toolchain.

As such, it seems like the best option as of now is to remove the test for rustup and the alternate run entirely, and rely on whatever rustfmt is on the PATH: Anyone using nightly should (with minor setup) get the nightly version run, and once the new rustfmt hits stable, this test will not need updated to accomodate.

@Kobata Kobata mentioned this issue Jan 1, 2018
Merged
bors-servo pushed a commit that referenced this issue Jan 1, 2018
Auto merge of #1204 - Kobata:rustfmt-pr, r=emilio
e8d5230 
Revert to only calling plain rustfmt

rust-lang/rustup#1294 added a proxy executable for rustfmt that will act on the correct bundled rustfmt for the default release channel set by rustup.

And, assuming I'm reading the expected release notes correctly, it seems like rust 1.23, due out fairly shortly, will add the bundled version of rustfmt to stable.

This fixes #1184
@Kobata Kobata closed this as completed Jan 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-rustfmt enhancement help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Revert to only calling plain rustfmt
2 participants
@fitzgen @Kobata