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

Opaque errors due to conflicting crate versions #1636

Closed
Ryman opened this issue May 19, 2015 · 13 comments
Closed

Opaque errors due to conflicting crate versions #1636

Ryman opened this issue May 19, 2015 · 13 comments

Comments

@Ryman
Copy link

Ryman commented May 19, 2015

In a dependency chain such that:
Library A is published up to 0.5
Library B depends on A = "=3.0"
Users application C depends on B = "*" and A = "*"

Cargo chooses version 0.5 of A for the downstream crate C which means anything bound on traits from A=0.3 in B are not implemented for the types in use in the interaction between B and C. Which means things fail to compile with errors about trait Foo is not implemented for Bar where they clearly are in the docs. The error messages given by rustc don't help a user realize that a version mismatch is the root cause in this scenario.

To get the downstream crate to build, we need to bound their A dependency to use to the same version as B, or do a lot of re-exports from B. This means that if B updates their dependencies in future, downstream will have to update their Cargo.toml again to B's version of A. (This will require the user probably looking at B's Cargo.toml, or the Cargo.lock)

This feels like it will become a worse problem as we starting seeing more crates semver'ing which will involve locking dependency versions down to their own needs.

Should it be documented as a 'best practice' to do re-exports for everything that downstream crates may interact with?
Or, should we consider some kind of sugar version specification e.g. A = "B" meaning use the same version of A as B does.

Concrete example here

@emberian
Copy link
Member

I'm not sure this is super a cargo issue, but it'd be cool if cargo could pass in some metadata that rustc would print for each particular version of a crate. So you'd see stuff like foo(version 1.2.0)::baz::Bar

@larsbergstrom
Copy link

I agree with @cmr. I am in the middle of a multi-day Cargo debugging session for Servo right now, which mainly comes down to errors of that form (we have many transitive dependencies on libraries like x11-rs, and a small change to it has horrific rippling changes).

@mitchmindtree
Copy link

+1 to @cmr 's suggestion, it's especially mysterious the first time a user may come across it and some cargo support could go a long way. The extra metadata would only have to appear in situations where multiple versions of the same crate are depended upon in some way.

@alexcrichton
Copy link
Member

The error message should indeed be fixed, but that's pretty orthogonal to this bug report I believe. I would consider it under the purview of rust-lang/rust#15142.

@Ryman this use case sounds like it's largely a problem with B for having such a tied down dependency on A and yet relying on so much of its API being exposed. Cargo could try to reduce the number of versions of A in the dependency graph, but that's not always the right answer as often times you want the most up-to-date version of A for everything but one small portion of the graph.

Should it be documented as a 'best practice' to do re-exports for everything that downstream crates may interact with?

I don't think this is going to be a best practice.

Or, should we consider some kind of sugar version specification e.g. A = "B" meaning use the same version of A as B does.

This is also quite difficult to do as it makes the dependencies of B part of the public API of the crate. I would expect B to also have to explicitly opt-in to this behavior if necessary to ensure that it's clearly messaged that dropping a dependency on A is a breaking change.

@shepmaster
Copy link
Member

I don't think this is going to be a best practice.

This is also quite difficult to do as it makes the dependencies of B part of the public API of the crate.

@alexcrichton could you expand on these two points a bit? A question popped up that boiled down to nickel using hyper 0.6 and then hyper 0.7 was included from the Cargo.toml.

My first thought was "I wonder if there's a nickel::hyper re-export of everything". My second thought was "hmm, I have to look at the Cargo.lock file and copy the version; I wish I could just say "use the same".

@alexcrichton
Copy link
Member

Ah, the landscape here has changed quite a bit since I last looked at this. We plan to write up an RFC soon about some changes to Cargo to help make errors like this a little more precise and tweak how resolution works, but to answer your specific questions:

I don't think "just reexporting" is going to help much because you still have a problem that nickel is using hyper 0.6 and I'm using hyper 0.7. The only way for the types to agree is for me to link to hyper 0.7 or for me to exclusively use hyper through nickel (which seems unidiomatic). The best thing for cargo to do here actually is to detect this situation and give you an error immediately saying that you can't have multiple versions of hyper like this (as it won't compile).

In terms of making dependencies part of the public API of a crate, that's actually something we do intend to basically do. If you export any dependency's type from your API, then any major version bump in your dependencies must force a major version bump of yourself. This is often forgotten and often leads to problems as well.

@shepmaster
Copy link
Member

for me to exclusively use hyper through nickel (which seems unidiomatic)

Let's pretend there's these two chains of dependencies:

Hyper <- Nickel <- My binary
Hyper <- Cool library <- My binary 

If we just follow line one, I'd definitely want the ability to say "use what nickel thinks hyper should be". However, once I bring in the "cool library" that builds atop hyper and knows nothing of nickel, then that wouldn't make much sense.

So I think I agree with your assertion that just re-exporting wouldn't be generally useful.

If you export any dependency's type from your API

This seems like the right trigger, but I'm not sure if erroring would be always appropriate.

Perhaps what could happen is if you export a type that comes from one of your dependencies, then Cargo will also be able to use that version as a version specifier. Something like this:

Hyper <- Nickel <- Binary
Hyper <- CoolLib <- Binary
Hyper <- RestApiLibrary <- Binary

In this example, I am exposing two orthogonal APIs, one hand-rolled and one powered by "RestApiLibrary". I don't care what version of hyper RestApiLibrary uses, even though it exposes some types from hyper. My Cargo.toml could look something like:

[dependencies]
nickel = "*" # Use the newest cause I'm rad like that
cool_lib = "hyper::nickel" # A version of that depends on the same version of hyper that nickel uses
rest_api_library = "*" # Whatever is good

This would give a lot of flexibility, but that flexibility comes with a pretty high complexity. Perhaps it's not worth it?

Maybe it would just be better to allow multiple versions when a crate has a completely "sealed" usage of a dependency? I wonder if you could "seal" a crate after-the-fact? In the example above, could I somehow say

[dependencies,rest_api_library]
sealed = true
version = "*"

And thus prevent using any re-exported types from this crate (or at least acknowledge that I am ok with there being conflicting types).

This is hard stuff to think about! Sorry if I'm just blabbing on! 👼

@alexcrichton
Copy link
Member

Yeah that's actually what we were thinking as well :)

The rest-api-library crate would declare its dependency on hyper as private (e.g. just an internal implementation detail) which would in theory allow that hyper to be duplicated. I'd personally prefer to avoid the case of explicitly telling Cargo "use the version this library is using" and instead just have it automatically deduce that, and I think it should be possible at least.

@shepmaster
Copy link
Member

The rest-api-library crate would declare its dependency on hyper as private

This sounds close, but is a bit different than what I was thinking. We may have to pick somewhere in the middle...

Ideally, I would hope that during compilation of a crate, we could detect if it exposes any types that came from another crate. That would then automatically set the "just an implementation detail" flag. This would have to be on a dependency-by-dependency basis, otherwise the standard library crate will break this scheme for everything!

This could be extended to have a flag in Cargo.toml that states a dependency is internal-only. This would cause a compilation error if the programmer accidentally exposed a type. Perhaps the flag should go the other way though, and you have to opt-in to exposing types.

However, I think that the consumer of a crate should also be able to say that a dependency of a dependency is "just an implementation detail". This would allow me to use a crate that did expose types from its dependency as if it didn't expose them. It would then be up to me to ensure that I don't misuse those types.

I'm looking forward to the RFC or whatever where some of these hard ideas get debated in a broader setting!

@alexcrichton
Copy link
Member

Hm yeah that's a good point that any restriction Cargo places should likely be overridable from the top level as well to say "no, I know what I'm doing". I'll try to remember to cc this when that RFC comes into existence though!

@pnkfelix
Copy link
Member

cc me

@shepmaster
Copy link
Member

This is coming up again recently as the ecosystem slowly shifts to Serde 1.0 and some dependencies are pinned to 0.8 or 0.9. Sometimes crates have requirements on all three conflicting versions!

@carols10cents
Copy link
Member

I think this issue is addressed with RFC #1977, public/private dependencies. Please reopen if I'm mistaken!

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

8 participants