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

On managing the stability guarantee in 2.0 and later #4128

Closed
topecongiro opened this issue Apr 19, 2020 · 13 comments
Closed

On managing the stability guarantee in 2.0 and later #4128

topecongiro opened this issue Apr 19, 2020 · 13 comments
Milestone

Comments

@topecongiro
Copy link
Contributor

I would like to discuss how we enforce the stability guarantee in 2.0 and later.

rustfmt must guarantee that the default formatting will not change between versions as long as their major versions remain the same.

During 1.x, we managed it by adding version-gates to code and checking the version at runtime. Unfortunately, this method was prone to bugs and code complexity. In 2.0 and later, we would like to use a different approach to guarantee stability.

Initially, I was thinking of using different versions of rustfmt-lib, depending on which version a user has specified. Unfortunately, it turns out that this method requires some serous rearchitecting of the code. Since we would like to release 2.0 as soon as possible, I am looking for another approach.

One way is to separate the development branch from the release branch. Currently, the development and release of rustfmt are done almost entirely on the master branch. I want to change this to the following way:

  • We will keep using the master branch as a development branch. Every PR will be based on and merged to the master branch.
  • We will create a new branch for release (I will call it release). PRs merged into the master branch that does not change the default formatting will be merged into the release branch; publishment to crates.io and submodule registration to the rust repository will be made only from the release branch.

This way, we can guarantee stability without adding extra code to our development codebase. The downside is that users will lose access to the latest format style unless they build rustfmt from the source. I am expecting that the branch management won't be that bad, though we may need to introduce bot for auto-merging PRs.

@topecongiro topecongiro added this to the 2.0.0 milestone Apr 19, 2020
@calebcartwright
Copy link
Member

The downside is that users will lose access to the latest format style unless they build rustfmt from the source.

Agreed, and that could indeed be problematic for some folks. IMO there's also a benefit for rustfmt with the current feature-gate approach because it allows users to test the new gated style/features and provide feedback. I wonder if in this new model we could publish the "bleeding edge" version of rustfmt (master) as a separate crate and/or as pre-built binaries (perhaps as part of a GitHub pre-Release) to let those users get access to the latest without having to build from source?

I am expecting that the branch management won't be that bad, though we may need to introduce bot for auto-merging PRs

This one I fear could get complicated, at least conceptually, especially if there's a long cycle between major releases. Seems like we could end up needing to keep a mental map of the breaking changes that have been merged to master to be able to address required changes/bug fixes on the released version of rustfmt.

For example: if there's a breaking change that's merged to master, and subsequently a bug is discovered in the current/stable/released version where the bug fix deals with the same source code as that already-merged-to-master breaking change.

IIUC a user that wanted to fix that bug would need to do so directly against the release branch, and against master (very similar to what we are doing today for rustfmt 1.x and 2.x) as well, since we couldn't do the typical merge to master and then releases workflow without potentially introducing the breaking change?

That would probably be manageable as long as the number of breaking changes on master is relatively small, and we don't have breaking/unreleased changes on master for an extended period of time. However, I feel that could be tricky and increase the potential for unintentional breaking changes being released if we do end up going a long time with unreleased breaking changes.

@topecongiro
Copy link
Contributor Author

Thank you for your feedback!

Publishing a bleeding-edge sounds good to me. We can think of many possible installation methods. Currently, for historical reasons, there are two versions of rustfmt on crates.io; rustfmt and rustfmt-nightly. The former is deprecated, and the latter is stable (same as the one you can get via rustup). We can reorganize them so that rustfmt will be the stable version, and rustfmt-nightly will be the bleeding-edge version. To summarize,

  • The stable version can be installed via
    • rustup
    • cargo install rustfmt
  • The bleeding-edge version can be installed via
    • cargo install rustfmt-nightly
    • From the GitHub release page

The divergence between the master and release branches is a concern for me as well. I was wondering whether breaking the development phase could mitigate it. For example, if we define the development period for 2.0 as six months, then in the first four months, we only accept PRs that won't affect the default formatting (e.g., critical bug fixes, updating syntax and issues related to configuration). After that, we will be open to PRs that will change the default formatting.

@calebcartwright
Copy link
Member

calebcartwright commented Apr 21, 2020

The stable version can be installed via
    rustup
    cargo install rustfmt
The bleeding-edge version can be installed via
    cargo install rustfmt-nightly
    From the GitHub release page

SGTM!

For example, if we define the development period for 2.0 as six months, then in the first four months, we only accept PRs that won't affect the default formatting (e.g., critical bug fixes, updating syntax and issues related to configuration). After that, we will be open to PRs that will change the default formatting

That sounds reasonable, and the shorter cycle should definitely help keep things manageable. I suppose we could also consider having something of a vNext branch so that folks who really wanted to work on a breaking changing during that first ~4 months could base their changes on too if we find there's a lot of breaking changes being proposed/submitted.

@topecongiro
Copy link
Contributor Author

The idea of preparing a branch for breaking changes seems reasonable to me. My first thought was to simply reject PRs during the first period, though that leaves a bad feeling to contributors who are unfamiliar with the rule. I would like to avoid it if possible.

@calebcartwright
Copy link
Member

As discussed over in #4154, another thing we'll probably need to address is how to provide documentation on available configuration options with this new model.

I've seen a few questions now in Discord and in GH related to Configuration options as described in-repo vs. what's actually available for folks using a released rustfmt version.

I.e. what configuration options are available on

  • bleeding edge rustfmt/rustfmt in source
  • rustfmt on stable channel
  • rustfmt on nightly channel

@topecongiro
Copy link
Contributor Author

The following is a summary of how we manage rustfmt development after 2.0:

  • Make the master branch a release only branch
  • Create a new develop branch; most of the rustfmt development will happen in this branch
  • Divide the 2.x development period into two parts; the non-breaking change period and the breaking-change period. During the former period, the develop branch will only accept Prs that do not change the default formatting style.
  • Create a new 3.0-rc branch; this branch will keep breaking changes from PRs sent during the non-breaking-change period. We limit the PRs merged to this branch to those with small diff. Anything involving extensive refactoring, for example, is only accepted during the breaking-change period to ease the sync cost between this and the develop branch. We merge this branch into the develop branch at the beginning of the breaking-change period.

rustfmt-branching-model

@topecongiro
Copy link
Contributor Author

Ah, this does not quite work...I'll have to think again.

@topecongiro
Copy link
Contributor Author

As for the documentation, I think it would be good to enrich the GitHub Page and make it so that you can specify which version to display like the CMake documentation page.

@calebcartwright
Copy link
Member

Something else we should consider is adding some additional target repositories to use for our integration tests (perhaps even rustc and/or tokio) to get us some extra coverage and validation.

Related aside, we're currently re-building rustfmt in each integration test job, so it may also be worth seeing if we can build the binary once and then just using that rustfmt binary in each job run (seems it could help speed them up)

@topecongiro
Copy link
Contributor Author

Currently, cargo publish to crates.io is blocked by rust-lang/cargo#8280. We may need to find a workaround for this.

@topecongiro
Copy link
Contributor Author

@calebcartwright I have rethought how we manage versioning and branches on rustfmt 2.0 and later. Let me hear your thought.

To conclude, I'd like to create a branch for 2.y.z releases (let's say release/2. The master branch will still be the development branch.

First of all, I've ditched the idea of the complicated branch management that I mentioned before. We don't have enough time or human resources to do anything complicated unless it's necessary. I also want to make it easier to move on to other management methods is the new management method doesn't work.

The master branch will be a development branch as before. What' s different is that we no longer release 2.y.z versions from the master branch but the relaese/2.

The release/2 branch is for 2.y.z versions release. We cherry-pick commits from the master branch to the release/2 branch. We restrict the commits to cherry-pick to bug fixes and syntax updates. Commits that introduce new features can be cherry-picked as well, but this is not a must-do.

As the difference between the master branch and the release/2 branch becomes more significant, it is more likely that we observe merge conflicts and auto-merging is not possible. In that case, depending on the severity of the fix, we'll either resolve the conflict manually or pass on the fix until 3.0.

I've come to this conclusion because I want to keep the development easy and straightforward.

The downside is that delivering new features may be delayed once the difference between the master and release/2 is significant enough that auto merging the cherry-pick becomes impossible. I am willing to take this risk to make our life easier

@calebcartwright
Copy link
Member

To conclude, I'd like to create a branch for 2.y.z releases (let's say release/2. The master branch will still be the development branch.

First of all, I've ditched the idea of the complicated branch management that I mentioned before. We don't have enough time or human resources to do anything complicated unless it's necessary. I also want to make it easier to move on to other management methods is the new management method doesn't work.

Makes sense and SGTM. I generally prefer to use feature flags over separate branches, but understand the desire to move away from version and try something different. Sounds like a key to this (or any other strategy) will be to minimize the length of time we accrue unreleased breaking changes.

Do you feel like we've settled on the repository/module layout for 2.0 and going forward? It seems like we are, but obviously additional restructuring would complicate cherry picking commits.

@topecongiro
Copy link
Contributor Author

I think we have, and hence I am going to close this issue. I will add documentation about branch and version management in Wiki when I have time.

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

2 participants