rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.75k stars 2.42k forks source link

target/test/deps and target/doc-build/deps are redundant with target/deps #348

Closed SimonSapin closed 10 years ago

SimonSapin commented 10 years ago

When running cargo build, cargo test, and cargo doc on the same packages, each command build its own copy of the dependencies. According to the --verbose output, the rustc commands being run are exactly the same in each case, except for the target paths.

Is there a reason for duplicating the dependency builds, or could they be shared? (I’m mostly worried about build times.)

alexcrichton commented 10 years ago

The thought behind this decision was that these three modes are likely subtly different and are not guaranteed to be compatible with each other (looking forward to various configuration in the future). We could in theory have one giant cache locally which everything looks up in, but this was easier to implement in the meantime.

@wycats, care to weigh in as well?

wycats commented 10 years ago

More than just "subtly different", the different environment already have some different flags, and are likely to grow different flags in the future. Also in the future, we will support configurable profiles. I'd rather avoid subtle bugs in the future due to differences between the profiles.

wycats commented 10 years ago

example problem: "why is my Rust code so slow when running cargo bench?". 20 hours later: "fuuuuuuuuu it was caching my dependencies from dev mode with optimizations disabled".

alexcrichton commented 10 years ago

For now I'm going to close this as working as intended, but we can revisit this in the future if it becomes a pressing problem.

SimonSapin commented 10 years ago

We’ll have to try to be sure, but I expect it’ll be a problem for Servo where doubling or tripling build times is not acceptable. (Servo’s "main" crate is fairly small, it’s almost all in dependencies.)

Sure, if you specify different profiles with different build flags, you’ll not want to mix the build results. But that doesn’t mean that the default has to be repeating the exact same work three times.

metajack commented 10 years ago

Simon is right. Currently we build servo, then compile test crates, and then run rustdoc. None of this has a separate set of build flags other than the --test. We don't currently run cargo bench so we are obviously running tests either optimized or deoptimized depending on how the toplevel project was configured.

This means that a Travis CI run of Servo will build all dependencies 3 times, one of those dependencies is Spidermonkey. Skia is also not a tiny compile. This won't quite result in tripling our integration cycle, but I suspect it will be north of 2x as long. Since we're already splitting across 3-4 machines to keep it under 50 minutes total, this is not likely to be acceptable.

metajack commented 10 years ago

@alexcrichton pointed out that this is technically only two extra times, since cargo test can build the servo binary and run the tests. So we can skip cargo build altogether. That's still doubling the build part of the integration time.

wycats commented 10 years ago

It sounds like basing the builds on a hash of the Profile's flags is the right thing. @alexcrichton how hard do you think that will be?

alexcrichton commented 10 years ago

I'm sure we can work something out!

SimonSapin commented 10 years ago

Works great, thanks @alexcrichton !