-
Notifications
You must be signed in to change notification settings - Fork 440
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
Define backwards compatibility policy for rules_rust
#832
Conversation
CC @philwo |
I will not submit before @illicitonion, @dfreese, and @UebelAndre approve. @damienmg already approved offline. |
Feel free to comment, propose a change, or request more information if something is unclear :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM - thanks for putting this together!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
#847 for an example of an incompatible flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small questions but looking really good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only have one more question-ish thing open but I like what's here and can approve once that conversation is resolved 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, thanks for all the work you put into this @hlopko! Very much appreciated!
Co-authored-by: UebelAndre <[email protected]>
Co-authored-by: UebelAndre <[email protected]>
Co-authored-by: UebelAndre <[email protected]>
Co-authored-by: UebelAndre <[email protected]>
Co-authored-by: UebelAndre <[email protected]>
Co-authored-by: Daniel Wagner-Hall <[email protected]>
Co-authored-by: Daniel Wagner-Hall <[email protected]>
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider it just a nit, but otherwise LGTM. thanks.
Co-authored-by: David Freese <[email protected]>
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
1 similar comment
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
1 similar comment
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
I believe this was baking for long enough. Thank you all for your contributions and review! |
Introduction
This document proposes a backward compatibility policy for
rules_rust
and tries to find a balance between:As of 2021-07-08 the Bazel backward compatibility policy can be summarized as:
Proposed policy
See the added
COMPATIBILITY.md
file.Alternatives Considered
Match Bazel versioning
Instead of using SemVer,
rules_rust
could follow Bazel's versioning scheme. Major version number will match the Bazel LTS version supported. Minor version number will be increased on breaking changes, patch version number will be increased on backwards compatible changes.I don't feel strongly about this or SemVer. I picked SemVer because that is what is the standard in the Rust ecosystem, but I'm open to doing the standard thing in the Bazel ecosystem as well :)
Supporting past releases
Current proposal says the
rules_rust
are developed for the latest Bazel LTS (and the latest Bazel rolling release on the best effort basis). It does not promise old releases will be maintained.For users wanting to use Bazel LTS for the whole lifetime of it (not only the active phase that is typically 9 months, but also the inactive phase that is up to 3 years, in which the LTS will receive maintenance updates) supporting LTS for 9 months is not enough.
When a new Bazel LTS becomes active,
rules_rust
could create a branch for the old LTS as they create a newrules_rust
release with the major number incremented. Even better, whenrules\_rust
drop compatibility with an LTS release we could create a branch.rules_rust
would then cherry pick and create patch releases for inactive LTS for critical bug fixes, OS compatibility patches, and security patches. No new features will be cherry picked.If the community and maintainers of
rules_rust
are willing to make this promise, I'm open to that.Supporting all Bazel rolling releases of the current epoch
This approach is inspired by the Bazel LTS releases, but promises more stability. This approach maintains SemVer, and each minor version release of the
rules_rust
aims to support all Bazel rolling releases of the given major version number. In other words, a release ofrules_rust
marked as 4.X would be backwards compatible with all 4.Y releases where X > Y, and always compatible with Bazel 4.0 LTS, and all 5.0.0pre Bazel rolling releases for all Z releases released before release 4.X ofrules_rust
. When Bazel releases 5.0 we'd also release 5.0 which does not promise backwards compatibility with 4.x. New features will be made available to all Bazel versions.This approach was dismissed because of its maintenance overhead. Since Bazel promises no backwards compatibility between rolling releases,
rules_rust
might find themselves in a situation where they would have to release multiple implementations of the rules, and depending on the Bazel version pick one that works. We have to be prepared to do this for Bazel LTS releases (in the past we used aBAZEL_VERSION
check in a repository rule that conditionally generated a.bzl
file with implementations of functions for that Bazel version that were used by the rules in analysis). Having to do the same for each Bazel rolling release is too big of a burden - as it also was for the Bazel team, and the result was the current backwards compatibility policy involving LTS and rolling releases that are backwards incompatible.Second downside of this approach is the lost control over when we can make backward incompatible releases of
rules_rust
. We want to reserve the right to be pragmatic and decide to make a backward incompatible change as a reaction to a substantial Bazel backward incompatible change.Supporting the current Bazel rolling release
Currently the proposal doesn't promise strict compatibility with the current Bazel rolling release. The compatibility is only on the best effort basis.
Bazel rolling releases are planned to be released rather often (approximately weekly), and each can potentially break
rules_rust
. Promising strict compatibility with the current Bazel rolling release can create frequent high priority wor.rules_rust
maintainers would have to be able to potentially every week stop what they were doing and fix Bazel backward incompatible changes (and do so in a way that is backward compatible with Bazel LTS) to be able to continue regular rules development (to have a green CI) and to be able make new releases.I believe maintainers of
rules_rust
don't have the capacity to make this promise responsibly.Fixes #600.