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

Require stable serde #100

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Require stable serde #100

wants to merge 1 commit into from

Conversation

atouchet
Copy link
Contributor

@atouchet atouchet commented Mar 8, 2023

Serde 1.0 came out 6 years ago. Is there any issue with dropping these old versions?

@mbrubeck
Copy link
Contributor

mbrubeck commented Mar 8, 2023

This crate is very conservative about breaking changes. More than 17000 downstream crates depend on unicode-bidi, so even very rare situations can come up frequently among our users. We try not to make changes like this unless there is some important benefit that outweighs the risk of downstream breakage.

@atouchet
Copy link
Contributor Author

atouchet commented Mar 9, 2023

For whatever it's worth unicode-bidi only has 15 public dependents listed on crates.io and none of them directly depend on serde 0.8 or 0.9. The vast majority of downloads come from only 2 crates: idna and stringprep.

@mbrubeck
Copy link
Contributor

mbrubeck commented Mar 9, 2023

That figure was including transitive dependencies, mostly via the url crate which depends on idna. It's true that idna does not use the serde feature of this crate, which does limit the possible impact.

However, it does mean that bumping the major version of this crate is highly disruptive, and that would be technically required for a breaking change like this...

@atouchet
Copy link
Contributor Author

atouchet commented Mar 9, 2023

Maybe I'm just confused here but why would this be a breaking change? The crate can already use serde 1.0, this would just ensure that.

@Manishearth
Copy link
Member

Most Rust crates do not consider bumps to their dependencies to affect semver, but the closer you are to the bottom of the dep graph, the more you have to care about this stuff because it is possible to break builds by forcing there to be multiple incompatible versions of a crate (which sometimes works and sometimes fails builds).

@mbrubeck
Copy link
Contributor

mbrubeck commented Mar 9, 2023

Concretely, this would break a crate that depends on unicode-bidi with the serde feature enabled, and also depends directly on serde version 0.8.x, and passes (for example) a unicode_bidi::Level to the serde::Serialize::serialize function.

In the language of RFC 1977, serde is a “public dependency” of unicode-bidi. As discussed in that RFC, “if you bump a public dependency's version, it's a breaking change of your own crate.”

@Manishearth
Copy link
Member

Manishearth commented Mar 9, 2023

(And I'll caveat this by mentioning that a large subset of the community does ignore the de jure concept of public-dependency semver breakages, following a de facto definition that doesn't incorporate dependency changes, however as a heavily used dependency we are more beholden to the stricter de jure definition as compared to the de facto definition)

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #127) made this pull request unmergeable. Please resolve the merge conflicts.

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 this pull request may close these issues.

4 participants