serde dependencies must be synchronized #7

Closed
opened 2016-03-13 00:53:56 +13:00 by SkylerLipthay · 6 comments
SkylerLipthay commented 2016-03-13 00:53:56 +13:00 (Migrated from github.com)

Unfortunately, the ^0 introduced around 13fa7c98f7 is problematic. While SemVer requires a major version bump for breaking changes for versions >= 1.0.0, it allows breaking changes between minor version releases for versions < 1.0.0. "0.x and 0.x+1 are not considered compatible, but 1.x and 1.x+1 are." For this reason, specifying 0 or ^0 is an anti-pattern.

Serde 0.7 is not backwards compatible with 0.6, so the Serde dependencies should be set to 0.7.

What ended up happening to someone in IRC was that serde_codegen 0.7 was being used alongside serde 0.6 by textnonce, because the version rules were relaxed enough for Cargo to decide that was OK. This resulted in this compile error.

Similarly, Syntex's minor version should be specified in Cargo.toml.

I recommend cargo yanking the affected versions (0.2.3 and 0.2.4, I believe) of textnonce and releasing 0.2.5 with the fixed dependencies.

Unfortunately, the `^0` introduced around 13fa7c98f7e60c120f230a7b6f83b10cbcdf804f is problematic. While SemVer requires a major version bump for breaking changes for versions `>= 1.0.0`, it allows breaking changes between minor version releases for versions `< 1.0.0`. ["0.x and 0.x+1 are not considered compatible, but 1.x and 1.x+1 are."](https://github.com/steveklabnik/semver) For this reason, specifying `0` or `^0` is an anti-pattern. Serde 0.7 is not backwards compatible with 0.6, so the Serde dependencies should be set to `0.7`. What ended up happening to someone in IRC was that `serde_codegen 0.7` was being used alongside `serde 0.6` by `textnonce`, because the version rules were relaxed enough for Cargo to decide that was OK. This resulted in [this compile error](http://pastebin.com/JciLCsbP). Similarly, Syntex's minor version should be specified in Cargo.toml. I recommend `cargo yank`ing the affected versions (`0.2.3` and `0.2.4`, I believe) of textnonce and releasing `0.2.5` with the fixed dependencies.
mikedilger commented 2016-03-13 01:33:42 +13:00 (Migrated from github.com)

But textnonce works with serde 0.6. textnonce doesn't really care, and that is what ^0 is saying, and I think rightly so.

What I noticed sfackler did in rust-postgres was specify version = ">= 0.6, < 0.8" . This I am happy to do.

I understand that some things which use textnonce depend on serde 0.7. But equally true (at least up until a few days ago, perhaps still), some things which use textnonce still depend on serde 0.6.

If you go through Cargo.lock to find where dependencies collide, I suspect you will find some dependency other than textnonce that is requiring serde 0.6. If not, it seems to suggest a bug in Cargo.

But `textnonce` works with `serde 0.6`. `textnonce` doesn't really care, and that is what `^0` is saying, and I think rightly so. What I noticed sfackler did in rust-postgres was specify `version = ">= 0.6, < 0.8"` . This I am happy to do. I understand that some things which use `textnonce` depend on `serde 0.7`. But equally true (at least up until a few days ago, perhaps still), some things which use `textnonce` still depend on `serde 0.6`. If you go through `Cargo.lock` to find where dependencies collide, I suspect you will find some dependency _other_ than `textnonce` that is requiring `serde 0.6`. If not, it seems to suggest a bug in Cargo.
SkylerLipthay commented 2016-03-13 11:02:56 +13:00 (Migrated from github.com)

If you go through Cargo.lock to find where dependencies collide, I suspect you will find some dependency other than textnonce that is requiring serde 0.6.

Correct, this is the problem. If you do a fresh cargo build of textnonce, the resulting Cargo.lock specifies the latest dependencies: serde 0.7 and serde_codegen 0.7. If you manually change the latter to serde_codegen 0.6.14 and cargo build again, textnonce compilation fails with the aforementioned error.

This can happen innocently depending to your Cargo workflow. Example:

  • Start a fresh crate with one dependency: iron = "0.2.6"
  • cargo build → success!
  • Add the dependency params = "0.0.3"
  • cargo buildtextnonce v0.2.4 fails the compilation.

I linked the current Cargo.toml on irc.mozilla.org#cargo and sfackler had this to say:

sfackler: Skyler: that seems like a setup that would not work
...
sfackler: yeah, seems like it just needs a "cargo update" kick the 0.6 dependency up to 0.7

  • cargo update
  • cargo build → success!

I'm not sure if there's a case where cargo update would not resolve this issue, but I still think it is worth addressing. version = ">= 0.6, < 0.8" would be better as it would prevent 0.8 from e.g. changing #[derive(Serialize)] to #[derive(SerdeSerialize)] and spontaneously breaking the crate, but it doesn't address the problem that we're facing with serde and serde_codegen strictly requiring their simultaneous usage to be locked at the same minor version.

I'm sorry this is such a hassle, I didn't recognize that this problem would be so complicated. I'm learning a lot about Cargo... so not all is lost for me! 😛

> If you go through `Cargo.lock` to find where dependencies collide, I suspect you will find some dependency other than `textnonce` that is requiring `serde 0.6`. Correct, this is the problem. If you do a fresh `cargo build` of `textnonce`, the resulting `Cargo.lock` specifies the latest dependencies: `serde 0.7` and `serde_codegen 0.7`. If you manually change the latter to `serde_codegen 0.6.14` and `cargo build` again, `textnonce` compilation fails with the aforementioned error. This can happen innocently depending to your Cargo workflow. Example: - Start a fresh crate with one dependency: `iron = "0.2.6"` - `cargo build` → success! - Add the dependency `params = "0.0.3"` - `cargo build` → `textnonce v0.2.4` fails the compilation. I linked the current `Cargo.toml` on irc.mozilla.org#cargo and sfackler had this to say: > sfackler: Skyler: that seems like a setup that would not work > ... > sfackler: yeah, seems like it just needs a "cargo update" kick the 0.6 dependency up to 0.7 - `cargo update` - `cargo build` → success! I'm not sure if there's a case where `cargo update` would not resolve this issue, but I still think it is worth addressing. `version = ">= 0.6, < 0.8"` would be better as it would prevent `0.8` from e.g. changing `#[derive(Serialize)]` to `#[derive(SerdeSerialize)]` and spontaneously breaking the crate, but it doesn't address the problem that we're facing with `serde` and `serde_codegen` strictly requiring their simultaneous usage to be locked at the same minor version. I'm sorry this is such a hassle, I didn't recognize that this problem would be so complicated. I'm learning a lot about Cargo... so not all is lost for me! :stuck_out_tongue:
mikedilger commented 2016-03-16 11:49:14 +13:00 (Migrated from github.com)

The right solution requires changes to Cargo, and I've filed an issue (see above). Several well known community crates explicitly require serde 0.6 still (I dare say most do), while a few now explicitly require serde 0.7, and even fewer still allow for either version of serde. Specifying one or the other excludes use with the other set of crates, so I'm really in a pickle here. There is no way to make this work for everybody at the moment. I even think cargo update -p serde; cargo update -p serde_codegen isn't sufficient; manual editing of Cargo.lock may be required in some cases. Manual forking of some packages to tweak their dependencies may also be required. If you come up with something that works for everybody, I'm all ears.

The right solution requires changes to Cargo, and I've filed an issue (see above). Several well known community crates explicitly require `serde 0.6` still (I dare say most do), while a few now explicitly require `serde 0.7`, and even fewer still allow for either version of serde. Specifying one or the other excludes use with the other set of crates, so I'm really in a pickle here. There is no way to make this work for everybody at the moment. I even think `cargo update -p serde; cargo update -p serde_codegen` isn't sufficient; manual editing of `Cargo.lock` may be required in some cases. Manual forking of some packages to tweak their dependencies may also be required. If you come up with something that works for everybody, I'm all ears.
SkylerLipthay commented 2016-03-16 12:41:11 +13:00 (Migrated from github.com)

I like that resolution, thank you for filing an issue!

As a grubby 0.7 consumer, my instinct is to force all 0.6 consumers to specify textnonce 0.2.2 until they can upgrade to 0.7. I understand the trouble in this ruling (feature lock), so I don't know if it's standard practice by any other crate authors.

Anyway, thanks for hashing this out with me!

I like that resolution, thank you for filing an issue! As a grubby `0.7` consumer, my instinct is to force all `0.6` consumers to specify `textnonce 0.2.2` until they can upgrade to `0.7`. I understand the trouble in this ruling (feature lock), so I don't know if it's standard practice by any other crate authors. Anyway, thanks for hashing this out with me!
mikedilger commented 2016-03-16 14:07:00 +13:00 (Migrated from github.com)

I'll reopen to track the upstream issue

I'll reopen to track the upstream issue
mikedilger commented 2018-06-14 10:05:25 +12:00 (Migrated from github.com)

This is unlikely to still be an issue as serde has been stabilized for some time now.

This is unlikely to still be an issue as serde has been stabilized for some time now.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
mikedilger/textnonce#7
No description provided.