Skip to content

Disagreement between x.py tidy and rustfmt #74274

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

Closed
Lucretiel opened this issue Jul 12, 2020 · 5 comments
Closed

Disagreement between x.py tidy and rustfmt #74274

Lucretiel opened this issue Jul 12, 2020 · 5 comments
Labels
A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@Lucretiel
Copy link
Contributor

rust/src/libstd/io/buffered.rs

Lines 1136 to 1140 in 9d09331

impl Read for ShortReader {
fn read(&mut self, _: &mut [u8]) -> io::Result<usize> {
if self.lengths.is_empty() { Ok(0) } else { Ok(self.lengths.remove(0)) }
}
}

rustfmt and x.py fmt disagree about the correct formatting of this block. This causes issues with editors configured to "rustfmt-on-save", because you have to keep manually reverting / running tidy. You can see this in the commit log for #72808, which has many commits that unintentionally / accidentally bounce between the two formats:

1bf8ba3
e89e2e4
8df5ae0
f7650fe

@lcnr
Copy link
Contributor

lcnr commented Jul 12, 2020

Isn't this somewhat expected (if unfortunate) as ./x.py fmt is pinned to a specific version of rustfmt and rustfmt has
some slightly changes between different versions?

@Nemo157
Copy link
Member

Nemo157 commented Jul 13, 2020

Specifically see

rust/src/stage0.txt

Lines 19 to 23 in 9d09331

# We use a nightly rustfmt to format the source because it solves some
# bootstrapping issues with use of new syntax in this repo. If you're looking at
# the beta/stable branch, this key should be omitted, as we don't want to depend
# on rustfmt from nightly there.
rustfmt: nightly-2020-04-22

Unfortunately it's pinned to a specific nightly rather than using the previous stable like rustc/cargo do, so using something like a rust-toolchain file to pick the toolchain and integrate with rustup wouldn't fix this problem.

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 8, 2020
@jyn514 jyn514 added the A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust label Sep 13, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 10, 2020

This has now been documented in rust-lang/rustc-dev-guide#883. @Lucretiel do you think that's good enough to close the issue?

@Lucretiel
Copy link
Contributor Author

I'll give it a try and see if it fixes

@Lucretiel
Copy link
Contributor Author

Yes, that's fixed it, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

4 participants