Skip to content

Commit

Permalink
Prefer patched versions of dependencies
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
djmitche committed Jul 9, 2021
1 parent cc80b40 commit bd4a353
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 11 deletions.
1 change: 1 addition & 0 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ pub fn resolve_with_config_raw(
&[],
&mut registry,
&HashSet::new(),
&HashMap::new(),
Some(config),
true,
);
Expand Down
25 changes: 18 additions & 7 deletions src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(),
Expand Down Expand Up @@ -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());
Expand Down
14 changes: 13 additions & 1 deletion src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
///
Expand All @@ -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> {
Expand All @@ -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();
Expand Down
20 changes: 17 additions & 3 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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())
Expand Down
49 changes: 49 additions & 0 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit bd4a353

Please sign in to comment.