rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.97k stars 12.53k forks source link

Transitive crate dependency resolution ignores metadata #2138

Closed brson closed 12 years ago

brson commented 12 years ago

We have crate A, B, and C, A depends on B and B depends on C. When building crate A rustc finds the library for crate B based on the metadata, then it goes through and finds all the dependencies of crate B, etc. Once it recurses and starts resolving dependencies of dependencies it stops doing any metadata matching and just looks for the name of the crate. This could easily result in situations where rustc loads the wrong crate.

To fix this we need to encode all the matched metadata for a crate's dependencies.

brson commented 12 years ago

We actually need to know the exact version of dependencies of dependencies, and the real rules for making these resolve correctly is going to a bit more subtle than I can imagine just now.

graydon commented 12 years ago

It should not be scanning metadata our resolving dependencies past the A -> B edge. The dependency from B to C was already resolved to a specific mangled library name during compilation of B, and the bound library name should be integrated (through hashing, along with the metadata of B) into the mangled library name of B.

At least, if I understand this bug correctly.

brson commented 12 years ago

We need to know the exact version of C that B depends on at compile time because we have to load its metadata. Currently when compiling A, we see that C is crate 'foo' and then locate any crate called 'foo' to read the metadata from. What we really need to do is see that C is crate 'foo-HASH-VERSION' and locate that exact crate.

graydon commented 12 years ago

I agree that crate B should export the fact that it depends on foo-HASH-VERSION. That's the "specific mangled name" I referred to above. But I don't think (or am not sure why) B needs to re-export any of C's metadata beyond that foo-HASH-VERSION dependency. For that matter, I don't get why A would need metadata from C at all.

(Also, I thought lht did this stuff already, but I guess he only did transitive name-dependencies, not foo-HASH-VERSION?)

brson commented 12 years ago

B doesn't need to reexport C's link metadata. I thought that it did originally.

A need's C's metadata because B's public interface may refer to types (and maybe other things?) that live in C and that A needs to know about as well.

lht commented 12 years ago

Previously, I only implemented transitive hash calculation.

7aaa120 adds encoding version and hash of dependent crates, and checks versions when searching for crate.

Another thing is if B reexports types or functions from C, A needs C's metadata?

brson commented 12 years ago

Nice work @lht. From what I can tell this issue is fixed. I've logged the only remaining related problem I'm aware of as #2155.

brson commented 12 years ago

Crate resolution and linking now works much better. Here's an example (from a test) of the kinds of things that we can do now:

// These both have the same version but differ in other metadata
use cr6_1 (name = "crateresolve6", vers = "0.1", calories="100");
use cr6_2 (name = "crateresolve6", vers = "0.1", calories="200");

fn main() {
    assert cr6_1::f() == 100;
    assert cr6_2::f() == 200;
}
lht commented 12 years ago

One more thing left is checking hashes of indirect dependent crates. I'll work on this.

And we may also avoid the glob search when finding dependents' dependent crates, given we now have hash and version information.

lht commented 12 years ago

Close this one after commiting 7d227f2.