rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.58k stars 2.39k forks source link

Additional SemVer compatibility items. #8736

Open ehuss opened 3 years ago

ehuss commented 3 years ago

This is a dump of some elements that I considered adding to the SemVer compatibility chapter, but didn't have time or knowledge to finish.

As an alternative to documentation, we could make these lints in a tool like cargo-crate-api

jhpratt commented 3 years ago

Another thing that might be worth mentioning is the standback crate in the MSRV bit. It does exactly what is recommended — copies code from stdlib — but also re-exports when possible based on the compiler version in use. I created it specifically for that reason, and have yet to run into any issues. Imports are essentially identical, as a bonus.

QuineDot commented 3 years ago
  • Fundamental types, adding blanket trait impls, and the re-rebalancing RFC. There is a section on fundamental types that has changed as part of RFC 1023 and RFC 2451 that I don't fully understand.
  • Maybe discuss the impact of adding this impl has: #75180

75180 is a blanket trait impl (and thus a major breaking change) as per RFC 2451: it's an impl for &T, but as & is fundamental, &T is not covered, and thus the impl is a blanket impl. This comment (and some of the crater results) illustrate how it can break existing downstream implementations.

Skepfyr commented 3 years ago

Another thing I believe is missing is updating dependencies whose types are part of your public API.

epage commented 2 years ago

We talked about this in the cargo team meeting and we are leaning towards automating this, see https://github.com/rust-lang/cargo/issues/374, like with cargo-crate-api

udoprog commented 2 years ago

I don't know if this falls under here (or if there's a duplicate issue or any prior discussion), but due to the way that the cargo resolver currently works removing a feature flag even from a non-public dependency appears to constitute a breaking change.

Consider crate a and b:

[package]
name = "a"
version = "1.0.0"

[dependencies]
uuid = { version = "1.0.0", features = ["serde"] }
[package]
name = "a"

[dependencies]
a = "1.0.0"
uuid = "1.0.0"

Crate b can now use uuid::Uuid as if it implements Serialize / Deserialize since the serde feature is transitively activated.

If crate a then changes their manifest to:

[package]
name = "a"
version = "1.0.1"

[dependencies]
uuid = "1.0.0"

Once crate b updates its dependency to crate a, uuid::Uuid no longers implements the Serialize / Deserialize traits and any code which assumed so stops compiling. Removing a dependency in a constitutes removing a constraint requiring any non-default features.

If we bring -Z min-versions into the mix, removing or relaxing a dependency has additional versioning hazards since it might potentially relax a minimal version constraint.

Example of breakage in the wild: tokio-rs/tokio#4663

RalfJung commented 1 year ago

Compatibility of types of fields within a repr(packed) type. See https://github.com/rust-lang/rust/issues/115305#issuecomment-1696860270 and https://github.com/rust-lang/rust/pull/115315.

FWIW I don't think this should lead to a docs change; it should lead to a rustc change instead, similar to https://github.com/rust-lang/rust/pull/99020.