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.

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

preparation for potential rustfmt 1.4.19 #4283

Merged
merged 5 commits into from Jul 21, 2020

Conversation

calebcartwright
Copy link
Member

@calebcartwright calebcartwright commented Jun 27, 2020

We don't have any explicit plans for another rustfmt 1.x release, but opening this against a 1.4.19 branch proactively in case we do end up having to do another release.

This includes a bump of the rustc-ap-* crates to v668 v669 which had some minor breaking changes, and also backports the fix from #3975 to ensure that fix will be in the next released version of rustfmt regardless of whether that's a 1.x or 2.x release (Refs #4282 and #3974)

@calebcartwright
Copy link
Member Author

Have bumped the rustc-ap crate versions to the latest (v668) should we want to see if bumping the crates helps with rust-lang/rust#74081

@calebcartwright
Copy link
Member Author

@topecongiro - think we'll need to proceed with another 1.x release to fix the broken toolstate on nightly. There was discussion today in the wg-rustfmt Discord channel with some questions/concerns raised about when/how the breaking 2.0 changes should be released that warrant some additional discussion (cc @Mark-Simulacrum @Manishearth in case I've summarized anything inaccurately)

In the meantime I'm going to work on a writeup of all the changes coming with 2.0, but please let me know if there's anything else I can do to help!

@topecongiro
Copy link
Contributor

Thank you for letting me know, I have not logged in to Discord for a while.

@calebcartwright
Copy link
Member Author

calebcartwright commented Jul 15, 2020

Sure! I forgot to actually summarize the discussion sorry 😆, but basically there were questions whether the rustfmt 2.0 update should be done as part of a 2021 edition rollout, and/or discussed at a dev tools meeting and/or if released independently of an edition change that we'd want to at least have a blog post that described the what/why of the changes that could be posted on Inside Rust

@Mark-Simulacrum
Copy link
Member

It sounded to me like rustfmt 2.0 wouldn't fit too well into edition structure because it's not a case where we can effectively ship both 1.x and 2.x indefinitely. I do hope that as we figure out how to ship 2.0 we can identify ways of preventing need for 3.0 in the future (or determining how to communicate such to the community at large).

@calebcartwright
Copy link
Member Author

#4286 also contains some relevant discussion on versioning and the formatting stability guarantee

@topecongiro
Copy link
Contributor

@Mark-Simulacrum Hmm, so you are saying that you prefer not to make any major version update to rustfmt altogether?

@Mark-Simulacrum
Copy link
Member

Well - I think that is definitely my preference wherever possible with regards to Rust tooling. I think my impression based on very brief discussions so far is that 2.0 at this point is inevitable (but I'd love to be proven wrong). I believe jt largely consist of two groups of things: some formatting changes, but nothing too significant (and obtainable already on 1.x by setting version = 2 in config) and the CLI changes.

I hope that without too much trouble we can remove breaking CLI changes (e.g. deprecating options into doing nothing for example instead of removing), and switch defaults in the next edition.

But, I am speaking from an uninformed position - as I mentioned in today's discussion, I think an RFC of sorts documenting in more detail what the breaking changes are (from a user's perspective) and the rationale for making them (and alternatives, if they exist).

I don't want us to throw away any work on rustfmt though so I'm hopeful we can find a way to switch to the 2.x branch quite soon.

@calebcartwright
Copy link
Member Author

I feel like there will be conflicting interests between trying to gate breaking changes on editions and ensuring that rustfmt's defaults are aligned to the style guide, all while maintaining the stability guarantee.

The style guide's a living document and does change from time to time, and some of the formatting changes that are packed into rustfmt 2.0 are aligning to style guide changes, for example sorting and empty match arms.

If we can only have rustfmt's defaults break the stability guarantee with edition changes then I feel like we'd almost certainly end up in a scenario where rustfmt's defaults would not align to explicit requirements in the style guide, and folks would have to override config options to get rustfmt to format according to the style guide. Being in a state like that for months/years doesn't seem like it'd be tenable either.

Not sure the best place to have the discussion (RFC or otherwise), though this PR may not be the best place as we'll need to close it as part of fixing the current broken tool state on nightly.

@Mark-Simulacrum
Copy link
Member

Yeah so styling changes seem largely fine to me - they're not even really breaking IMO, more of an annoyance to manage until it hits stable.

@calebcartwright
Copy link
Member Author

Yeah so styling changes seem largely fine to me - they're not even really breaking IMO, more of an annoyance to manage until it hits stable.

A loosening of the stability guarantee would certainly open up some options as well, but right now it's a pretty hard constraint we have to work within. Currently we have to evaluate every patch to rustfmt through the lens of "could this patch cause any formatting change to any part of any codebase", which is not always the easiest mental exercise.

This was previously handled through version gating, but as those accumulate over time it really starts to weigh heavy on the rustfmt source.

Unfortunately I'm skeptical there's an easy/obvious solution for this that will satisfy all concerns and current constraints in place. Perhaps for next steps, I can capture some of the discussed options with their respective pros & cons as a starting point for a broader discussion, RFC, or however we want to proceed.

Curious what your thoughts are @topecongiro

@topecongiro
Copy link
Contributor

As @calebcartwright mentioned, we are planning to release rustfmt 2.0. The release will include breaking change; specifically, we will update the default formatting style/add and remove configuration options/update CLI /change the default behavior (non-recursive by default)/etc. Some changes are inevitable (fixing the default formatting style must happen at some point), while others are rather just improvements or fixing the rushed part before releasing 1.0.

To be honest, I am surprised that releasing a new major version for rustfmt involves RFC or whatnot. I was even assuming that we will do a major version release every 1-2 years, though that clearly seems unacceptable from the core team's point of view. Is there any piece of information that describes the specification the 'official' rust tools must follow?

@Mark-Simulacrum Would you mind elaborating on what kind of breaking changes you are most concerned with? Or is it that rustfmt and rustc having a different major version? I would like to know so we can look for an alternative for making a major version bump if that's necessary.

@Mark-Simulacrum
Copy link
Member

To be clear, there's no "core team opinion" here -- I'm speaking just for myself.

My primary concern is that -- historically -- we have tried to be very cautious about breaking changes since 1.0, without a major version bump for any of the tools shipped on stable. That said, it does sound like in the rush to get rustfmt 1.0 out the door, perhaps the CLI and related tooling was not as well-tested and such that we can reasonably expect to support 1.0 indefinitely, like we do for rustc/cargo/rustdoc (at least).

I don't really have specific concerns with any part of the breaking changes, more so that I want to be careful about how we roll them out. I think at the very least we need a plan for communicating these changes (e.g., a blog post) and likely after that's ready some discussion amongst the dev-tools team.

@Manishearth
Copy link
Member

Yeah, I'm speaking in my personal capacity here.

@Mark-Simulacrum a thing I had missed was that the official rustfmt RFC does allow for there to be a 2.0. rust-lang/rfcs#2437

I still think a 2.0 should involve an RFC, so that the community gets a chance to get notified and opine on how it should be carried out. I don't actually care about the changes themselves (though I will note that config entry renames can be done by having a temporary deprecated alias in rustfmt for a while, this is what clippy does).

Tools have typically had a more lax stability policy: e.g. Clippy is not stable under deny(warnings). But still, this will break a lot of CI, perhaps in an unfixable way if people are using multiple versions of rust. So we should tread carefully.

@calebcartwright
Copy link
Member Author

@topecongiro are you okay with merging this and releasing rustfmt v1.4.19? racer's been updated and published with the same version of the rustc-ap crates (racer-rust/racer#1118 (comment)) so pending on us now

@topecongiro
Copy link
Contributor

@Mark-Simulacrum @Manishearth Thank you for the feedback. I was instead feeling ok to make disruptive changes on a major version update, but I am reconsidering.

We should probably start by posting documentation about the 2.0 release to the blog or forum and see how the community feels? @calebcartwright Perhaps you've already started writing?

@topecongiro
Copy link
Contributor

@calebcartwright Sorry for being late; I will release the new version today.

@topecongiro topecongiro merged commit cef1c0d into rust-lang:rustfmt-1.4.19 Jul 21, 2020
@Manishearth
Copy link
Member

Sounds good!

@calebcartwright calebcartwright deleted the rustfmt-1.4.19 branch July 21, 2020 15:43
@calebcartwright
Copy link
Member Author

@calebcartwright Perhaps you've already started writing?

Have started collecting things yes, though we've got 8+ months worth of PRs plus all the existing changes that were version gated so this will take a while. Will provide an update once I've something to share for review

@topecongiro
Copy link
Contributor

Awesome, thank you! We are not in a hurry, just wanted to confirm before I started to work on it to avoid duplicated work.

@calebcartwright
Copy link
Member Author

Semi-related, could we create a tag and GH release for v.1.4.19? I'd like to see the new GHA Workflow in action with the pre-built binaries 🚀

Happy to give it a try myself (haven't actually checked to see if I have perms), but wanted to check first to make sure we weren't holding off on new tags for any reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants