Repository navigation
Breaking changes in rustfmt with 1.45.0 #74443
Description
This seems to be a re-occurrence of #73078. The rustfmt released with 1.45.0 no longer respects #![cfg_attr(rustfmt, rustfmt_skip)] which breaks our CI for some auto-generated files.
Reacted by Stepan Koltsov
Activity
[-]Breaking changes in rustfmt with 1.44.0[/-][+]Breaking changes in rustfmt with 1.45.0[/+]Update to latest version of the protobuf library
calebcartwright commentedon Jul 19, 2020
Looks like 1.45.0 shipped with an older version of rustfmt
Not sure how that happened, but rustfmt v1.4.16 is the version that had the fix and both 1.4.14 and 1.4.15 had the bug from #73078
JohnTitor commentedon Jul 19, 2020
I think we missed a beta backport, as mentioned in https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/rustfmt.20in.201.2E45.2E0.
Mark-Simulacrum commentedon Jul 19, 2020
"Missed" sort of implies that someone had raised it for beta backport, which I think never happened -- it seems like we just didn't consider a beta backport at all. I would not be opposed to doing a 1.45.1 release personally with the backport, cc @rust-lang/release for opinions.
We should, regardless, make sure that at least rustfmt 1.4.16 is in beta and nightly today.
jonas-schievink commentedon Jul 19, 2020
👍 for backport + stable patch release
calebcartwright commentedon Jul 19, 2020
nightly is on rustfmt 1.4.18 (though broken/unavailable ATM) and I believe beta is as well. Let us know if anything is needed from the rustfmt side though.
miketimofeev commentedon Jul 20, 2020
Is there any chance of a new release with rustfmt 1.4.16 or even 1.4.17 today or tomorrow?
Mark-Simulacrum commentedon Jul 20, 2020
It won't be that quick, for sure, but I am currently thinking Tuesday or Thursday next week. I'm hoping to prepare artifacts and such today or tomorrow.
Mark-Simulacrum commentedon Jul 20, 2020
Okay I looked at which rustfmt release to promote to 1.45.1 and wasn't able to find a good candidate. I think we'll want the rustc-ap crates to be at version 659 -- but no rustfmt release has that. @calebcartwright @topecongiro -- could one of you take a look? If you'd like, you can also post a PR against rust-lang/rust stable branch which bumps rustfmt to whatever commit you think is the right one to release.
calebcartwright commentedon Jul 20, 2020
IIRC rustfmt v1.4.17 has v659 of the rustc-ap crates, though may need to find the right pairing of rustfmt and rls that'll work there
Mark-Simulacrum commentedon Jul 20, 2020
Initial draft is up at #74574, still has a few things left but I'm hoping to get that landed in the next few days so we can have artifacts ready for testing.
calebcartwright commentedon Jul 21, 2020
@Mark-Simulacrum - the same thing appears to have happened with rls, as 1.45.0 has an older version of rls than 1.44.1
I'm thinking the same combination of rls/rustfmt from 1.44.1 should be fine together:
rls 1.41.0 (7d3aba2 2020-06-17)andrustfmt v1.4.16.rustfmt v1.4.16 was released specifically for the 1.44.1 patch, whereas rustfmt v1.4.17 was intended to to do the same for nightly but was never actually merged to master because there was a broken toolstate issue that occurred around the same time (refs #73113). The only difference between these two rustfmt versions is rustc-ap v654 for rustfmt v1.4.16 and rustc-ap v659 for rustfmt v1.4.17
There may be some other combinations of rls/rustfmt that could work but I'd think the same ones from 1.44.1 would be safest.
Mark-Simulacrum commentedon Jul 21, 2020
Okay, I'll try that tomorrow, cherry-picking exactly 1.44.1 RLS and rustfmt to the stable branch. FWIW 1.4.17 does seem to work fine on the current PR (at least toolstate is green for RLS and rustfmt).
calebcartwright commentedon Jul 21, 2020
With which version of rls? From the rustfmt perspective 1.4.16 vs. 1.4.17 doesn't really matter (they're basically identical), so as long as the rustc-ap versions line up with rls then IMO which of those two versions of rustfmt to use would just be a be a matter of whatever's easiest.
Mark-Simulacrum commentedon Jul 21, 2020
Ah okay then I'll probably leave things as is, since the current PR leaves RLS as-is (which seems good) and only bumps rustfmt, without duplicating any of the autopublished crates.
Mark-Simulacrum commentedon Jul 21, 2020
Although, wait, no, you're saying we bumped RLS in 1.44.1 too... gah. Okay, I'll need to think about this some more tomorrow, I'm so confused how we're seemingly bumping things in both directions here :/
calebcartwright commentedon Jul 21, 2020
Yeah the rustc-ap shuffle can be a real pain sometimes. In fairness, the version of rls that shipped with v1.44.1 seems to be chronologically newer, though I'm not sure what the actual expected/desired version for rls in 1.45.0 would be.
@Xanewok what version of rls were you expecting with rust 1.45.0?
Here's what I'm seeing
rustc 1.44.0 --> rls 1.41.0 (2659cbf 2020-04-14)
rustc 1.44.1 --> rls 1.41.0 (7d3aba2 2020-06-17)
rustc 1.45.0 --> rls 1.41.0 (085f24b 2020-05-28)
calebcartwright commentedon Jul 29, 2020
Sure others have already validated, but just wanted to confirm that the rustfmt version in the 1.45.1 prerelrease looks good and resolves the issue 👍
calebcartwright commentedon Aug 6, 2020
Can this be closed now?
cuviper commentedon Aug 6, 2020
Yes, thanks!