Auto merge of #9639 - djmitche:issue9535, r=Eh2406

Prefer patched versions of dependencies

When selecting among several versions of a paackage, prefer versions
from `[patch]` sections over other versions, similar to how locked
versions are preferred.

Patches come in the form of a Dependency and not a PackageId, so this
preference is expressed with `prefer_patch_deps`, distinct from
`try_to_use`.

Fixes #9535

Author: bors

crates/resolver-tests/src/lib.rs

@@ -184,6 +184,7 @@ pub fn resolve_with_config_raw(
         &[],
         &mut registry,
         &HashSet::new(),
+        &HashMap::new(),
         Some(config),
         true,
     );

src/cargo/core/resolver/dep_cache.rs

@@ -29,6 +29,7 @@ pub struct RegistryQueryer<'a> {
     pub registry: &'a mut (dyn Registry + 'a),
     replacements: &'a [(PackageIdSpec, Dependency)],
     try_to_use: &'a HashSet<PackageId>,
+    prefer_patch_deps: &'a HashMap<InternedString, HashSet<Dependency>>,
     /// If set the list of dependency candidates will be sorted by minimal
     /// versions first. That allows `cargo update -Z minimal-versions` which will
     /// specify minimum dependency versions to be used.
@@ -49,12 +50,14 @@ impl<'a> RegistryQueryer<'a> {
         registry: &'a mut dyn Registry,
         replacements: &'a [(PackageIdSpec, Dependency)],
         try_to_use: &'a HashSet<PackageId>,
+        prefer_patch_deps: &'a HashMap<InternedString, HashSet<Dependency>>,
         minimal_versions: bool,
     ) -> Self {
         RegistryQueryer {
             registry,
             replacements,
             try_to_use,
+            prefer_patch_deps,
             minimal_versions,
             registry_cache: HashMap::new(),
             summary_cache: HashMap::new(),
@@ -164,14 +167,22 @@ impl<'a> RegistryQueryer<'a> {
             }
         }
 
-        // 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.
+        // 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` or `prefer_patch_deps`) and failing that we list everything from the
+        // maximum version to the lowest version.
+        let should_prefer = |package_id: &PackageId| {
+            self.try_to_use.contains(package_id)
+                || self
+                    .prefer_patch_deps
+                    .get(&package_id.name())
+                    .map(|deps| deps.iter().any(|d| d.matches_id(*package_id)))
+                    .unwrap_or(false)
+        };
         ret.sort_unstable_by(|a, b| {
-            let a_in_previous = self.try_to_use.contains(&a.package_id());
-            let b_in_previous = self.try_to_use.contains(&b.package_id());
-            let previous_cmp = a_in_previous.cmp(&b_in_previous).reverse();
+            let prefer_a = should_prefer(&a.package_id());
+            let prefer_b = should_prefer(&b.package_id());
+            let previous_cmp = prefer_a.cmp(&prefer_b).reverse();
             match previous_cmp {
                 Ordering::Equal => {
                     let cmp = a.version().cmp(b.version());

src/cargo/core/resolver/mod.rs

@@ -58,6 +58,7 @@ use crate::core::PackageIdSpec;
 use crate::core::{Dependency, PackageId, Registry, Summary};
 use crate::util::config::Config;
 use crate::util::errors::CargoResult;
+use crate::util::interning::InternedString;
 use crate::util::profile;
 
 use self::context::Context;
@@ -106,6 +107,10 @@ mod types;
 ///   when sorting candidates to activate, but otherwise this isn't used
 ///   anywhere else.
 ///
+/// * `prefer_patch_deps` - this is a collection of dependencies on `[patch]`es, which
+///   should be preferred much like `try_to_use`, but are in the form of Dependencies
+///   and not PackageIds.
+///
 /// * `config` - a location to print warnings and such, or `None` if no warnings
 ///   should be printed
 ///
@@ -124,6 +129,7 @@ pub fn resolve(
     replacements: &[(PackageIdSpec, Dependency)],
     registry: &mut dyn Registry,
     try_to_use: &HashSet<PackageId>,
+    prefer_patch_deps: &HashMap<InternedString, HashSet<Dependency>>,
     config: Option<&Config>,
     check_public_visible_dependencies: bool,
 ) -> CargoResult<Resolve> {
@@ -133,7 +139,13 @@ pub fn resolve(
         Some(config) => config.cli_unstable().minimal_versions,
         None => false,
     };
-    let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions);
+    let mut registry = RegistryQueryer::new(
+        registry,
+        replacements,
+        try_to_use,
+        prefer_patch_deps,
+        minimal_versions,
+    );
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut cksums = HashMap::new();

src/cargo/ops/resolve.rs

@@ -27,7 +27,7 @@ use crate::util::errors::CargoResult;
 use crate::util::{profile, CanonicalUrl};
 use anyhow::Context as _;
 use log::{debug, trace};
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 
 /// Result for `resolve_ws_with_opts`.
 pub struct WorkspaceResolve<'cfg> {
@@ -242,11 +242,23 @@ pub fn resolve_with_previous<'cfg>(
             }
     };
 
-    // This is a set of PackageIds of `[patch]` entries that should not be
-    // locked.
+    // This is a set of PackageIds of `[patch]` entries, and some related locked PackageIds, for
+    // which locking should be avoided (but which will be preferred when searching dependencies,
+    // via prefer_patch_deps below)
     let mut avoid_patch_ids = HashSet::new();
+
+    // This is a set of Dependency's of `[patch]` entries that should be preferred when searching
+    // dependencies.
+    let mut prefer_patch_deps = HashMap::new();
+
     if register_patches {
         for (url, patches) in ws.root_patch()?.iter() {
+            for patch in patches {
+                prefer_patch_deps
+                    .entry(patch.package_name())
+                    .or_insert_with(HashSet::new)
+                    .insert(patch.clone());
+            }
             let previous = match previous {
                 Some(r) => r,
                 None => {
@@ -353,6 +365,7 @@ pub fn resolve_with_previous<'cfg>(
         }
     }
     debug!("avoid_patch_ids={:?}", avoid_patch_ids);
+    debug!("prefer_patch_deps={:?}", prefer_patch_deps);
 
     let keep = |p: &PackageId| pre_patch_keep(p) && !avoid_patch_ids.contains(p);
 
@@ -426,6 +439,7 @@ pub fn resolve_with_previous<'cfg>(
         &replace,
         registry,
         &try_to_use,
+        &prefer_patch_deps,
         Some(ws.config()),
         ws.unstable_features()
             .require(Feature::public_dependency())

tests/testsuite/patch.rs

@@ -441,6 +441,55 @@ fn unused() {
     );
 }
 
+#[cargo_test]
+fn prefer_patch_version() {
+    Package::new("bar", "0.1.2").publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.0.1"
+                authors = []
+
+                [dependencies]
+                bar = "0.1.0"
+
+                [patch.crates-io]
+                bar = { path = "bar" }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1"))
+        .file("bar/src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_stderr(
+            "\
+[UPDATING] `[ROOT][..]` index
+[COMPILING] bar v0.1.1 ([CWD]/bar)
+[COMPILING] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+    p.cargo("build")
+        .with_stderr(
+            "\
+[FINISHED] [..]
+",
+        )
+        .run();
+
+    // there should be no patch.unused in the toml file
+    let lock = p.read_lockfile();
+    let toml: toml::Value = toml::from_str(&lock).unwrap();
+    assert!(toml.get("patch").is_none());
+}
+
 #[cargo_test]
 fn unused_from_config() {
     Package::new("bar", "0.1.0").publish();