Skip to content

Prefer patched versions of dependencies #9639

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

Merged
merged 1 commit into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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