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

Cargo requires a registry update when adding dep that already exists in dag #2895

Closed
brson opened this issue Jul 19, 2016 · 2 comments · Fixed by #3144
Closed

Cargo requires a registry update when adding dep that already exists in dag #2895

brson opened this issue Jul 19, 2016 · 2 comments · Fixed by #3144

Comments

@brson
Copy link
Contributor

brson commented Jul 19, 2016

Starting with two projects, with foo depending on bar:

[package]
name = "foo"
version = "0.1.0"
authors = ["Brian Anderson <banderson@mozilla.com>"]

[dependencies]
bar = { path = "bar" }
[package]
name = "bar"
version = "0.1.0"
authors = ["Brian Anderson <banderson@mozilla.com>"]

[dependencies]
libc = "0.2.0"

If I then add libc = "0.2.0" to foo/Cargo.toml Cargo will sync the registry. It already has all the info it needs to solve the graph though. Moving dependencies around in the graph is common in large projects, so Cargo should recognize this case and not hit the network.

@brson
Copy link
Contributor Author

brson commented Jul 19, 2016

cc @wycats After some experimenting with a few dag-modifying ops this is the only one I found that still requires a registry sync. Better than I thought.

@aidanhs
Copy link
Member

aidanhs commented Jul 20, 2016

+1 for me (per my comment at #2111 (comment))

Also want to respond to #2811 (comment)

The various flags proposed in this PR assert that you won't need to hit the network. In general, if you don't need to hit the network but end up blocked on proceeding due to the lack of network that is a bug in Cargo.

"doesn't need to hit the network" is a little fuzzy - currently cargo says "I need to hit the network to write out a correct new Cargo.lock". I believe this is sane default behaviour (otherwise builds depend on what's already been cached). However

  1. sometimes people want to say "just assume the index you have is fine" because they have appropriate packages cached
  2. more subtly, sometimes people want to say "try really hard to build this package using the cache you have available", e.g. if I have greatlib 1.0.0 cached, but an index update has downloaded the info for greatlib 1.0.1, I actually don't want cargo to try and fetch greatlib 1.0.1

2 seems very hard and potentially extremely slow if no limits are put in place for cargo searching for things, but I can't currently think of a situation where just 1 would be better than 1+2.

As a result, I'd like to propose a new flag called --dev-working-offline (or whatever) and the intent is that this flag would be an umbrella for anything that makes cargo to try really hard to work offline - initially just 1, then much later 2 and potential others.

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Sep 30, 2016
Cargo previously erroneously updated the registry whenever a new dependency was
added on a crate which already exists in the DAG. This commit fixes this
behavior by ensuring that if the new dependency matches a previously locked
version it uses that instead.

This commit involved a bit of refactoring around this logic to be a bit more
clear how the locking and "falling back to the registry" is happening.

Closes rust-lang#2895
Closes rust-lang#2931
bors added a commit that referenced this issue Oct 5, 2016
Avoid updating registry when adding existing deps

Cargo previously erroneously updated the registry whenever a new dependency was
added on a crate which already exists in the DAG. This commit fixes this
behavior by ensuring that if the new dependency matches a previously locked
version it uses that instead.

This commit involved a bit of refactoring around this logic to be a bit more
clear how the locking and "falling back to the registry" is happening.

Closes #2895
@bors bors closed this as completed in #3144 Oct 6, 2016
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 a pull request may close this issue.

2 participants