diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index b39bbd18f39..d3c5d545e9a 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -6,8 +6,8 @@ use core::{Dependency, FeatureValue, PackageId, SourceId, Summary}; use util::CargoResult; use util::Graph; -use super::types::RegistryQueryer; -use super::types::{ActivateResult, ConflictReason, DepInfo, GraphNode, Method, RcList}; +use super::types::{ConflictReason, DepInfo, GraphNode, Method, RcList, RegistryQueryer}; +use super::errors::ActivateResult; pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve}; pub use super::encode::{Metadata, WorkspaceResolve}; @@ -186,11 +186,10 @@ impl Context { // name. let base = reqs.deps.get(&dep.name_in_toml()).unwrap_or(&default_dep); used_features.insert(dep.name_in_toml()); - let always_required = !dep.is_optional() - && !s - .dependencies() - .iter() - .any(|d| d.is_optional() && d.name_in_toml() == dep.name_in_toml()); + let always_required = !dep.is_optional() && !s + .dependencies() + .iter() + .any(|d| d.is_optional() && d.name_in_toml() == dep.name_in_toml()); if always_required && base.0 { self.warnings.push(format!( "Package `{}` does not have feature `{}`. It has a required dependency \ @@ -230,11 +229,13 @@ impl Context { "Package `{}` does not have these features: `{}`", s.package_id(), features - ).into(), + ) + .into(), Some(p) => ( p.package_id().clone(), ConflictReason::MissingFeatures(features), - ).into(), + ) + .into(), }); } diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 33704ba16a1..c80ce59dfee 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -377,7 +377,8 @@ impl<'a, 'cfg> ser::Serialize for WorkspaceResolve<'a, 'cfg> { root: None, metadata, patch, - }.serialize(s) + } + .serialize(s) } } diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 74979c6df09..fbfe5e79e8c 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -4,8 +4,8 @@ use std::fmt; use core::{Dependency, PackageId, Registry, Summary}; use failure::{Error, Fail}; use semver; -use util::config::Config; use util::lev_distance::lev_distance; +use util::{CargoError, Config}; use super::context::Context; use super::types::{Candidate, ConflictReason}; @@ -49,6 +49,25 @@ impl fmt::Display for ResolveError { } } +pub type ActivateResult = Result; + +pub enum ActivateError { + Fatal(CargoError), + Conflict(PackageId, ConflictReason), +} + +impl From<::failure::Error> for ActivateError { + fn from(t: ::failure::Error) -> Self { + ActivateError::Fatal(t) + } +} + +impl From<(PackageId, ConflictReason)> for ActivateError { + fn from(t: (PackageId, ConflictReason)) -> Self { + ActivateError::Conflict(t.0, t.1) + } +} + pub(super) fn activation_error( cx: &Context, registry: &mut Registry, diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 196c25016fb..851dcd85b8f 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -62,21 +62,21 @@ use util::errors::CargoResult; use util::profile; use self::context::{Activations, Context}; -use self::types::{ActivateError, ActivateResult, Candidate, ConflictReason, DepsFrame, GraphNode}; +use self::types::{Candidate, ConflictReason, DepsFrame, GraphNode}; use self::types::{RcVecIter, RegistryQueryer, RemainingDeps, ResolverProgress}; pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve}; pub use self::encode::{Metadata, WorkspaceResolve}; +pub use self::errors::{ActivateError, ActivateResult, ResolveError}; pub use self::resolve::Resolve; pub use self::types::Method; -pub use self::errors::ResolveError; mod conflict_cache; mod context; mod encode; +mod errors; mod resolve; mod types; -mod errors; /// Builds the list of all packages required to build the first argument. /// @@ -403,7 +403,8 @@ fn activate_deps_loop( .clone() .filter_map(|(_, (ref new_dep, _, _))| { past_conflicting_activations.conflicting(&cx, new_dep) - }).next() + }) + .next() { // If one of our deps is known unresolvable // then we will not succeed. @@ -438,12 +439,15 @@ fn activate_deps_loop( // for deps related to us .filter(|&(_, ref other_dep)| { known_related_bad_deps.contains(other_dep) - }).filter_map(|(other_parent, other_dep)| { + }) + .filter_map(|(other_parent, other_dep)| { past_conflicting_activations .find_conflicting(&cx, &other_dep, |con| { con.contains_key(&pid) - }).map(|con| (other_parent, con)) - }).next() + }) + .map(|con| (other_parent, con)) + }) + .next() { let rel = conflict.get(&pid).unwrap().clone(); @@ -485,7 +489,8 @@ fn activate_deps_loop( &parent, backtracked, &conflicting_activations, - ).is_none() + ) + .is_none() } }; diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index dff80107f07..14e37c54447 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -171,6 +171,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated self.graph.contains(k) } + pub fn sort(&self) -> Vec { + self.graph.sort() + } + pub fn iter(&self) -> impl Iterator { self.graph.iter() } diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index b9494546db7..0f39783a99e 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -6,7 +6,8 @@ use std::time::{Duration, Instant}; use core::interning::InternedString; use core::{Dependency, PackageId, PackageIdSpec, Registry, Summary}; -use util::{CargoError, CargoResult, Config}; +use util::errors::CargoResult; +use util::Config; pub struct ResolverProgress { ticks: u16, @@ -348,25 +349,6 @@ impl RemainingDeps { // (dependency info, candidates, features activated) pub type DepInfo = (Dependency, Rc>, Rc>); -pub type ActivateResult = Result; - -pub enum ActivateError { - Fatal(CargoError), - Conflict(PackageId, ConflictReason), -} - -impl From<::failure::Error> for ActivateError { - fn from(t: ::failure::Error) -> Self { - ActivateError::Fatal(t) - } -} - -impl From<(PackageId, ConflictReason)> for ActivateError { - fn from(t: (PackageId, ConflictReason)) -> Self { - ActivateError::Conflict(t.0, t.1) - } -} - /// All possible reasons that a package might fail to activate. /// /// We maintain a list of conflicts for error reporting as well as backtracking diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs index 78aa2b93f16..50df74f873e 100644 --- a/src/cargo/util/graph.rs +++ b/src/cargo/util/graph.rs @@ -1,5 +1,5 @@ use std::borrow::Borrow; -use std::collections::hash_map::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::hash::Hash; @@ -42,6 +42,30 @@ impl Graph { self.nodes.get(from).into_iter().flat_map(|x| x.iter()) } + /// A topological sort of the `Graph` + pub fn sort(&self) -> Vec { + let mut ret = Vec::new(); + let mut marks = HashSet::new(); + + for node in self.nodes.keys() { + self.sort_inner_visit(node, &mut ret, &mut marks); + } + + ret + } + + fn sort_inner_visit(&self, node: &N, dst: &mut Vec, marks: &mut HashSet) { + if !marks.insert(node.clone()) { + return; + } + + for child in self.nodes[node].keys() { + self.sort_inner_visit(child, dst, marks); + } + + dst.push(node.clone()); + } + pub fn iter(&self) -> impl Iterator { self.nodes.keys() } diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index afe06f5fe58..9ce7ccc4e42 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -1,14 +1,14 @@ use std::env; -use cargo::core::dependency::Kind::Development; +use cargo::core::dependency::Kind; use cargo::core::{enable_nightly_features, Dependency}; use cargo::util::Config; use support::project; use support::registry::Package; use support::resolver::{ - assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg, pkg_dep, - pkg_id, pkg_loc, registry, registry_strategy, resolve, resolve_and_validated, + assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, + pkg, pkg_dep, pkg_id, pkg_loc, registry, registry_strategy, resolve, resolve_and_validated, resolve_with_config, PrettyPrintRegistry, ToDep, ToPkgId, }; @@ -33,6 +33,7 @@ proptest! { }, .. ProptestConfig::default() })] + #[test] fn passes_validation( PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) @@ -49,6 +50,56 @@ proptest! { ); } } + + #[test] + fn minimum_version_errors_the_same( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + enable_nightly_features(); + + let mut config = Config::default().unwrap(); + config + .configure( + 1, + None, + &None, + false, + false, + &None, + &["minimal-versions".to_string()], + ) + .unwrap(); + + let reg = registry(input.clone()); + // there is only a small chance that eny one + // crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(10) { + // minimal-versions change what order the candidates + // are tried but not the existence of a solution + let res = resolve( + &pkg_id("root"), + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + ); + + let mres = resolve_with_config( + &pkg_id("root"), + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + Some(&config), + ); + + prop_assert_eq!( + res.is_ok(), + mres.is_ok(), + "minimal-versions and regular resolver disagree about weather `{} = \"={}\"` can resolve", + this.name(), + this.version() + ) + } + } + #[test] fn limited_independence_of_irrelevant_alternatives( PrettyPrintRegistry(input) in registry_strategy(50, 20, 60), @@ -219,15 +270,15 @@ fn test_resolving_with_same_name() { #[test] fn test_resolving_with_dev_deps() { let reg = registry(vec![ - pkg!("foo" => ["bar", dep_kind("baz", Development)]), - pkg!("baz" => ["bat", dep_kind("bam", Development)]), + pkg!("foo" => ["bar", dep_kind("baz", Kind::Development)]), + pkg!("baz" => ["bat", dep_kind("bam", Kind::Development)]), pkg!("bar"), pkg!("bat"), ]); let res = resolve( &pkg_id("root"), - vec![dep("foo"), dep_kind("baz", Development)], + vec![dep("foo"), dep_kind("baz", Kind::Development)], ®, ) .unwrap(); @@ -598,13 +649,13 @@ fn resolving_with_many_equivalent_backtracking() { let next = format!("level{}", l + 1); for i in 1..BRANCHING_FACTOR { let vsn = format!("1.0.{}", i); - reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")])); + reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep(next.as_str())])); } } let reg = registry(reglist.clone()); - let res = resolve(&pkg_id("root"), vec![dep_req("level0", "*")], ®); + let res = resolve(&pkg_id("root"), vec![dep("level0")], ®); assert!(res.is_err()); @@ -614,7 +665,7 @@ fn resolving_with_many_equivalent_backtracking() { let reg = registry(reglist.clone()); - let res = resolve(&pkg_id("root"), vec![dep_req("level0", "*")], ®).unwrap(); + let res = resolve(&pkg_id("root"), vec![dep("level0")], ®).unwrap(); assert_contains(&res, &names(&[("root", "1.0.0"), ("level0", "1.0.0")])); @@ -629,7 +680,7 @@ fn resolving_with_many_equivalent_backtracking() { let res = resolve( &pkg_id("root"), - vec![dep_req("level0", "*"), dep_req("constrained", "*")], + vec![dep("level0"), dep("constrained")], ®, ) .unwrap(); @@ -647,7 +698,7 @@ fn resolving_with_many_equivalent_backtracking() { let res = resolve( &pkg_id("root"), - vec![dep_req("level0", "1.0.1"), dep_req("constrained", "*")], + vec![dep_req("level0", "1.0.1"), dep("constrained")], ®, ) .unwrap(); @@ -686,7 +737,7 @@ fn resolving_with_deep_traps() { let next = format!("backtrack_trap{}", l + 1); for i in 1..BRANCHING_FACTOR { let vsn = format!("1.0.{}", i); - reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")])); + reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep(next.as_str())])); } } { @@ -708,7 +759,7 @@ fn resolving_with_deep_traps() { let res = resolve( &pkg_id("root"), - vec![dep_req("backtrack_trap0", "*"), dep_req("cloaking", "*")], + vec![dep("backtrack_trap0"), dep("cloaking")], ®, ); @@ -729,7 +780,7 @@ fn resolving_with_constrained_cousins_backtrack() { let next = format!("backtrack_trap{}", l + 1); for i in 1..BRANCHING_FACTOR { let vsn = format!("1.0.{}", i); - reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")])); + reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep(next.as_str())])); } } { @@ -761,9 +812,9 @@ fn resolving_with_constrained_cousins_backtrack() { let res = resolve( &pkg_id("root"), vec![ - dep_req("backtrack_trap0", "*"), + dep("backtrack_trap0"), dep_req("constrained", "2.0.1"), - dep_req("cloaking", "*"), + dep("cloaking"), ], ®, ); @@ -777,12 +828,12 @@ fn resolving_with_constrained_cousins_backtrack() { let next = format!("level{}", l + 1); for i in 1..BRANCHING_FACTOR { let vsn = format!("1.0.{}", i); - reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")])); + reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep(next.as_str())])); } } reglist.push( - pkg!((format!("level{}", DEPTH).as_str(), "1.0.0") => [dep_req("backtrack_trap0", "*"), - dep_req("cloaking", "*") + pkg!((format!("level{}", DEPTH).as_str(), "1.0.0") => [dep("backtrack_trap0"), + dep("cloaking") ]), ); @@ -790,7 +841,7 @@ fn resolving_with_constrained_cousins_backtrack() { let res = resolve( &pkg_id("root"), - vec![dep_req("level0", "*"), dep_req("constrained", "2.0.1")], + vec![dep("level0"), dep_req("constrained", "2.0.1")], ®, ); @@ -798,7 +849,7 @@ fn resolving_with_constrained_cousins_backtrack() { let res = resolve( &pkg_id("root"), - vec![dep_req("level0", "*"), dep_req("constrained", "2.0.0")], + vec![dep("level0"), dep_req("constrained", "2.0.0")], ®, ) .unwrap(); @@ -985,7 +1036,7 @@ fn incomplete_information_skiping_2() { dep("bad"), ]), pkg!(("h", "3.8.3") => [ - dep_req("g", "*"), + dep("g"), ]), pkg!(("h", "6.8.3") => [ dep("f"), @@ -994,10 +1045,10 @@ fn incomplete_information_skiping_2() { dep_req("to_yank", "=8.8.1"), ]), pkg!("i" => [ - dep_req("b", "*"), - dep_req("c", "*"), - dep_req("e", "*"), - dep_req("h", "*"), + dep("b"), + dep("c"), + dep("e"), + dep("h"), ]), ]; let reg = registry(input.clone()); @@ -1043,7 +1094,7 @@ fn incomplete_information_skiping_3() { ] }, pkg!{("b", "2.0.2") => [ dep_req("to_yank", "3.3.0"), - dep_req("a", "*"), + dep("a"), ] }, pkg!{("b", "2.3.3") => [ dep_req("to_yank", "3.3.0"), @@ -1052,7 +1103,7 @@ fn incomplete_information_skiping_3() { ]; let reg = registry(input.clone()); - let res = resolve(&pkg_id("root"), vec![dep_req("b", "*")], ®).unwrap(); + let res = resolve(&pkg_id("root"), vec![dep("b")], ®).unwrap(); let package_to_yank = ("to_yank", "3.0.3").to_pkgid(); // this package is not used in the resolution. assert!(!res.contains(&package_to_yank)); @@ -1066,7 +1117,7 @@ fn incomplete_information_skiping_3() { ); assert_eq!(input.len(), new_reg.len() + 1); // it should still build - assert!(resolve(&pkg_id("root"), vec![dep_req("b", "*")], &new_reg).is_ok()); + assert!(resolve(&pkg_id("root"), vec![dep("b")], &new_reg).is_ok()); } #[test] diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index 9b97daf0b0e..ae89eaf71f9 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -50,7 +50,7 @@ pub fn resolve_and_validated( })); } } - let out: Vec = resolve.iter().cloned().collect(); + let out = resolve.sort(); assert_eq!(out.len(), used.len()); Ok(out) } @@ -62,8 +62,7 @@ pub fn resolve_with_config( config: Option<&Config>, ) -> CargoResult> { let resolve = resolve_with_config_raw(pkg, deps, registry, config)?; - let out: Vec = resolve.iter().cloned().collect(); - Ok(out) + Ok(resolve.sort()) } pub fn resolve_with_config_raw( @@ -230,11 +229,16 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary { } pub fn dep(name: &str) -> Dependency { - dep_req(name, "1.0.0") + dep_req(name, "*") } pub fn dep_req(name: &str, req: &str) -> Dependency { Dependency::parse_no_deprecated(name, Some(req), ®istry_loc()).unwrap() } +pub fn dep_req_kind(name: &str, req: &str, kind: Kind) -> Dependency { + let mut dep = dep_req(name, req); + dep.set_kind(kind); + dep +} pub fn dep_loc(name: &str, location: &str) -> Dependency { let url = location.to_url().unwrap(); @@ -277,12 +281,30 @@ impl fmt::Debug for PrettyPrintRegistry { } else { write!(f, "pkg!((\"{}\", \"{}\") => [", s.name(), s.version())?; for d in s.dependencies() { - write!( - f, - "dep_req(\"{}\", \"{}\"),", - d.name_in_toml(), - d.version_req() - )?; + if d.kind() == Kind::Normal + && &d.version_req().to_string() == "*" + { + write!(f, "dep(\"{}\"),", d.name_in_toml())?; + } else if d.kind() == Kind::Normal { + write!( + f, + "dep_req(\"{}\", \"{}\"),", + d.name_in_toml(), + d.version_req() + )?; + } else { + write!( + f, + "dep_req_kind(\"{}\", \"{}\", {}),", + d.name_in_toml(), + d.version_req(), + match d.kind() { + Kind::Development => "Kind::Development", + Kind::Build => "Kind::Build", + Kind::Normal => "Kind::Normal", + } + )?; + } } write!(f, "]),")?; } @@ -299,21 +321,28 @@ fn meta_test_deep_pretty_print_registry() { PrettyPrintRegistry(vec![ pkg!(("foo", "1.0.1") => [dep_req("bar", "1")]), pkg!(("foo", "1.0.0") => [dep_req("bar", "2")]), + pkg!(("foo", "2.0.0") => [dep_req("bar", "*")]), pkg!(("bar", "1.0.0") => [dep_req("baz", "=1.0.2"), dep_req("other", "1")]), pkg!(("bar", "2.0.0") => [dep_req("baz", "=1.0.1")]), pkg!(("baz", "1.0.2") => [dep_req("other", "2")]), pkg!(("baz", "1.0.1")), + pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", Kind::Build)]), + pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", Kind::Development)]), pkg!(("dep_req", "1.0.0")), pkg!(("dep_req", "2.0.0")), ]) ), "vec![pkg!((\"foo\", \"1.0.1\") => [dep_req(\"bar\", \"^1\"),]),\ pkg!((\"foo\", \"1.0.0\") => [dep_req(\"bar\", \"^2\"),]),\ + pkg!((\"foo\", \"2.0.0\") => [dep(\"bar\"),]),\ pkg!((\"bar\", \"1.0.0\") => [dep_req(\"baz\", \"= 1.0.2\"),dep_req(\"other\", \"^1\"),]),\ pkg!((\"bar\", \"2.0.0\") => [dep_req(\"baz\", \"= 1.0.1\"),]),\ pkg!((\"baz\", \"1.0.2\") => [dep_req(\"other\", \"^2\"),]),\ - pkg!((\"baz\", \"1.0.1\")),pkg!((\"dep_req\", \"1.0.0\")),\ + pkg!((\"baz\", \"1.0.1\")),\ + pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", Kind::Build),]),\ + pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", Kind::Development),]),\ + pkg!((\"dep_req\", \"1.0.0\")),\ pkg!((\"dep_req\", \"2.0.0\")),]" ) } @@ -357,7 +386,7 @@ pub fn registry_strategy( let max_deps = max_versions * (max_crates * (max_crates - 1)) / shrinkage; let raw_version_range = (any::(), any::()); - let raw_dependency = (any::(), any::(), raw_version_range); + let raw_dependency = (any::(), any::(), raw_version_range, 0..=1); fn order_index(a: Index, b: Index, size: usize) -> (usize, usize) { let (a, b) = (a.index(size), b.index(size)); @@ -374,22 +403,35 @@ pub fn registry_strategy( .collect(); let len_all_pkgid = list_of_pkgid.len(); let mut dependency_by_pkgid = vec![vec![]; len_all_pkgid]; - for (a, b, (c, d)) in raw_dependencies { + for (a, b, (c, d), k) in raw_dependencies { let (a, b) = order_index(a, b, len_all_pkgid); let ((dep_name, _), _) = list_of_pkgid[a]; if (list_of_pkgid[b].0).0 == dep_name { continue; } let s = &crate_vers_by_name[dep_name]; + let s_last_index = s.len() - 1; let (c, d) = order_index(c, d, s.len()); - dependency_by_pkgid[b].push(dep_req( + dependency_by_pkgid[b].push(dep_req_kind( &dep_name, - &if c == d { + &if c == 0 && d == s_last_index { + "*".to_string() + } else if c == 0 { + format!("<={}", s[d].0) + } else if d == s_last_index { + format!(">={}", s[c].0) + } else if c == d { format!("={}", s[c].0) } else { format!(">={}, <={}", s[c].0, s[d].0) }, + match k { + 0 => Kind::Normal, + 1 => Kind::Build, + // => Kind::Development, // Development has not impact so don't gen + _ => panic!("bad index for Kind"), + }, )) }