Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Addition of name member to Fail trait is a semver-breaking change #312

Open
jdm opened this issue May 14, 2019 · 4 comments
Open

Addition of name member to Fail trait is a semver-breaking change #312

jdm opened this issue May 14, 2019 · 4 comments

Comments

@jdm
Copy link

jdm commented May 14, 2019

df68d72 added a member to a public trait, but it was published as 0.1.5. This breaks the semver guarantees and can cause code to stop compiling when failure_derive gets updated independently of the failure crate.

@jdm jdm changed the title Addition of name member to Error trait is a semver-breaking change Addition of name member to Fail trait is a semver-breaking change May 14, 2019
@BurntSushi
Copy link

Adding a new method to a trait with a default implementation is generally not considered a breaking change. See the API evolution RFC: https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#minor-change-adding-a-defaulted-item

If this is causing an issue with failure_derive, could you give an example?

@jdm
Copy link
Author

jdm commented May 14, 2019

One of the Servo contributors added a dependency to https://github.com/jrmuizel/raqote/ to Servo, which depends on https://crates.io/crates/font-kit and transitively the failure crate. This apparently introduced failure_derive into Servo's build (it's not present on master), which brought in failure_derive 0.1.5 while Servo was depending on failure 0.1.3. This caused the font-kit build to error out on a derive(Fail) because the name method did not exist in the trait that was being derived.

@BurntSushi
Copy link

BurntSushi commented May 14, 2019

Interesting. But I don't think that's a semver violation. It's probably a result of an incorrect minimal version specification in failure_derive: https://github.com/rust-lang-nursery/failure/blob/7b7d03cc6ef247eb3835a6c7f91aa81d1d1d0afb/failure_derive/Cargo.toml#L19

Cargo has a way to do a minimal version check which should catch bugs like this on failure's side, but this has proved difficult to enact in practice since maintainers of core crates in the ecosystem don't specify correct minimal versions.

@dekellum
Copy link

The failure_derivefailure is only a dev. dependency. The other direction, failurefailure_derive, should be more relevant. For failure 0.1.5 it is as follows. Its been using this same pattern since 0.1.1.

[dependencies.failure_derive]
optional = true
version = "0.1.5"
path = "./failure_derive"

As reported in #234, the failure 0.1.2 release broke actix due to a similar combination of failure_derive 0.1.2 and failure 0.1.1. As these combinations aren't really being tested for new releases here, perhaps it would be more reliable in the long run if the above dependency pattern was narrowed to a single version, e.g. version = "=0.1.5", for subsequent releases?

In the meantime, the workaround suggested in #234 might fix Servo?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants