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.

Sign up for GitHub

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

Suggest MSRV policy #227

Closed
wants to merge 4 commits into from
Closed

Suggest MSRV policy #227

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
48b75c5
Suggest MSRV policy
matklad Nov 6, 2020
f2acecc
Mention that bumping major on MSRV is not bad
matklad Nov 6, 2020
830cfc0
Apply suggestions from code review
matklad Nov 7, 2020
38e610d
Exclude tests from MSRV testing recipe
matklad Nov 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/checklist.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
- **Necessities** *(to whom they matter, they really matter)*
- [ ] Public dependencies of a stable crate are stable ([C-STABLE])
- [ ] Crate and its dependencies have a permissive license ([C-PERMISSIVE])
- [ ] Crate has an MSRV policy ([C-MSRV])


[C-CASE]: naming.html#c-case
Expand Down Expand Up @@ -139,3 +140,4 @@

[C-STABLE]: necessities.html#c-stable
[C-PERMISSIVE]: necessities.html#c-permissive
[C-MSRV]: necessities.html#c-msrv
39 changes: 39 additions & 0 deletions src/necessities.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,42 @@ cannot be used in some situations where most of the Rust runtime stack can.
The license of a crate's dependencies can affect the restrictions on
distribution of the crate itself, so a permissively-licensed crate should
generally only depend on permissively-licensed crates.

<a id="c-msrv"></a>
## Crate has an MSRV policy (C-MSRV)

A crate should clearly document its Minimal Supported Rust Version:

* Which versions versions of Rust are supported now?
* Under what conditions is MSRV increased?
* How are MSRV increases reflected in the semver version of the crate?

Compliance with a crate’s stated MSRV should be tested in CI.

The API guidelines tentatively suggest that, for libraries, an MSRV increase should
*not* be considered a semver-breaking change. Specifically, when increasing
MSRV:

* for crates past `1.0.0`, increment minor version (`1.1.3` -> `1.2.0`),
* for crates before `1.0.0`, increment patch version (`0.1.3` -> `0.1.4`).

This reduces the amount of ecosystem-wide work for MSRV upgrades and prevents
incompatibilities. It also is a de-facto practice for many cornerstone crates.
This policy gives more power to library consumers to manually select working
combinations of library and compiler versions, at the cost of breaking `cargo
update` workflow for older compilers.

However, do not increase MSRV without a good reason, and, if possible, batch
MSRV increases with semver-breaking changes.

Nonetheless, some crates intentionally choose to treat MSRV increases as a semver
breaking change. This is also a valid strategy, but it is not recommended as the
default choice.

To reliably test MSRV on CI, use a dedicated `Cargo.lock` file with dependencies
pinned to minimal versions:
matklad marked this conversation as resolved.
Show resolved Hide resolved

```bash
$ cp ci/Cargo.lock.min ./Cargo.lock
$ cargo +$MSRV test
Copy link
Member

@jyn514 jyn514 Nov 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually the first good usecase I've seen for RUSTC_BOOTSTRAP.

Suggested change
To reliably test MSRV on CI, use a dedicated `Cargo.lock` file with dependencies
pinned to minimal versions:
```bash
$ cp ci/Cargo.lock.min ./Cargo.lock
$ cargo +$MSRV test
To reliably test MSRV on CI, use `-Z minimal-versions`:
```bash
$ RUSTC_BOOTSTRAP=1 cargo +MSRV -Z minimal-versions test
```

That said, maybe it's not a good idea to recommend it so prominently - it might be possible to find the equivalent nightly but it sounds annoying to do.

https://doc.rust-lang.org/cargo/reference/unstable.html#minimal-versions

Copy link
Member

@taiki-e taiki-e Nov 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely do not recommend using RUSTC_BOOTSTRAP + -Z minimal-versions:

  • -Z minimal-versions is added in cargo 1.27. It does not work on older versions. (rust-lang/cargo@9a09892)
  • If dependencies are incompatible with -Z minimal-versions, compile will fail for reasons unrelated to MSRV.
  • It is an unstable feature, so may be changed in the future.

(Note that dev-dependencies may raise version requirements, so running test with -Z minimal-versions may not work properly for its intended purpose. Ideally, it needs to run cargo update -Z minimal-versions after completely remove the dev-dependencies. See tokio-rs/tokio#3131 (comment) for more.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I concur that manually updating Cargo.lock.min at this moment is likely to require less maintenance. But -Z minimal-versions is clearly an interesting alternative, I bet someone will learn about it from this comment!

```