feat(resolver): -Zdirect-minimal-versions

This is an alternative to `-Zminimal-versions` as discussed in #5657.

The problem with `-Zminimal-versions` is it requires the root most
dependencies to verify it and we then percolate that up the stack.  This
requires a massive level of cooperation to accomplish and so far there
have been mixed results with it to the point that cargo's unstable
documentation discourages its use.

`-Zdirect-minimal-versions` instead only applies this rule to your
direct dependencies, allowing anyone in the stack to immediately adopt
it, independent of everyone else.

Special notes
- Living up to the name and the existing design, this ignores yanked
  crates.  This makes sense for `^1.1` version requirements but might
  look weird for `^1.1.1` version requirements as it could select
  `1.1.2`.
- This will error if an indirect dependency requires a newer version.
  Your version requirement will need to capture what you use **and** all
  of you dependencies.  An alternative design would have tried to merge
  the result of minimum versions for direct dependencies and maximum
  versions for indirect dependencies.  This would have been complex and
  led to weird corner cases, making it harder to predict.  I also suspect
  the value gained would be relatively low as you can't verify that
  version requirement in any other way.
  - The error could be improved to call out that this was from minimal
    versions but I felt getting this out now and starting to collect
    feedback was more important.

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()))],
+                &reg,
+            );
+
+            let mres = resolve_with_config(
+                vec![dep_req(&this.name(), &format!("={}", this.version()))],
+                &reg,
+                &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(

src/cargo/core/features.rs

@@ -718,6 +718,7 @@ unstable_cli_options!(
     features: Option<Vec<String>>  = (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" => {

src/cargo/core/resolver/dep_cache.rs

@@ -39,6 +39,10 @@ pub struct RegistryQueryer<'a> {
     /// a cache of `Candidate`s that fulfil a `Dependency`
     registry_cache: HashMap<Dependency, Poll<Rc<Vec<Summary>>>>,
     /// 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<PackageId>, Summary, ResolveOpts),
         (Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>, bool),
@@ -96,7 +100,11 @@ 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<CargoResult<Rc<Vec<Summary>>>> {
+    pub fn query(
+        &mut self,
+        dep: &Dependency,
+        first_minimal_version: bool,
+    ) -> Poll<CargoResult<Rc<Vec<Summary>>>> {
         if let Some(out) = self.registry_cache.get(dep).cloned() {
             return out.map(Result::Ok);
         }
@@ -192,14 +200,14 @@ 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));
 
@@ -218,6 +226,7 @@ impl<'a> RegistryQueryer<'a> {
         parent: Option<PackageId>,
         candidate: &Summary,
         opts: &ResolveOpts,
+        first_minimal_version: bool,
     ) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> {
         // if we have calculated a result before, then we can just return it,
         // as it is a "pure" query of its arguments.
@@ -237,22 +246,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::<CargoResult<Vec<DepInfo>>>()?;
 
         // Attempt to resolve dependencies with fewer candidates before trying

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<Context> {
     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<Option<(DepsFrame, Duration)>> {
     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()

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<Summary>, version_ordering: VersionOrdering) {
+    pub fn sort_summaries(
+        &self,
+        summaries: &mut Vec<Summary>,
+        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()

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)