Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

byteorder = ">=1.3.0, < 1.4.0" restriction might be controversial #380

Closed
mexus opened this issue Feb 24, 2021 · 7 comments · Fixed by #390
Closed

byteorder = ">=1.3.0, < 1.4.0" restriction might be controversial #380

mexus opened this issue Feb 24, 2021 · 7 comments · Fixed by #390

Comments

@mexus
Copy link

mexus commented Feb 24, 2021

Let's get a minimal library that got bincode and byteorder as the only dependencies:

[package]
name = "test-bincode-byteorder"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
bincode = "1"
byteorder = "1.4" # Yes, I deliberately use "non-compatible" version of byteorder

Then cargo check it and see the contents of the generated Cargo.lock:

# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
[[package]]
name = "bincode"
version = "1.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f30d3a39baa26f9651f17b375061f3233dde33424a8b72b0dbe93a68a0bc896d"
dependencies = [
 "byteorder",
 "serde",
]

[[package]]
name = "byteorder"
version = "1.4.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ae44d1a3d5a19df61dd0c8beb138458ac2a53a7ac09eba97d55592540004306b"

[[package]]
name = "serde"
version = "1.0.123"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "92d5161132722baa40d802cc70b15262b98258453e85e5d1d365c757c73869ae"

[[package]]
name = "test-bincode-byteorder"
version = "0.1.0"
dependencies = [
 "bincode",
 "byteorder",
]

As you can see, the bincode's byteorder < 1.4.0 requirement is completely ignored.

If we assume it's simply a bug in cargo (I'm not quite sure about this to be honest) and it will get fixed sooner or later, then this restriction will prevent users from utilizing byteorder's features introduced in 1.4.0 (because there will be conflicting minor versions in the tree). Please correct me if I'm wrong, but isn't the only purpose of pinning the byteorder's minor version to make MSRV to be 1.18.0? But won't it be simpler for the end user to pin the crates not supporting rust-1.18 with their own hands? I mean, most probably they'll need to do that anyhow with other crates.

Also the restriction makes cargo outdated go crazy :))

$ cargo outdated
error: failed to select a version for `byteorder`.
    ... required by package `bincode v1.3.2`
    ... which is depended on by `test-bincode-byteorder v0.1.0 (/tmp/cargo-outdated1R5PzV)`
versions that meet the requirements `>=1.3.0, <1.4.0` are: 1.3.4, 1.3.3, 1.3.2, 1.3.1, 1.3.0

all possible versions conflict with previously selected packages.

  previously selected package `byteorder v1.4.2`
    ... which is depended on by `test-bincode-byteorder v0.1.0 (/tmp/cargo-outdated1R5PzV)`

failed to select a version for `byteorder` which could resolve this conflict

Could there be a chance to.. well.. eliminate the , < 1.4.0 part?

Thanks!

@ZoeyR
Copy link
Collaborator

ZoeyR commented Feb 25, 2021

Not respecting the < 1.4.0 seems to be a bug on cargo's part. This restriction doesn't actually prevent downstream users from using byteorder 1.4 elsewhere in their crate, it will just result in two different versions of byteorder being included in the final build. The policy of this crate is that it must build on rust 1.18.0 for the 1.0 version. Bumping the MSRV without changing major versions can cause someone's build to break so changing MSRV is a breaking change.

@ZoeyR
Copy link
Collaborator

ZoeyR commented Feb 25, 2021

also I would like to point out your cargo.lock uses bincode 1.3.1, which doesn't have the 1.4 restriction. This is cargo trying to solve for minimal deps.

@mexus
Copy link
Author

mexus commented Feb 25, 2021

also I would like to point out your cargo.lock uses bincode 1.3.1, which doesn't have the 1.4 restriction. This is cargo trying to solve for minimal deps.

Indeed! Totally missed that.

This restriction doesn't actually prevent downstream users from using byteorder 1.4 elsewhere in their crate

Well it does:

[dependencies]
bincode = "=1.3.2"
byteorder = "=1.4.0"
$ cargo check
error: failed to select a version for `byteorder`.
    ... required by package `bincode v1.3.2`
    ... which is depended on by `two-bincodes v0.1.0 (/home/mexus/test/two-bincodes)`
versions that meet the requirements `>=1.3.0, <1.4.0` are: 1.3.4, 1.3.3, 1.3.2, 1.3.1, 1.3.0

all possible versions conflict with previously selected packages.

  previously selected package `byteorder v1.4.0`
    ... which is depended on by `two-bincodes v0.1.0 (/home/mexus/test/two-bincodes)`

failed to select a version for `byteorder` which could resolve this conflict

Hm, it also seems that making such a restriction is a breaking change, because if I already use byteorder-1.4.0 and bincode-1.3.1, then updating bincode to 1.4.0 will break my compilation. Or am I wrong?

@ZoeyR
Copy link
Collaborator

ZoeyR commented Feb 25, 2021

looking into this further it seems like a missing component of cargo rust-lang/cargo#6584 (comment). Rust can technically handle this but cargo doesn't allow it to. Either Cargo or byteorder needs to fix this since Cargo is missing an important feature and byteorder isn't following semver properly.

@ZoeyR ZoeyR closed this as completed Feb 25, 2021
@mexus
Copy link
Author

mexus commented Feb 25, 2021

Either Cargo or byteorder needs to fix this

@ZoeyR could you please elaborate a bit on how byteorder can possible address the issue?

@ZoeyR
Copy link
Collaborator

ZoeyR commented Feb 25, 2021

@mexus either byteorder needs to revert its MSRV changes or yank 1.4 and release breaking changes in 2.0

@BurntSushi
Copy link

byteorder isn't following semver properly

That's false. semver only applies to incompatible API changes. The API of byteorder 1.3 is compatible with the API of byteorder 1.4.

It's your project, so it's your rules. If you want to treat MSRV bumps as breaking changes, then that's your prerogative. But don't misrepresent me by claiming I'm doing something that I'm not.

@ZoeyR ZoeyR linked a pull request Apr 10, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants