rust-lang / jobserver-rs

Apache License 2.0
69 stars 39 forks source link

Reduce MSRV to 1.63 with minor change in RawFd type #62

Closed hoxxep closed 7 months ago

hoxxep commented 9 months ago

I noticed that a patch version increment has caused other crates MSRV values to change, such as the zstd crate: https://github.com/gyscos/zstd-rs/issues/253

This MR changes the RawFd type to one present in older versions of rust, so that the MSRV can be reduced to 1.63.

My understanding is that this type should be equivalent because it's gated against #[cfg(unix)], and so the benefits of std::os::fd::RawFd also supporting WASM is negated. https://github.com/rust-lang/rust/issues/98699

In theory the MSRV of the crate library is 1.60, but there are dev-dependencies that require 1.63 which would break the MSRV testing workflow (rustix, required by tempfile).

If we insist on keeping the usage of std::os::fd::RawFd, I can feed back to the zstd crate and ask them to bump their MSRV instead. Thank you!

epage commented 9 months ago

Not speaking to the value of this change but to the motivation:

I noticed that a patch version increment has caused other crates MSRV values to change, such as the zstd crate: https://github.com/gyscos/zstd-rs/issues/253

A change in MSRV in this crate does not affect the MSRV of any other crate unless they take an action like bump their version requirement. You can maintain a valid dependency tree for your MSRV via Cargo.lock / cargo update. You can even use cargo +nightly generate-lockfile -Zmsrv-policy. We are discussing stabilization on internals.

hoxxep commented 9 months ago

I agree in principle that the jobserver crate should be allowed to increase its MSRV, but might want to be careful about doing so.

On a rust 1.64 toolchain cargo update is no longer able to resolve a valid dependency tree for the zstd crate. Currently, the only real option for end users on older toolchains is to manually search for and specify an older version of jobserver.

The reasons for submitting this PR are:

  1. This crate is widely used (https://crates.io/crates/cc/reverse_dependencies)
  2. Rust 1.66 is <1y old.
  3. The MSRV bump was introduced by jobserver in a patch version, introducing breaking downstream changes, potentially for enterprise users on older toolchains. Example: for a fresh install of the zstd dependency on rustc 1.64, the zstd crate will no longer compile by default as it resolves jobserver 0.1.27. There are workarounds for end users as you say, but that involves searching though older jobserver versions as cargo update is currently unable to do MSRV-aware dependency resolution.
  4. The change required to enable jobserver to use a lower MSRV is fairly trivial in this PR. If it made the code objectively worse, I would understand that there's a bigger trade-off between compatibility and allowing crates to use newer features.

@epage I see you're contributing to the issue on MSRV-aware dependency resolution at https://github.com/rust-lang/cargo/issues/9930. My two cents would be that MSRV bumps could require at least a minor version bump, so that older toolchains can still benefit from bug and/or security patches in the future.

epage commented 9 months ago

MSRV bumps are generally recognized as non-breaking.

From a tooling perspective, minor and patch are equivalent.

Also, a 1 year old MSRV covers 99% of requests to crates.io.

hoxxep commented 9 months ago

@epage I've copied our discussion into your internals thread since it seemed a better place to discuss than this PR 😊 https://internals.rust-lang.org/t/pre-rfc-msrv-aware-resolver/19871/50

Agreed the current interpretation is that MSRV bumps are considered non-breaking. They clearly are breaking for some users though, and so perhaps alongside the MSRV aware RFC we could change the precedent to consider them "semi-breaking" changes. An MSRV bump doesn't require a major version bump, but crates could benefit by leaving room in the versioning scheme to have the option to release patches on the old MSRV for security fixes.

Regarding the crates.io request metrics, I don't doubt the rust community is more up to date than C++ users, but I'd note that legacy/enterprise users will often have their own crates.io mirrors and forked repositories. So crates.io metrics could be skewed in favour of smaller non-legacy projects.