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

rustfmt development, versioning, distribution, etc. #4346

Open
calebcartwright opened this issue Jul 23, 2020 · 9 comments
Open

rustfmt development, versioning, distribution, etc. #4346

calebcartwright opened this issue Jul 23, 2020 · 9 comments

Comments

@calebcartwright
Copy link
Member

calebcartwright commented Jul 23, 2020

During some recent discussions related to (unreleased) breaking changes in rustfmt, several distinct but related topics and questions were raised. Opening this to both summarize those items and track closure, though imagine some may spin off as separate issues and/or be moved elsewhere.

First, some salient points on the current state:

  • rustfmt is versioned independently of rustc (discussed in Rustfmt stability rfcs#2437)
    • rustfmt v1.4.18 is the most recent version to hit nightly
    • rustfmt v1.4.16 the most recent on stable
  • Updates to rustfmt have to work within two strict constraints:
    • Formatting with default config options must align to the Style Guide which can and does change over time
    • The stability guarantee prevents any style/formatting changes within minor/patch updates (more detail in Stability Guarantee/Version Strategy #4286), so we have to mentally attempt to determine if a given change to rustfmt could impact any formatting in any project, and if so that change has to be rejected or gated/made opt-in
  • Version gating (feature flag of sorts) has been used historically to try to avoid introducing formatting changes, but this often adds a lot of additional code and complexity that complicates maintenance
  • rustfmt is distributed and consumed through multiple channels:
  • rustfmt is a submod in rust-lang/rust
  • rustfmt has a heavy reliance on rustc internals which are consumed via rustc-ap (primarily rustc_ast, rustc_span, and rustc_parse)
  • rustfmt is a dependency of rls which uses rustfmt as a lib
    • rls also depends on racer, which also uses the rustc-ap crates, so addressing breaking rustc-ap changes has to be coordinated across three projects to keep the rustc-ap versions in sync within rust-lang/rust

The default branch in this repo has been the active/primary development branch for 8+ months accumulating unreleased bug fixes, features, and some breaking changes. The published 1.x versions of rustfmt have mostly been in maintenance mode during that window and have only received rustc-ap bumps as necessary and the backporting of fixes for some higher impact bugs.


Questions/topics

Below are some of the topics/questions/feedback from recent discussions, and I'll also add that the decision and outcome for several of these questions could directly impact the others.

  • Should rustfmt continue to be versioned independently from rustc?
  • Should rustfmt continue to be distributed and made consumable from crates.io?
  • Default edition for rustfmt versions should not vary from the distribution they ship with (rustc is still 2015 as of 1.45.0 so rustfmt should be as well)
  • Would the subtree approach work better than the submod approach within rust-lang/rust? (insert issue link here)
    • Big pro is the elimination of the rustc-ap reliance
    • Drawback is the loss of ability to distribute rustfmt via crates.io
  • Should style/formatting changes continue to be considered breaking changes?
  • When do we want to allow breaking changes (style/formatting, or otherwise) to be released?
    • How should breaking changes be shared in advance with the community? (we're working on documenting the current set of unreleased breaking changes already)

Finally, I'll add that one concern I have is that I think it's highly likely we'd end up in a scenario where rustfmt would be violating one or the other core constraints if those both remain in place, and rustfmt is not allowed to ship any new major versions outside of an Edition accompaniment. For example, certain changes to the Style Guide would require an update to default formatting in rustfmt that would in turn break the stability guarantee.

@tkaitchuck
Copy link

tkaitchuck commented Jul 27, 2020

Should style/formatting changes continue to be considered breaking changes?
When do we want to allow breaking changes (style/formatting, or otherwise) to be released?
How should breaking changes be shared in advance with the community? (we're working on documenting the current set of unreleased breaking changes already)

One plausible mechanism here is to reduce the scope of the guarantee.

For example the guarantee could be reduced from "Changes to formatting/style are considered breaking"
to "Changes to formatting/style are considered breaking if the would cause code which was formatted by a previous version to be reformatted (Assuming no changes to config/edition)"

This is weaker because the reverse does not hold, IE: an old version for rustfmt might reformat code in the format output by the new version.

Then a new flag --strict could be introduced to reformat according to how the current version would like to see things formated.

Once an addition boundary occurs the scope of what is permitted can be culled.

So an upgrade path would look like this:

rustfmt_v1  # Code gets formatted according to standard 1

rustfmt_v2  # Does nothing. Because while standard 2 is prefered, standard 1 is allowed. The code is compliant with standard 1 so no changes are made.

nano src/lib.rs  # user makes some change to source of lib.rs (does not properly format them)

rustfmt_v2  # lib.rs gets refromatted into standard 2 because it was not correctly formatted according to either standard. The remainder of the code is still formatted to standard 1

rustfmt_v2 --strict  # Reformats all code according to standard 2.

rustfmt_v2.1 --edition 2021  # Standard 2 is now mandatory. (If code were written in standard 1 it would be reformatted)

@calebcartwright
Copy link
Member Author

"Changes to formatting/style are considered breaking if the would cause code which was formatted by a previous version to be reformatted (Assuming no changes to config/edition)"

This is weaker because the reverse does not hold, IE: an old version for rustfmt might reformat code in the format output by the new version.

Thanks for the input! However, that doesn't buy us much in practice, as it's essentially what we're already dealing with today (the reverse already exists in places, for example with newer syntax). The biggest issue by far is that we can't be in a scenario where a newer version of rustfmt formats the code differently than any previous version (i.e. we can never have a cargo fmt -- --check or rustfmt --check failing after an upgrade).

Once an addition boundary occurs the scope of what is permitted can be culled.
So an upgrade path would look like this:

Similarly, this isn't too far off from what has been in place historically since the 1.0 release of rustfmt and it actually adds more complexity with the additional cli flags. This approach ends up creating a mess within the rustfmt codebase over time. Short-lived feature flags work great, but the notion of having feature flags and code live 2-3+ years is just not viable IMO. Plus, don't forget that there's only ever one standard: the Style Guide, and it can evolve independently.

The whole objective with a major version release is to allow us to finally ship such breaking changes (which is in alignment with the original RFC on rustfmt versioning) so shipping a v2 of rustfmt that does the exact same thing as v1 doesn't buy us anything.

One plausible mechanism here is to reduce the scope of the guarantee.

IMO something is going to have to give. I understand why folks have raised concerns about a 2.0 version of rustfmt being released outside a major rust release, but I just don't see how rustfmt can be fully complaint with the stability guarantee (in its current form), and always directly aligned to the Style Guide, and only be allowed to make breaking changes (as currently defined) every few years, all at the same time.

@topecongiro
Copy link
Contributor

Default edition for rustfmt versions should not vary from the distribution they ship with (rustc is still 2015 as of 1.45.0 so rustfmt should be as well)

The problem is that, unlike rustc, users often use rustfmt outside of the cargo context, and having 2015 as the default edition has been confusing to users. If we decide to sync the default edition in rustc and rustfmt, at least we should update rustfmt so that it respects the edition written in Cargo.toml even when it is run directly.

Should style/formatting changes continue to be considered breaking changes?
When do we want to allow breaking changes (style/formatting, or otherwise) to be released?

I have been thinking about removing the style/formatting changes from what we consider as breaking changes. If users need the formatting stability then they can pin to the specific release of rustup.

If this is possible then we can avoid making a major version release after 2.0 (or avoid 2.0 release altogether with some hassle).

@glaubitz

This comment has been minimized.

@calebcartwright

This comment has been minimized.

@glaubitz

This comment has been minimized.

@calebcartwright

This comment has been minimized.

@topecongiro
Copy link
Contributor

It has been a long time since I last posted; I'd like to summarize the current situation (to make sure I remember things correctly) and dump my thoughts.

The current master branch contains breaking changes since the latest release (1.4.22). The scope of breaking changes is broad:

  • Changing the default formatting style
  • Removing command line arguments
  • Removing configuration options
  • Updating rustfmt-as-a-library APIs
    We need a major version update for a release containing these changes, as defined in RFC 2437.

However, a concern was raised that whether we should make a major version update of rustfmt, which happens to be an official tool, in the first place. As such, we should start by creating an RFC (or pre-RFC to the forum?) about the rustfmt 2.0 release. To be honest, I have been reluctant to do so, as I expect the amount of communication to be enormous, and I don't have enough energy and time to handle such communication.

Instead of, or in parallel with, preparing 2.0, we may restore breaking changes and release it as 1.5.

  • Undo breaking changes to command line arguments and configuration options, in particular, the version configuration option
  • Add Version::Three, and make it so that the breaking changes to the default formatting style is only usable when version is set to Version::Three.
    Releasing the master branch as 1.5 is challenging. The number of breaking changes is massive, especially ones related to the default formatting style, so it is not feasible to use version gates as before. One way to do this is to split the rustfmt into separate crates (rustfmt_bin and rustfmt_lib), as we tried once before.

@calebcartwright
Copy link
Member Author

However, a concern was raised that whether we should make a major version update of rustfmt, which happens to be an official tool, in the first place. As such, we should start by creating an RFC (or pre-RFC to the forum?) about the rustfmt 2.0 release. To be honest, I have been reluctant to do so, as I expect the amount of communication to be enormous, and I don't have enough energy and time to handle such communication.

From what I've seen the questions about release 2.0 are more about doing so outside an edition release, and/or doing so without a substantive communication campaign. The latter of which I believe has generally universal agreement, as we wouldn't want to silently drop 2.0 on nightly with nothing but our changelog. I don't think this necessarily would require an RFC just for 2.0 release, but at a minimum we'd want a blog post (e.g. inside rust)

I think we'll want to revisit the stability guarantee more generally first, which would require an RFC (at least IMO), as that will definitely impact the release/versioning aspects.

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

No branches or pull requests

4 participants