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

Please don't break current Rust versions because of concerns about outdated Rust versions #388

Closed
kornelski opened this issue Apr 7, 2021 · 14 comments · Fixed by #390
Closed

Comments

@kornelski
Copy link
Contributor

kornelski commented Apr 7, 2021

I see that you've blocked updates of byteorder due to concern for Rust 1.18 compatibility (#381, #380). However, in doing so, you're causing problems for all Rust versions:

error: failed to select a version for `byteorder`.
    ... required by package `bincode v1.3.2`

I know your change isn't technically a semver-breaking, but because of the way Cargo resolves dependencies, it is breaking builds (I have byteorder 1.3.2 elsewhere in the dependency tree, and cargo update can't deal with it).

Paradoxically, in a good-faith attempt to avoid breaking anyone, you've caused more breakage.

Please don't cause pain for all Rust users out of concern for users who use an outdated Rust version.

@VictorKoenders
Copy link
Contributor

See #380 for more information on this

Currently bincode supports rust 1.18 and up, and byteorder released a version that is not compatible with this. We're working on a (breaking) change version that has a higher minimum rust version that fixes this.

@dtolnay
Copy link
Contributor

dtolnay commented Apr 8, 2021

@VictorKoenders, this is a frustrating response to the OP's concern.

Your minimum version policy is supposed to be about avoiding breaking other people's projects. The policy for the sake of policy is not the point. The policy for the sake of minimizing breakages is the point. If your policy is causing vastly more breakage than it is preventing, it is misguided and you need to rethink it, not double down.

Putting a < bound for this purpose is never the right call, and needs to be fixed in 1.x, not 2.x.

For instance you could fork and maintain a backported byteorder instead of forcing a < bounded real byteorder across the downstream project's whole dependency graph. That eliminates all downstream breakage the < is causing. I still think it is the wrong approach (as opposed to fixing your version policy), but it is at least uniformly better than the current approach.

@VictorKoenders
Copy link
Contributor

If your policy is causing vastly more breakage than it is preventing

We do not know if our current policy is breaking more than breaking MSRV, as there currently are no reported breakages because of MSRV.

For instance you could fork and maintain a backported byteorder

If you want to release a 1.3 version of byteorder to crates.io and make a PR to bincode that points at this dependency, then we can merge that. This will fix the issues people are having, although I hate the idea of having duplicate crates in a dependency tree. Otherwise we'll have to wait for bincode 2.0.

@kornelski
Copy link
Contributor Author

kornelski commented Apr 8, 2021

Please don't release 2.0. There are over 1400 crates depending on bincode 1. A 2.x release would fragment this user base, cause duplicate bincode dependencies, and worst of all — prolong the breakage caused by bincode 1.x conflict-causing dependency.

If you insist on being strict about your MSRV, I would prefer if you forked or dropped byteorder instead. That would be smaller duplication, and could be reversible and hopefully temporary.

@kornelski
Copy link
Contributor Author

@VictorKoenders byteorder doesn't want to support Rust versions that are as old as 1.18, because it would require adding unsafe code back to the crate and reverting edition upgrade.

Could you compromise with MSRV around Rust 1.31? That should be old enough to be supported by all Linux distros and mrustc, and would be an acceptable target for byteorder.

@VictorKoenders
Copy link
Contributor

Referring back to the #380 issue that I linked earlier.

The policy of this crate is that it must build on rust 1.18.0 for the 1.0 version. I'm assuming that policy is put in place for a reason. I'm just here to do light maintenance like answering issues and approving PRs, so I'm not going to go against big decisions like that.

There are 2 solutions I can see.

  1. As pointed out in byteorder = ">=1.3.0, < 1.4.0" restriction might be controversial #380, byteorder can yank 1.4 and release a 2.0 with the breaking MSRV change
  2. As suggested by @dtolnay, someone can fork and maintain byteorder 1.3, release it under another name, and make a PR to bincode that uses that fork.

@dtolnay
Copy link
Contributor

dtolnay commented Apr 9, 2021

I'm assuming that policy is put in place for a reason.

I am inclined to assume the opposite — it was put in place to prioritize the needs of a fictional user who is still stuck on a compiler from 2017 four years later, yet critically requires access to bleeding edge bincode releases; not because there is any real actual human in that situation that we know of.

From ZoeyR's comment you linked, it also seems the policy is partially based on misunderstanding how Cargo resolves dependencies:

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.

This is not true as pointed out in the thread, and as seen in #388 (comment).

@dtolnay dtolnay reopened this Apr 9, 2021
@dtolnay
Copy link
Contributor

dtolnay commented Apr 9, 2021

I'll go ahead and reopen in light of this: "I'm just here to do light maintenance like answering issues and approving PRs, so I'm not going to go against big decisions like that."

@kornelski
Copy link
Contributor Author

kornelski commented Apr 9, 2021

Yes, I'm asking you to break your existing policy. I assume it was put there because at that time Rust 1.18 was still relevant, but mere passage of time has likely changed that.

Over time old Rust versions lose relevance. The old reason may have been because Linux distros had this version, or mrustc supported 1.19 only (bootstrapping is important for some users), or because Servo used that Rust version. These reasons are likely not relevant any more, and bincode could change its policy without causing any problems for anyone, or at least cause fewer problems for fewer users than < version requirement already does.

@BurntSushi
Copy link

I'd also like to note that byteorder's current MSRV of 1.41.1 is in and of itself already quite conservative. It's the current version of rustc in Debian stable and was chosen for that reason.

@luben
Copy link
Contributor

luben commented Apr 9, 2021

I'm assuming that policy is put in place for a reason. I'm just here to do light maintenance like answering issues and approving PRs, so I'm not going to go against big decisions like that.

Who is in charge making such decisions? Can we escalate instead of closing the issue?

@ZoeyR
Copy link
Collaborator

ZoeyR commented Apr 10, 2021

We have a fundamental disagreement on what constitutes a breaking change. My opinion is, and has always been, that MSRV bumps are breaking changes. Former compilations using a rustc compiler could stop working, that is breaking full stop. The issue with Cargo not wanting to compile 1.3 byteorder and 1.4 in the same dependency tree is cargo's fault, see my comment #380 (comment) regarding this. I will never change to a policy of "well it doesn't affect any real users" since there is no way to determine who is a real user. If there was no reason to continue supporting rust 1.18 why does serde maintain support for 1.13+?

@kornelski
Copy link
Contributor Author

@ZoeyR 👋 I'm a real user who you are currently breaking. You don't care about breaking my build because I use Rust 1.51, but you'd care if I used Rust 1.18?

Cargo's behavior is well-documented. You have already effectively broken your MSRV, because your effective (not intended, but real in practice) MSRV is now some future version where Cargo has support for MSRV-based version resolution.

@ZoeyR ZoeyR linked a pull request Apr 10, 2021 that will close this issue
@ZoeyR
Copy link
Collaborator

ZoeyR commented Apr 10, 2021

Fixed in 1.3.3

@ZoeyR ZoeyR closed this as completed Apr 10, 2021
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.

6 participants