diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index 9d8f25a2bd8..df74826f078 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -100,6 +100,64 @@ proptest! { } } + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. + #[test] + fn prop_direct_minimum_version_error_implications( + PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) + ) { + let mut config = Config::default().unwrap(); + config.nightly_features_allowed = true; + config + .configure( + 1, + false, + None, + false, + false, + false, + &None, + &["direct-minimal-versions".to_string()], + &[], + ) + .unwrap(); + + let reg = registry(input.clone()); + // there is only a small chance that any one + // crate will be interesting. + // So we try some of the most complicated. + for this in input.iter().rev().take(10) { + // direct-minimal-versions reduces the number of available solutions, so we verify that + // we do not come up with solutions maximal versions does not + let res = resolve( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + ); + + let mres = resolve_with_config( + vec![dep_req(&this.name(), &format!("={}", this.version()))], + ®, + &config, + ); + + if res.is_err() { + prop_assert!( + mres.is_err(), + "direct-minimal-versions should not have more solutions than the regular, maximal resolver but found one when resolving `{} = \"={}\"`", + this.name(), + this.version() + ) + } + if mres.is_ok() { + prop_assert!( + res.is_ok(), + "direct-minimal-versions should not have more solutions than the regular, maximal resolver but found one when resolving `{} = \"={}\"`", + this.name(), + this.version() + ) + } + } + } + /// NOTE: if you think this test has failed spuriously see the note at the top of this macro. #[test] fn prop_removing_a_dep_cant_break( diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index f2c58d9f43f..5bf43debcab 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -718,6 +718,7 @@ unstable_cli_options!( features: Option> = (HIDDEN), jobserver_per_rustc: bool = (HIDDEN), minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"), + direct_minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum (direct dependencies only)"), mtime_on_use: bool = ("Configure Cargo to update the mtime of used files"), no_index_update: bool = ("Do not update the registry index even if the cache is outdated"), panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"), @@ -948,6 +949,7 @@ impl CliUnstable { "no-index-update" => self.no_index_update = parse_empty(k, v)?, "avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?, "minimal-versions" => self.minimal_versions = parse_empty(k, v)?, + "direct-minimal-versions" => self.direct_minimal_versions = parse_empty(k, v)?, "advanced-env" => self.advanced_env = parse_empty(k, v)?, "config-include" => self.config_include = parse_empty(k, v)?, "check-cfg" => { diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index dd86170035e..4fd27538555 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -36,9 +36,13 @@ pub struct RegistryQueryer<'a> { /// versions first. That allows `cargo update -Z minimal-versions` which will /// specify minimum dependency versions to be used. minimal_versions: bool, - /// a cache of `Candidate`s that fulfil a `Dependency` - registry_cache: HashMap>>>, + /// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_minimal_version`) + registry_cache: HashMap<(Dependency, bool), Poll>>>, /// a cache of `Dependency`s that are required for a `Summary` + /// + /// HACK: `first_minimal_version` is not kept in the cache key is it is 1:1 with + /// `parent.is_none()` (the first element of the cache key) as it doesn't change through + /// execution. summary_cache: HashMap< (Option, Summary, ResolveOpts), (Rc<(HashSet, Rc>)>, bool), @@ -96,8 +100,13 @@ impl<'a> RegistryQueryer<'a> { /// any candidates are returned which match an override then the override is /// applied by performing a second query for what the override should /// return. - pub fn query(&mut self, dep: &Dependency) -> Poll>>> { - if let Some(out) = self.registry_cache.get(dep).cloned() { + pub fn query( + &mut self, + dep: &Dependency, + first_minimal_version: bool, + ) -> Poll>>> { + let registry_cache_key = (dep.clone(), first_minimal_version); + if let Some(out) = self.registry_cache.get(®istry_cache_key).cloned() { return out.map(Result::Ok); } @@ -106,7 +115,8 @@ impl<'a> RegistryQueryer<'a> { ret.push(s); })?; if ready.is_pending() { - self.registry_cache.insert(dep.clone(), Poll::Pending); + self.registry_cache + .insert((dep.clone(), first_minimal_version), Poll::Pending); return Poll::Pending; } for summary in ret.iter() { @@ -128,7 +138,8 @@ impl<'a> RegistryQueryer<'a> { let mut summaries = match self.registry.query_vec(dep, QueryKind::Exact)? { Poll::Ready(s) => s.into_iter(), Poll::Pending => { - self.registry_cache.insert(dep.clone(), Poll::Pending); + self.registry_cache + .insert((dep.clone(), first_minimal_version), Poll::Pending); return Poll::Pending; } }; @@ -192,18 +203,18 @@ 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. VersionPreferences implements this notion. - self.version_prefs.sort_summaries( - &mut ret, - if self.minimal_versions { - VersionOrdering::MinimumVersionsFirst - } else { - VersionOrdering::MaximumVersionsFirst - }, - ); + let ordering = if first_minimal_version || self.minimal_versions { + VersionOrdering::MinimumVersionsFirst + } else { + VersionOrdering::MaximumVersionsFirst + }; + let first_version = first_minimal_version; + self.version_prefs + .sort_summaries(&mut ret, ordering, first_version); let out = Poll::Ready(Rc::new(ret)); - self.registry_cache.insert(dep.clone(), out.clone()); + self.registry_cache.insert(registry_cache_key, out.clone()); out.map(Result::Ok) } @@ -218,6 +229,7 @@ impl<'a> RegistryQueryer<'a> { parent: Option, candidate: &Summary, opts: &ResolveOpts, + first_minimal_version: bool, ) -> ActivateResult, Rc>)>> { // if we have calculated a result before, then we can just return it, // as it is a "pure" query of its arguments. @@ -237,22 +249,24 @@ impl<'a> RegistryQueryer<'a> { let mut all_ready = true; let mut deps = deps .into_iter() - .filter_map(|(dep, features)| match self.query(&dep) { - Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))), - Poll::Pending => { - all_ready = false; - // we can ignore Pending deps, resolve will be repeatedly called - // until there are none to ignore - None - } - Poll::Ready(Err(e)) => Some(Err(e).with_context(|| { - format!( - "failed to get `{}` as a dependency of {}", - dep.package_name(), - describe_path_in_context(cx, &candidate.package_id()), - ) - })), - }) + .filter_map( + |(dep, features)| match self.query(&dep, first_minimal_version) { + Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))), + Poll::Pending => { + all_ready = false; + // we can ignore Pending deps, resolve will be repeatedly called + // until there are none to ignore + None + } + Poll::Ready(Err(e)) => Some(Err(e).with_context(|| { + format!( + "failed to get `{}` as a dependency of {}", + dep.package_name(), + describe_path_in_context(cx, &candidate.package_id()), + ) + })), + }, + ) .collect::>>()?; // Attempt to resolve dependencies with fewer candidates before trying diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index b0551891da1..48607c3d370 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -133,11 +133,21 @@ pub fn resolve( Some(config) => config.cli_unstable().minimal_versions, None => false, }; + let direct_minimal_versions = match config { + Some(config) => config.cli_unstable().direct_minimal_versions, + None => false, + }; let mut registry = RegistryQueryer::new(registry, replacements, version_prefs, minimal_versions); let cx = loop { let cx = Context::new(check_public_visible_dependencies); - let cx = activate_deps_loop(cx, &mut registry, summaries, config)?; + let cx = activate_deps_loop( + cx, + &mut registry, + summaries, + direct_minimal_versions, + config, + )?; if registry.reset_pending() { break cx; } else { @@ -189,6 +199,7 @@ fn activate_deps_loop( mut cx: Context, registry: &mut RegistryQueryer<'_>, summaries: &[(Summary, ResolveOpts)], + direct_minimal_versions: bool, config: Option<&Config>, ) -> CargoResult { let mut backtrack_stack = Vec::new(); @@ -201,7 +212,14 @@ fn activate_deps_loop( // Activate all the initial summaries to kick off some work. for &(ref summary, ref opts) in summaries { debug!("initial activation: {}", summary.package_id()); - let res = activate(&mut cx, registry, None, summary.clone(), opts); + let res = activate( + &mut cx, + registry, + None, + summary.clone(), + direct_minimal_versions, + opts, + ); match res { Ok(Some((frame, _))) => remaining_deps.push(frame), Ok(None) => (), @@ -399,7 +417,15 @@ fn activate_deps_loop( dep.package_name(), candidate.version() ); - let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, &opts); + let direct_minimal_version = false; // this is an indirect dependency + let res = activate( + &mut cx, + registry, + Some((&parent, &dep)), + candidate, + direct_minimal_version, + &opts, + ); let successfully_activated = match res { // Success! We've now activated our `candidate` in our context @@ -611,6 +637,7 @@ fn activate( registry: &mut RegistryQueryer<'_>, parent: Option<(&Summary, &Dependency)>, candidate: Summary, + first_minimal_version: bool, opts: &ResolveOpts, ) -> ActivateResult> { let candidate_pid = candidate.package_id(); @@ -662,8 +689,13 @@ fn activate( }; let now = Instant::now(); - let (used_features, deps) = - &*registry.build_deps(cx, parent.map(|p| p.0.package_id()), &candidate, opts)?; + let (used_features, deps) = &*registry.build_deps( + cx, + parent.map(|p| p.0.package_id()), + &candidate, + opts, + first_minimal_version, + )?; // Record what list of features is active for this package. if !used_features.is_empty() { @@ -856,11 +888,14 @@ fn generalize_conflicting( }) { for critical_parents_dep in critical_parents_deps.iter() { + // We only want `first_minimal_version=true` for direct dependencies of workspace + // members which isn't the case here as this has a `parent` + let first_minimal_version = false; // A dep is equivalent to one of the things it can resolve to. // Thus, if all the things it can resolve to have already ben determined // to be conflicting, then we can just say that we conflict with the parent. if let Some(others) = registry - .query(critical_parents_dep) + .query(critical_parents_dep, first_minimal_version) .expect("an already used dep now error!?") .expect("an already used dep now pending!?") .iter() diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 8eb800c4058..73cce5db87a 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -42,7 +42,12 @@ impl VersionPreferences { /// Sort the given vector of summaries in-place, with all summaries presumed to be for /// the same package. Preferred versions appear first in the result, sorted by /// `version_ordering`, followed by non-preferred versions sorted the same way. - pub fn sort_summaries(&self, summaries: &mut Vec, version_ordering: VersionOrdering) { + pub fn sort_summaries( + &self, + summaries: &mut Vec, + version_ordering: VersionOrdering, + first_version: bool, + ) { let should_prefer = |pkg_id: &PackageId| { self.try_to_use.contains(pkg_id) || self @@ -66,6 +71,9 @@ impl VersionPreferences { _ => previous_cmp, } }); + if first_version { + let _ = summaries.split_off(1); + } } } @@ -115,13 +123,13 @@ mod test { summ("foo", "1.0.9"), ]; - vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string() ); - vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string() @@ -140,13 +148,13 @@ mod test { summ("foo", "1.0.9"), ]; - vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string() ); - vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string() @@ -166,13 +174,13 @@ mod test { summ("foo", "1.0.9"), ]; - vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst); + vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false); assert_eq!( describe(&summaries), "foo/1.2.3, foo/1.1.0, foo/1.2.4, foo/1.0.9".to_string() ); - vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst); + vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false); assert_eq!( describe(&summaries), "foo/1.1.0, foo/1.2.3, foo/1.0.9, foo/1.2.4".to_string() diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 0b21118a88e..d31b99a1f08 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -67,6 +67,7 @@ Each new feature described below should explain how to use it. * [no-index-update](#no-index-update) --- Prevents cargo from updating the index cache. * [avoid-dev-deps](#avoid-dev-deps) --- Prevents the resolver from including dev-dependencies during resolution. * [minimal-versions](#minimal-versions) --- Forces the resolver to use the lowest compatible version instead of the highest. + * [direct-minimal-versions](#direct-minimal-versions) — Forces the resolver to use the lowest compatible version instead of the highest. * [public-dependency](#public-dependency) --- Allows dependencies to be classified as either public or private. * Output behavior * [out-dir](#out-dir) --- Adds a directory where artifacts are copied to. @@ -176,6 +177,23 @@ minimum versions that you are actually using. That is, if Cargo.toml says `foo = "1.0.0"` that you don't accidentally depend on features added only in `foo 1.5.0`. +### direct-minimal-versions +* Original Issue: [#4100](https://github.com/rust-lang/cargo/issues/4100) +* Tracking Issue: [#5657](https://github.com/rust-lang/cargo/issues/5657) + +When a `Cargo.lock` file is generated, the `-Z direct-minimal-versions` flag will +resolve the dependencies to the minimum SemVer version that will satisfy the +requirements (instead of the greatest version) for direct dependencies only. + +The intended use-case of this flag is to check, during continuous integration, +that the versions specified in Cargo.toml are a correct reflection of the +minimum versions that you are actually using. That is, if Cargo.toml says +`foo = "1.0.0"` that you don't accidentally depend on features added only in +`foo 1.5.0`. + +Indirect dependencies are resolved as normal so as not to be blocked on their +minimal version validation. + ### out-dir * Original Issue: [#4875](https://github.com/rust-lang/cargo/issues/4875) * Tracking Issue: [#6790](https://github.com/rust-lang/cargo/issues/6790) diff --git a/tests/testsuite/direct_minimal_versions.rs b/tests/testsuite/direct_minimal_versions.rs new file mode 100644 index 00000000000..0e62d6ce083 --- /dev/null +++ b/tests/testsuite/direct_minimal_versions.rs @@ -0,0 +1,236 @@ +//! Tests for minimal-version resolution. +//! +//! Note: Some tests are located in the resolver-tests package. + +use cargo_test_support::project; +use cargo_test_support::registry::Package; + +#[cargo_test] +fn simple() { + Package::new("dep", "1.0.0").publish(); + Package::new("dep", "1.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + dep = "1.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("generate-lockfile -Zdirect-minimal-versions") + .masquerade_as_nightly_cargo(&["direct-minimal-versions"]) + .run(); + + let lock = p.read_lockfile(); + + assert!( + lock.contains("1.0.0"), + "dep minimal version must be present" + ); + assert!( + !lock.contains("1.1.0"), + "dep maximimal version cannot be present" + ); +} + +#[cargo_test] +fn mixed_dependencies() { + Package::new("dep", "1.0.0").publish(); + Package::new("dep", "1.1.0").publish(); + Package::new("dep", "1.2.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + dep = "1.0" + + [dev-dependencies] + dep = "1.1" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("generate-lockfile -Zdirect-minimal-versions") + .masquerade_as_nightly_cargo(&["direct-minimal-versions"]) + .with_status(101) + .with_stderr( + r#"[UPDATING] [..] +[ERROR] failed to select a version for `dep`. + ... required by package `foo v0.0.1 ([CWD])` +versions that meet the requirements `^1.1` are: 1.1.0 + +all possible versions conflict with previously selected packages. + + previously selected package `dep v1.0.0` + ... which satisfies dependency `dep = "^1.0"` of package `foo v0.0.1 ([CWD])` + +failed to select a version for `dep` which could resolve this conflict +"#, + ) + .run(); +} + +#[cargo_test] +fn yanked() { + Package::new("dep", "1.0.0").yanked(true).publish(); + Package::new("dep", "1.1.0").publish(); + Package::new("dep", "1.2.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + dep = "1.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("generate-lockfile -Zdirect-minimal-versions") + .masquerade_as_nightly_cargo(&["direct-minimal-versions"]) + .run(); + + let lock = p.read_lockfile(); + + assert!( + lock.contains("1.1.0"), + "dep minimal version must be present" + ); + assert!( + !lock.contains("1.0.0"), + "yanked minimal version must be skipped" + ); + assert!( + !lock.contains("1.2.0"), + "dep maximimal version cannot be present" + ); +} + +#[cargo_test] +fn indirect() { + Package::new("indirect", "2.0.0").publish(); + Package::new("indirect", "2.1.0").publish(); + Package::new("indirect", "2.2.0").publish(); + Package::new("direct", "1.0.0") + .dep("indirect", "2.1") + .publish(); + Package::new("direct", "1.1.0") + .dep("indirect", "2.1") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + direct = "1.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("generate-lockfile -Zdirect-minimal-versions") + .masquerade_as_nightly_cargo(&["direct-minimal-versions"]) + .run(); + + let lock = p.read_lockfile(); + + assert!( + lock.contains("1.0.0"), + "direct minimal version must be present" + ); + assert!( + !lock.contains("1.1.0"), + "direct maximimal version cannot be present" + ); + assert!( + !lock.contains("2.0.0"), + "indirect minimal version cannot be present" + ); + assert!( + !lock.contains("2.1.0"), + "indirect minimal version cannot be present" + ); + assert!( + lock.contains("2.2.0"), + "indirect maximal version must be present" + ); +} + +#[cargo_test] +fn indirect_conflict() { + Package::new("indirect", "2.0.0").publish(); + Package::new("indirect", "2.1.0").publish(); + Package::new("indirect", "2.2.0").publish(); + Package::new("direct", "1.0.0") + .dep("indirect", "2.1") + .publish(); + Package::new("direct", "1.1.0") + .dep("indirect", "2.1") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + direct = "1.0" + indirect = "2.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("generate-lockfile -Zdirect-minimal-versions") + .masquerade_as_nightly_cargo(&["direct-minimal-versions"]) + .with_status(101) + .with_stderr( + r#"[UPDATING] [..] +[ERROR] failed to select a version for `indirect`. + ... required by package `direct v1.0.0` + ... which satisfies dependency `direct = "^1.0"` of package `foo v0.0.1 ([CWD])` +versions that meet the requirements `^2.1` are: 2.2.0, 2.1.0 + +all possible versions conflict with previously selected packages. + + previously selected package `indirect v2.0.0` + ... which satisfies dependency `indirect = "^2.0"` of package `foo v0.0.1 ([CWD])` + +failed to select a version for `indirect` which could resolve this conflict +"#, + ) + .run(); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 66914e4dfd8..a1e293acd81 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -42,6 +42,7 @@ mod cross_publish; mod custom_target; mod death; mod dep_info; +mod direct_minimal_versions; mod directory; mod doc; mod docscrape;