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

Semantic versioning breakage in 3.4 #46

Closed
neithernut opened this issue Jul 9, 2017 · 16 comments
Closed

Semantic versioning breakage in 3.4 #46

neithernut opened this issue Jul 9, 2017 · 16 comments

Comments

@neithernut
Copy link

Version 3.4 uses features which were unstable in rust-1.16. Older versions appear to build fine with rust-1.16. This means unicode-bidi is not abiding semantic versioning strictly, since an upgrade of the library introduces build failures although the major version was not bumped and nothing else in the build environment changed.

I guess this detail is generally not considered in the rust community. Sadly, due to how cargo works, this also forces depending packages to break their contracts with regard to compatibility.

This is pretty much a wont-fix, but I suggest making the CI also build this project with some fixed versions of the compiler rather than with only the current stable, beta and unstable versions of rust.

@neithernut
Copy link
Author

neithernut commented Jul 9, 2017

I just realized stupid, tired me misread the version number and this is still pre-1.0.

@behnam
Copy link
Contributor

behnam commented Jul 10, 2017

We track the minimum-rustc-version-support in the CI file here: https://github.com/servo/unicode-bidi/blob/master/.travis.yml#L10

We're at rustc:1.17.0 at the moment.

@apoelstra
Copy link

My build was also broken by this, it is a dependency of the idna crate which is a dep of the post-1.0 url crate.

@behnam
Copy link
Contributor

behnam commented Jul 10, 2017

unicode-bidi was updated after url/idna got a bump: servo/rust-url@52699f7

Unfortunately, I believe there's no common best practice about version bumps on min-lang-version-needed updates. If there's guideline, we would be glad to follow. But looks minor bumps are a the common practice at the moment.

Please see also discussion on the PR: #41

/cc @rillian

@rillian
Copy link
Contributor

rillian commented Jul 13, 2017

It's an interesting question, whether min-rust-version should be part of semver. Nothing-ever-breaks argues "yes", Rust's presumptive moving wave of stabilization-with-backward-compatibility argues "no".

I think "no" makes more sense, at least until tooling evolves to the point where it can offer semver guidance. It's hard enough to keep track of API stability. I don't think checking no new language features have crept in anywhere in the dependency chain is feasible right now, and most people are using at least the latest stable.

@apoelstra
Copy link

It's an interesting question, whether min-rust-version should be part of semver. Nothing-ever-breaks argues "yes",

The point of semver is that things don't break in minor point releases.

Rust's presumptive moving wave of stabilization-with-backward-compatibility argues "no".

This is a non-sequitor. Whatever guarantees the language is supposed to provide, crates shouldn't break in minor point releases.

apoelstra added a commit to apoelstra/icboc that referenced this issue Jul 17, 2017
For wallet software we don't want to allow random updates of dependencies,
though there shouldn't be any security risk here anyway given that the
Ledger itself is a security boundary.

See also servo/unicode-bidi#46 , it seems like
semver is not respected even for flagship Rust products so it's probably
best not to rely on it anyway.
@anowell
Copy link

anowell commented Jul 19, 2017

As a transitive dependency (through url 1.5.1), this broke compilation for all (though still small in number) rust users on our platform.

It's an interesting question, whether min-rust-version should be part of semver. Nothing-ever-breaks argues "yes", Rust's presumptive moving wave of stabilization-with-backward-compatibility argues "no".

I find myself split between those. As an individual tracking stable (and nightly), "no" makes sense for almost everything I build (and I appreciate this as a crate author). But maintaining a platform that exposes rust to our users and that tends to lag stable by a few months (1.18 is queued up but blocked by unrelated-to-rust issues), this is the 2nd major ecosystem crate to suddenly break our platform for rust users. It seems clear at this point that we should generate projects with a known-working Cargo.lock (and be less aggressive in updating deps), but in the short-term, it seems I need to update our client to lock unicode-bidi to =0.3.3.

@Manishearth
Copy link
Member

I don't see how this is relevant here.

unicode-bidi is pre-1.0, which means each version bump may contain a breaking change. unicode-bidi didn't do anything wrong here, this is following semver.

The problem here is that the url crate bumped it without realizing. This is a common problem, where a child crate makes a breaking version bump and the parent crate doesn't realize it's a bump that will affect others. Transitive dependency management is pretty hard wrt semver , and that's a rather intrinsic problem to the system.

@anowell
Copy link

anowell commented Jul 20, 2017

pre-1.0, which means each version bump may contain a breaking change

I agree per strict semver, but I'd argue cargo's behavior makes it such that pre-1.0 breaking changes should generally be minor version bumps, and most crates seem to treat it that way - but I'm not trying to make this a black or white issue, as you put it, "transitive dependency management is pretty hard".

The problem here is that the url crate bumped it without realizing

Unless I'm missing something, that is not correct. url has depended on idna ^0.1.0 for several months, and idna on unicode-bidi ^0.3.0 for basically 2 months at this point.

But my intent here isn't to criticize unicode-bidi (and I apologize if my tone indicated otherwise). Rather I wanted to share an example of where a decision made by this crate caused breakage so that we (I have also caused breakage in my own crates) as an ecosystem can be more mindful of the impact of breaking changes, and perhaps it will inform future improvements to cargo/rustc around how to make these less common. Read the icebox commit message above - I would rather that not become a common reaction in the rust ecosystem; that is my real goal here.

I am extremely grateful crates like unicode-bidi - even if I didn't know it existed until I saw it fail to compile.

@Manishearth
Copy link
Member

Ah, right.

Yeah, that's fair. It's pretty hard to keep track of all versioning things here and we made a mistake. This stuff happens, and we'll try to be more careful in the future.

@Manishearth
Copy link
Member

Ultimately I don't like setting a precedent that all compiler upgrades should require a major semver bump. This is nearly impossible to support, and effectively means that crates get stuck on older versions for way too long. You can try to avoid it as much as possible, but that's about it; there's nothing stopping new code from being written that would not work on an older rust versions. Some crates test on older versions to prevent this; but then how far back do you go?

Because backwards compatibility is not an issue in Rust (or, it's not supposed to be), we shouldn't have to worry about forwards compatibility that much. Because it is supposed to be effortless to upgrade compilers, it may mean that a crate contains a change that needs an upgrade, and you should do it at that stage.

For custom setups with pinned rustc this doesn't work as well, so yeah, we should still try to avoid it, but these are special cases anyway. Heck, Firefox does this and we've had issues with it before, but ultimately we're violating the assumption that a compiler upgrade is easy.

I really think this discussion should be moved to users.rust-lang.org. I feel like it's an intractable problem in general, but let's see what the rest of the community things.

@neithernut
Copy link
Author

Ultimately I don't like setting a precedent that all compiler upgrades should require a major semver bump.

Well, compiler versions typically don't. But language versions do. They are just quite intermingled in the case of rust.

@Manishearth
Copy link
Member

Yeah, language version upgrades is what I meant.

With the Rust epoch stuff happening this may become clearer.

@cardoe
Copy link

cardoe commented Aug 7, 2017

For custom setups with pinned rustc this doesn't work as well, so yeah, we should still try to avoid it, but these are special cases anyway. Heck, Firefox does this and we've had issues with it before, but ultimately we're violating the assumption that a compiler upgrade is easy.

@Manishearth I'm a bit late to the party but yes, ultimately making the assumption that you can just upgrade compilers breaks the semver promise that is aimed at by the crates.io versioning system. Don't forget that there's also two things that technically get upgraded, the compiler and the standard library. There's currently no way to depend on either. But there is rust-lang/rfcs#1133 for the later however.

@cardoe
Copy link

cardoe commented Aug 7, 2017

In fact the release of 0.3.4 was actually a semver break and should have been called 0.4.0 based on http://semver.org/ The release comments note that Unicode 10 was added which required changes to a test. This implies there was functionality added in a backwards compatible fashion which implies that the minor version should have received a bump since this was not just a bug fix release.

cardoe referenced this issue in abonander/anterofit Aug 7, 2017
@behnam
Copy link
Contributor

behnam commented Aug 7, 2017

@cardoe,

The release comments note that Unicode 10 was added which required changes to a test. This implies there was functionality added in a backwards compatible fashion which implies that the minor version should have received a bump since this was not just a bug fix release.

Under unicode-bidi crate, there was no test data update, but only how tests were executed. (Under url crate, yes, there was a bunch of fixes for compatibility.) But, in general, that logic doesn't stand, because then every bug fix would be a behavior change and would need a breaking upgrade.

On the other hand, I agree that Unicode version upgrades should be more deliberate. It may not be necessary for some algorithms that are generally stable, like unicode-bidi, but could be much desired for other algorithms, like segmentation.

We have had some discussion about this exact matter here: https://users.rust-lang.org/t/introducing-unic-unicode-and-internationalization-crates-for-rust/11447


Anyway, I should note that this specific GH Issue is not about Unicode data update, but it's about the rustc dependency. I believe that's still an open question in general and varies project to project. IMHO, it's a conversation that should be taken to the users forum, since it's not specific to this module.

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

No branches or pull requests

7 participants