0.7.1 release of deadpool_redis is not semver-compatible with 0.7.0

[jbg]

The 0.7.1 release has this dependency spec for redis:

version = ">=0.19,<=0.20"

The 0.7.0 release had this dependency spec:

version = "0.19"

Since deadpool_redis re-exports redis, the public API of redis forms part of the public API of deadpool_redis, so 0.7.0 and 0.7.1 are not actually compatible in a semver sense.

It also causes a problem if some crate that depends on deadpool_redis also depends on redis directly (e.g. to adjust the cargo features), because in some situations you can end up with two different versions of redis. This was my situation and a simple cargo update (which should generally be safe if everything follows semver) broke the build because deadpool_redis updated from 0.7.0 to 0.7.1 but my direct dep of redis stayed on 0.19.x.

[bikeshedder]

I thought depending on deadpool-redis 0.7.1 and redis 0.19 would use redis 0.19 for the dependency of deadpool-redis 0.19, too. This doesn't seam to be the case which is quite unfortunate.

This basicly means that version ranges are a big no-no for libraries... which in turn sounds like this is a broken feature... I feel like this is a Cargo issue in its core. Currently there is no way to specify the dependency version of a depencendy (+ features) and there doesn't seam to be a deduplication logic in place that handles version ranges of crate dependencies.

All this results in this rather unfortunate behavior.

I only added the version range as I didn't feel the need for it since the deadpool-redis code didn't need to be changed at all and supports both redis 0.19 and 0.20. You could say that deadpool-redis crate did not break semantic versioning but Cargo did.

[jbg]

I only added the version range as I didn't feel the need for it since the deadpool-redis code didn't need to be changed at all and supports both redis 0.19 and 0.20. You could say that deadpool-redis crate did not break semantic versioning but Cargo did.

I'm not sure I 100% agree here. deadpool-redis re-exports redis, so if redis has a breaking change between 0.19 and 0.20 (which the version number indicates it did), then public types in deadpool-redis (the re-exported types) have breaking changes. It's actually even worse than a break between 0.7.0 and 0.7.1 - 0.7.1 can have a different public API depending on which version of redis Cargo chooses to use.

I don't think there is any reasonable way to re-export types from a crate whilst depending on it with a version range that crosses semantic versioning "boundaries".

[jbg]

And actually "re-export" could be generalised to "have types from redis in your public API" - even just the usage of redis types in the signatures of deadpool-redis's own fns would be sufficient to cause the problem with the version range.

[jbg]

Thinking about this further, the problem can be summed up even more simply, using one of the breaking changes between versions 0.19 and 0.20 of the redis crate as an example.

This code:

use deadpool_redis::redis::ErrorKind::{self, *};

match ErrorKind {
    ResponseError => {},
    AuthenticationFailed => {},
    TypeError => {},
    ExecAbortError => {},
    BusyLoadingError => {},
    NoScriptError => {},
    InvalidClientConfig => {},
    Moved => {},
    Ask => {},
    TryAgain => {},
    ClusterDown => {},
    CrossSlot => {},
    MasterDown => {},
    IoError => {},
    ClientError => {},
    ExtensionError => {},
    ReadOnly => {},
}

always compiles on deadpool-redis 0.7.0, but may or may not compile on 0.7.1 depending on which redis version is selected by cargo. (ErrorKind became non-exhaustive in redis 0.20.) In the absence of any other constraints on redis, it won't compile, so 0.7.0 and 0.7.1 are clearly not compatible.

IMO this is not a problem in cargo - by asking for redis = ">=0.19, <=0.20" you're asking for part of your public API to come from either version, but the versions are not compatible.

A suitable fix would be yanking 0.7.1 and publishing a 0.8.0 with redis = "0.20".

Optionally you could also publish a 0.7.2 that remains compatible with redis = "0.19" if there are other unrelated changes in deadpool that may be useful to people still using redis 0.19, but given that the changes between redis 0.19 and redis 0.20 are so minor it doesn't seem worth maintaining two version branches.

[bikeshedder]

Sorry for taking this long. You're 100% right with this. I had the wrong expectation about how Cargo works and calculates versions of dependencies of dependencies.

This brings us back to the PR #60 and issue #66. I'm glad I opted against re-exporting features from those crates as it wouldn't work break semantic versioning anyways if reexporting types from those crates.

TL;DR - Specifying version ranges for tokio-postgres, redis, etc. doesn't work. In the end there is only one correct solution to this: Create a new 0.X version (or X.0 version) every time a dependency increases its (0.X or X.0) version. 🙈

This is related to #94 - deadpool needs to reach 1.0 asap. It would be nice if the API was stable and the Manager trait could be provided by the library instead.

Maybe that Manager trait should be extracted into its own crate unrelated to deadpool. This could solve this dependencies and allow independent updates of deadpool and the other libraries.

[bikeshedder]

I guess anyone affected by this change has already been affected (and hopefully updated to redis 0.20). So yanking the 0.7.1 version now is likely to do more harm than good. I'm sorry for that. 😞

I'm about to release deadpool 0.8 soon and plan on having a 1.0 version pretty soon. Deadpool is almost feature complete and the API has seen very little changes in the past so there is the chance that the deadpool-* crates will only need to be updated when the related library is being updated. Also it might be desirable to have trait implementations directly in the library versions thus reducing the tight coupling of versions altogether. (See #94)

[bikeshedder]

Please let me know if you disagree and yanking 0.7.1 and rereleasing as 0.8 should be done nonetheless.

[jbg]

It seems reasonable to me. Existing users have likely already dealt with the issue, and new users are likely to be using redis 0.20 from the start.

[bikeshedder]

All right. Time to close this issue then. In the future I'll be releasing new versions of this crate and no longer use version ranges as dependencies then.

I still kind of wonder why library crates support this in the first place... need to check with the cargo devs...

[jbg]

I still kind of wonder why library crates support this in the first place... need to check with the cargo devs...

I think in some situations (if you don't use any part of the dependent crate in the public API of your crate), a dependency with a version range in a library crate could be fine.