Don't abort resolution on transitive updates

This commit is directed at fixing #4127, allowing the resolver to automatically
perform transitive updates when required. A few use casese and tagged links are
hanging off #4127 itself, but the crux of the issue happens when you either add
a dependency or update a version requirement in `Cargo.toml` which conflicts
with something listed in your `Cargo.lock`. In this case Cargo would previously
provide an obscure "cannot resolve" error whereas this commit updates Cargo to
automatically perform a conservative re-resolution of the dependency graph.

It's hoped that this commit will help reduce the number of "unresolvable"
dependency graphs we've seen in the wild and otherwise make Cargo a little more
ergonomic to use as well. More details can be found in the source's comments!

Closes #4127
Closes #5182

src/cargo/core/dependency.rs

@@ -343,9 +343,8 @@ impl Dependency {
     }
 
     /// Returns true if the package (`sum`) can fulfill this dependency request.
-    pub fn matches_ignoring_source(&self, sum: &Summary) -> bool {
-        self.name() == sum.package_id().name()
-            && self.version_req().matches(sum.package_id().version())
+    pub fn matches_ignoring_source(&self, id: &PackageId) -> bool {
+        self.name() == id.name() && self.version_req().matches(id.version())
     }
 
     /// Returns true if the package (`id`) can fulfill this dependency request.

src/cargo/core/package.rs

@@ -12,7 +12,6 @@ use lazycell::LazyCell;
 use core::{Dependency, Manifest, PackageId, SourceId, Target};
 use core::{SourceMap, Summary};
 use core::interning::InternedString;
-use ops;
 use util::{internal, lev_distance, Config};
 use util::errors::{CargoResult, CargoResultExt};
 

src/cargo/core/registry.rs

@@ -434,7 +434,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
                 patches.extend(
                     extra
                         .iter()
-                        .filter(|s| dep.matches_ignoring_source(s))
+                        .filter(|s| dep.matches_ignoring_source(s.package_id()))
                         .cloned(),
                 );
             }

src/cargo/core/resolver/encode.rs

@@ -6,7 +6,7 @@ use serde::ser;
 use serde::de;
 
 use core::{Dependency, Package, PackageId, SourceId, Workspace};
-use util::{internal, Config, Graph};
+use util::{internal, Graph};
 use util::errors::{CargoError, CargoResult, CargoResultExt};
 
 use super::Resolve;
@@ -188,7 +188,7 @@ impl EncodableResolve {
     }
 }
 
-fn build_path_deps(ws: &Workspace) -> (HashMap<String, SourceId>) {
+fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
     // If a crate is *not* a path source, then we're probably in a situation
     // such as `cargo install` with a lock file from a remote dependency. In
     // that case we don't need to fixup any path dependencies (as they're not

src/cargo/core/resolver/mod.rs

@@ -384,10 +384,39 @@ struct Context {
 type Activations = HashMap<(InternedString, SourceId), Rc<Vec<Summary>>>;
 
 /// Builds the list of all packages required to build the first argument.
+///
+/// * `summaries` - the list of package summaries along with how to resolve
+///   their features. This is a list of all top-level packages that are intended
+///   to be part of the lock file (resolve output). These typically are a list
+///   of all workspace members.
+///
+/// * `replacements` - this is a list of `[replace]` directives found in the
+///   root of the workspace. The list here is a `PackageIdSpec` of what to
+///   replace and a `Dependency` to replace that with. In general it's not
+///   recommended to use `[replace]` any more and use `[patch]` instead, which
+///   is supported elsewhere.
+///
+/// * `registry` - this is the source from which all package summaries are
+///   loaded. It's expected that this is extensively configured ahead of time
+///   and is idempotent with our requests to it (aka returns the same results
+///   for the same query every time). Typically this is an instance of a
+///   `PackageRegistry`.
+///
+/// * `try_to_use` - this is a list of package ids which were previously found
+///   in the lock file. We heuristically prefer the ids listed in `try_to_use`
+///   when sorting candidates to activate, but otherwise this isn't used
+///   anywhere else.
+///
+/// * `config` - a location to print warnings and such, or `None` if no warnings
+///   should be printed
+///
+/// * `print_warnings` - whether or not to print backwards-compatibility
+///   warnings and such
 pub fn resolve(
     summaries: &[(Summary, Method)],
     replacements: &[(PackageIdSpec, Dependency)],
     registry: &mut Registry,
+    try_to_use: &[&PackageId],
     config: Option<&Config>,
     print_warnings: bool,
 ) -> CargoResult<Resolve> {
@@ -400,12 +429,8 @@ pub fn resolve(
         warnings: RcList::new(),
     };
     let _p = profile::start("resolving");
-    let cx = activate_deps_loop(
-        cx,
-        &mut RegistryQueryer::new(registry, replacements),
-        summaries,
-        config,
-    )?;
+    let mut registry = RegistryQueryer::new(registry, replacements, try_to_use);
+    let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut resolve = Resolve {
         graph: cx.graph(),
@@ -637,16 +662,22 @@ impl ConflictReason {
 struct RegistryQueryer<'a> {
     registry: &'a mut (Registry + 'a),
     replacements: &'a [(PackageIdSpec, Dependency)],
+    try_to_use: HashSet<&'a PackageId>,
     // TODO: with nll the Rc can be removed
     cache: HashMap<Dependency, Rc<Vec<Candidate>>>,
 }
 
 impl<'a> RegistryQueryer<'a> {
-    fn new(registry: &'a mut Registry, replacements: &'a [(PackageIdSpec, Dependency)]) -> Self {
+    fn new(
+        registry: &'a mut Registry,
+        replacements: &'a [(PackageIdSpec, Dependency)],
+        try_to_use: &'a [&'a PackageId],
+    ) -> Self {
         RegistryQueryer {
             registry,
             replacements,
             cache: HashMap::new(),
+            try_to_use: try_to_use.iter().cloned().collect(),
         }
     }
 
@@ -739,9 +770,17 @@ impl<'a> RegistryQueryer<'a> {
             candidate.replace = replace;
         }
 
-        // When we attempt versions for a package, we'll want to start at
-        // the maximum version and work our way down.
-        ret.sort_unstable_by(|a, b| b.summary.version().cmp(a.summary.version()));
+        // When we attempt versions for a package we'll want to do so in a
+        // sorted fashion to pick the "best candidates" first. Currently we try
+        // prioritized summaries (those in `try_to_use`) and failing that we
+        // list everything from the maximum version to the lowest version.
+        ret.sort_unstable_by(|a, b| {
+            let a_in_previous = self.try_to_use.contains(a.summary.package_id());
+            let b_in_previous = self.try_to_use.contains(b.summary.package_id());
+            let a = (a_in_previous, a.summary.version());
+            let b = (b_in_previous, b.summary.version());
+            a.cmp(&b).reverse()
+        });
 
         let out = Rc::new(ret);
 

src/cargo/core/workspace.rs

@@ -7,9 +7,11 @@ use std::slice;
 use glob::glob;
 use url::Url;
 
-use core::{Dependency, PackageIdSpec, Profile, Profiles};
+use core::registry::PackageRegistry;
 use core::{EitherManifest, Package, SourceId, VirtualManifest};
+use core::{Dependency, PackageIdSpec, Profile, Profiles};
 use ops;
+use sources::PathSource;
 use util::errors::{CargoResult, CargoResultExt};
 use util::paths;
 use util::toml::read_manifest;
@@ -70,6 +72,8 @@ pub struct Workspace<'cfg> {
     // cases `false` also results in the non-enforcement of dev-dependencies.
     require_optional_deps: bool,
 
+    // A cache of lodaed packages for particular paths which is disjoint from
+    // `packages` up above, used in the `load` method down below.
     loaded_packages: RefCell<HashMap<PathBuf, Package>>,
 }
 
@@ -692,6 +696,37 @@ impl<'cfg> Workspace<'cfg> {
         loaded.insert(manifest_path.to_path_buf(), package.clone());
         Ok(package)
     }
+
+    /// Preload the provided registry with already loaded packages.
+    ///
+    /// A workspace may load packages during construction/parsing/early phases
+    /// for various operations, and this preload step avoids doubly-loading and
+    /// parsing crates on the filesystem by inserting them all into the registry
+    /// with their in-memory formats.
+    pub fn preload(&self, registry: &mut PackageRegistry<'cfg>) {
+        // These can get weird as this generally represents a workspace during
+        // `cargo install`. Things like git repositories will actually have a
+        // `PathSource` with multiple entries in it, so the logic below is
+        // mostly just an optimization for normal `cargo build` in workspaces
+        // during development.
+        if self.is_ephemeral {
+            return;
+        }
+
+        for pkg in self.packages.packages.values() {
+            let pkg = match *pkg {
+                MaybePackage::Package(ref p) => p.clone(),
+                MaybePackage::Virtual(_) => continue,
+            };
+            let mut src = PathSource::new(
+                pkg.manifest_path(),
+                pkg.package_id().source_id(),
+                self.config,
+            );
+            src.preload_with(pkg);
+            registry.add_preloaded(Box::new(src));
+        }
+    }
 }
 
 impl<'cfg> Packages<'cfg> {

src/cargo/ops/cargo_rustc/mod.rs

@@ -1082,7 +1082,7 @@ fn build_deps_args<'a, 'cfg>(
                 .pkg
                 .dependencies()
                 .iter()
-                .filter(|d| d.matches_ignoring_source(dep.pkg.summary()))
+                .filter(|d| d.matches_ignoring_source(dep.pkg.package_id()))
                 .filter_map(|d| d.rename())
                 .next();