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

[release-0.13.x] chore(Cargo.toml): Better document direct deps #495

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

polarathene
Copy link
Collaborator

@polarathene polarathene commented Nov 5, 2023

These are improvements from an extensive investigation I recently did to simplify maintenance of lockfile generation for the CI to benefit from, while respecting the MSRV.

Commit message:

Click to view
  • Slight adjustments to the version fields for compatibility with cargo +nightly update -Z direct-minimal-versions with the MSRV of 1.56.0.
  • Add rust-version field for leveraging cargo +nightly update -Z msrv-policy to generate a lockfile that respects the MSRV, and the benefit of downstreams.
  • Better communicate why dev-dependencies are required (identify associated examples / tests).
  • Avoid repeating deps again in in dev-dependencies.
  • Raise fixed warp dev dep to an MSRV compatible version (for a common tokio-util implicit dep version). Simplifies CI lock maintenance.

@polarathene
Copy link
Collaborator Author

polarathene commented Nov 5, 2023

More info

My verbosity is an issue for some, I've split this off and tried to condense / organize it. It's purely for reference to support and document the changes in the PR should anyone need more context.

Producing a compatible Cargo.lock for the MSRV

# Resolve a `Cargo.lock` with the latest compatible deps for `rust-version` (1.56.0):
cargo +nightly update -Z msrv-policy

# Minor corrections due to deps lack of proper `-Z msrv-policy` support for the MSRV
cargo update --package memchr --precise 2.5.0
cargo update --package ordered-multimap --precise 0.4.0
cargo update --package tokio-util --precise 0.7.8
Validating minimum versions build for MSRV (with 1 less pin)
# For verifying minimum versions of direct deps build for the MSRV is supported:
# `tokio ="1.27"` (or lower) implicitly prevents needing to pin tokio-util to 0.7.8 (as newer releases require a newer tokio)
cargo +nightly update -Z direct-minimal-versions -Z msrv-policy
cargo update --package memchr --precise 2.5.0
cargo update --package ordered-multimap --precise 0.4.0

The following passes successfully with a Cargo.lock generated from the above:

cargo +1.56.1 check --tests
cargo +1.56.1 clippy --locked --offline --all-targets --all-features -- -D warnings

Supplementary info on -Z msrv-policy and pinned versions

Click to view

-Z msrv-policy only works on nightly, but ideally resolves within the deps semver range a version that was last known as compatible with rust-version in Cargo.toml.

  • This differs from the default resolver which will attempt to satisfy the latest version in semver regardless of rust-version compatibility.
  • This also differs for older toolchains that don't understand newer Cargo.toml syntax like introduced in Rust 1.60.0.
    • Those would ignore such published crates and resolve to an earlier version of the crate implicitly. These pins are the same versions that would be resolved regardless for the MSRV.
    • memchr 2.6.0 is an implicit dep to quite a few crates in Cargo.lock, including the explicit nom 7 dep. memchr only adopted rust-version (1.60) since the 2.6.1 release, hence -Z msrv-policy is ineffective at resolving without a pin, while the MSRV toolchain resolver would resolve a compatible version as anything newer is considered invalid.
    • ordered-multimap from rust-ini has the same problem, except it's history is more complicated and documented in detail in this comment (ahash is the issue).
    • tokio-util is probably better pinned, that is easier by raising the explicitly pinned warp patch version in Cargo.toml. This is also a bit complicated, but is an example of when rust-version is not correctly maintained.
Additional context on deps

Minimum version:

  • async-trait: Minimum of 0.1.2 required to support examples/async_source.

MSRV bumps:

  • toml: Newer releases after 0.5.x series require MSRV 1.60 to 1.66, while the latest 0.8.6 release requires MSRV 1.67.
  • ron: MSRV raised to 1.64 with 0.8.1.
  • rust-ini: MSRV raised to 1.64 with 0.19.0, while 0.20.0 (latest) raises again to MSRV 1.71.1. Both due to ordered-multimap.
  • indexmap: Next major 2.0.0 raises MSRV to 1.63.
  • notify: The next major (5.0.0) keeps the 1.56 MSRV, while the latest 6.0.0 major raises that to MSRV 1.60
  • temp-env: Newer minor release adopts rust-version since 0.3.4 (latest 0.3.6 with MSRV 1.62.1).

Without -Z msrv-policy

The following will also support building without generating Cargo.lock with -Z msrv-policy. The versions need to be pinned (in either Cargo.toml or Cargo.lock).

Click to view
# These are the dev-dependencies with versions that last supported MSRV `1.56`

# Direct deps
tokio = { version = "1.29.1", features = ["rt-multi-thread", "macros"] } # latest 1.33 requires MSRV 1.63
chrono = { version = "0.4.26", features = ["serde"] } # newer versions raise MSRV to 1.57
reqwest = "0.11.15" # newer release bumps MSRV to 1.57 (latest 0.11.22 with MSRV 1.63)

# Implicit deps:
scoped-tls = "1.0.0" # 1.0.1 (latest) adds rust-version 1.59
byteorder = "1.4.3" # 1.5.0 (latest) adds rust-version 1.60 
log = "0.4.18" # newer versions add rust-version 1.60 (latest 0.4.20)
tempfile = "3.6.0" # newer versions raise to MSRV 1.63 (latest 3.8.1)
h2 = "0.3.20" # newer versions raise to MSRV 1.63 (latest 0.3.21)
tokio-util = "0.7.8" # newer versions incorrectly claim MSRV 1.56 (latest 0.7.10 with MSRV 1.63)

Alternatively pinning via Cargo.toml could avoid = pinning and instead rely on using -Z direct-minimal-versions. This requires extra pins due to the nightly toolchain resolver.

# Required only for nightly toolchain (eg: when using `-Z direct-minimal-versions`)
# For nom:
memchr = "2.5.0" # newer versions raise MSRV, but only set rust-version at 2.6.1, (latest 2.6.4 with MSRV 1.61)
# For rust-ini:
ordered-multimap = "0.4.0" # newer versions raise MSRV to 1.60 (latest 0.4.3 -> ahash 0.7.7)
# For json5:
# 2.5.7 for both pest deps is the last version supporting MSRV 1.56 (latest 2.75 with MSRV 1.61)
# (Avoidable with `-Z msrv-policy)
pest = "2.0.2"
pest_meta = "2.0.3"

The MSRV toolchain will resolve memchr and ordered-multimap correctly, thus they don't need to be pinned.

  • Obviously the above isn't any better, but is shared to contrast against the -Z msrv-policy approach.
  • 👎 Implicit deps (even when only for dev-dependencies / CI usage) should avoid being in Cargo.toml.
  • I've documented the latest MSRV for the semver range of each dep, which better illustrates the motivation of this recent PR (raising MSRV to 1.63 to keep the CI happy).

CI uses MSRV 1.59, while this PR supports MSRV 1.56

Click to view

Note that presently the CI is testing for an MSRV of 1.59, while the README (for this branch) presently documents 1.56 as the MSRV.

  • It was presumably bumped in the past to make the CI pass (seems to be a common pattern for config-rs?) without resolving a compatible lockfile (discussed recently as how MSRV bumps are being determined).
  • This PR enables maintaining the original MSRV of 1.56.0 (or if preferred newer).
    • rust-version is set to communicate the actual minimum that config-rs will build with.
    • hashbrown 0.14.2 has done similar. Their rust-version is only supported if you additionally pin a dep, or opt-out of a default feature that enables it.

@polarathene polarathene changed the title chore(Cargo.toml): Better document direct deps [release-0.13.x] chore(Cargo.toml): Better document direct deps Nov 5, 2023
@@ -10,6 +10,7 @@ authors = ["Ryan Leckey <[email protected]>"]
categories = ["config"]
license = "MIT/Apache-2.0"
edition = "2018"
rust-version = "1.56.0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As noted by #483 (comment)

  • This should only reflect what the project can build successfully with (even if that requires managing a Cargo.lock with some pinning).
  • It should only be raised in future for explicit changes to the project (not from implicit changes from deps semver raising MSRV):
    • Adding/Upgrading deps in Cargo.toml where pinning cannot maintain support for rust-version.
    • Introducing changes that require features from newer toolchains.

When it comes to actual support / maintenance the MSRV policy of the project may differ. Raising rust-version is a more granular edition field, but with support to opt-out / ignore it by downstream.

@ijackson

This comment was marked as resolved.

- Slight adjustments to the version fields for compatibility with
  `cargo +nightly update -Z direct-minimal-versions` for MSRV `1.56.0`
- Add `rust-version` field for leveraging 
  `cargo +nightly update -Z msrv-policy` to generate a lockfile that
  respects the MSRV, and the benefit of downstreams.
- Communicate why `dev-dependencies` are required (examples / tests).
- Avoid repeating deps in `dev-dependencies`.
- Raise fixed `warp` dev dep to a MSRV compatible version with common
  `tokio-util` implicit dep. Simplifies CI lock maintenance.

Signed-off-by: Brennan Kinney <[email protected]>
@polarathene polarathene force-pushed the chore/0.13.x-min-versions branch from b8cee00 to 519f488 Compare November 7, 2023 07:38
- Reduces multi-versioned deps
- Now compatible with Rust 1.56.1

Lock file was created with:

```bash
cargo +nightly update -Z msrv-policy
cargo update --package memchr --precise 2.5.0
cargo update --package ordered-multimap --precise 0.4.0
cargo update --package tokio-util --precise 0.7.8
```

Signed-off-by: Brennan Kinney <[email protected]>
- Previously raised to `1.59.0` for CI to pass.
- With an MSRV lock file committed this is no longer necessary.

Signed-off-by: Brennan Kinney <[email protected]>
@polarathene
Copy link
Collaborator Author

polarathene commented Nov 7, 2023

Full reproduction environment to verify generated lockfile:
#492 (comment)

Rebased onto #496 as requested. CI passes 👍


I've additionally:

  • Updated the Cargo.lock.msrv to reflect Cargo.toml changes, which also verifies the simpler lock file generation.
  • Updated the CI to relax the MSRV tested back to 1.56.1 (README states 1.56.0, which rust-version matches). It was previously raised in Dec 2022 to 1.59.0 to make the CI pass.

All tests pass again 👍

Awaiting approval from either @ijackson or @matthiasbeyer

@ijackson
Copy link
Contributor

ijackson commented Nov 7, 2023

Thanks, excellent work. It's great that we now have a non-awful method for generating a working lockfile. I have reviewed the changes and verified the lockfile creation.

The MSRV reduction is also nice.

Please do merge this.

@matthiasbeyer
Copy link
Member

I am on vacation until late tomorrow. I hope I can get to having a look on my rather long backlog for this repository after tomorrow! Please do bug me if I don't!

@matthiasbeyer matthiasbeyer merged commit 08197e4 into release-0.13.x Nov 9, 2023
27 checks passed
@matthiasbeyer matthiasbeyer deleted the chore/0.13.x-min-versions branch November 9, 2023 11:50
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.

3 participants