fix(update): don't reuse a previously locked dependency if multiple major versions are compatible

crates/resolver-tests/src/lib.rs

@@ -102,6 +102,7 @@ pub fn resolve_with_global_context_raw(
     struct MyRegistry<'a> {
         list: &'a [Summary],
         used: HashSet<PackageId>,
+        version_prefs: VersionPreferences,
     }
     impl<'a> Registry for MyRegistry<'a> {
         fn query(
@@ -135,6 +136,14 @@ pub fn resolve_with_global_context_raw(
         fn block_until_ready(&mut self) -> CargoResult<()> {
             Ok(())
         }
+
+        fn version_prefs(&self) -> &VersionPreferences {
+            &self.version_prefs
+        }
+
+        fn set_version_prefs(&mut self, version_prefs: VersionPreferences) {
+            self.version_prefs = version_prefs;
+        }
     }
     impl<'a> Drop for MyRegistry<'a> {
         fn drop(&mut self) {
@@ -157,6 +166,7 @@ pub fn resolve_with_global_context_raw(
     let mut registry = MyRegistry {
         list: registry,
         used: HashSet::new(),
+        version_prefs: VersionPreferences::default(),
     };
     let summary = Summary::new(
         pkg_id("root"),
@@ -172,11 +182,11 @@ pub fn resolve_with_global_context_raw(
     if gctx.cli_unstable().minimal_versions {
         version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst)
     }
+    registry.set_version_prefs(version_prefs);
     let resolve = resolver::resolve(
         &[(summary, opts)],
         &[],
         &mut registry,
-        &version_prefs,
         ResolveVersion::with_rust_version(None),
         Some(gctx),
     );

src/cargo/core/registry.rs

@@ -12,6 +12,7 @@
 use std::collections::{HashMap, HashSet};
 use std::task::{ready, Poll};
 
+use crate::core::resolver::VersionPreferences;
 use crate::core::PackageSet;
 use crate::core::{Dependency, PackageId, SourceId, Summary};
 use crate::sources::config::SourceConfigMap;
@@ -63,6 +64,12 @@ pub trait Registry {
 
     /// Block until all outstanding [`Poll::Pending`] requests are [`Poll::Ready`].
     fn block_until_ready(&mut self) -> CargoResult<()>;
+
+    /// Get version preferences.
+    fn version_prefs(&self) -> &VersionPreferences;
+
+    /// Set version preferences.
+    fn set_version_prefs(&mut self, version_prefs: VersionPreferences);
 }
 
 /// This structure represents a registry of known packages. It internally
@@ -131,6 +138,9 @@ pub struct PackageRegistry<'gctx> {
     /// This is constructed during calls to [`PackageRegistry::patch`],
     /// along with the `patches` field, thoough these entries never get locked.
     patches_available: HashMap<CanonicalUrl, Vec<PackageId>>,
+
+    /// Version preferences.
+    version_prefs: VersionPreferences,
 }
 
 /// A map of all "locked packages" which is filled in when parsing a lock file
@@ -209,6 +219,7 @@ impl<'gctx> PackageRegistry<'gctx> {
             patches: HashMap::new(),
             patches_locked: false,
             patches_available: HashMap::new(),
+            version_prefs: VersionPreferences::default(),
         })
     }
 
@@ -503,7 +514,12 @@ impl<'gctx> PackageRegistry<'gctx> {
         for summaries in self.patches.values_mut() {
             for summary in summaries {
                 debug!("locking patch {:?}", summary);
-                *summary = lock(&self.locked, &self.patches_available, summary.clone());
+                *summary = lock(
+                    &self.locked,
+                    &self.patches_available,
+                    summary.clone(),
+                    &self.version_prefs,
+                );
             }
         }
         self.patches_locked = true;
@@ -581,7 +597,12 @@ impl<'gctx> PackageRegistry<'gctx> {
     /// through.
     pub fn lock(&self, summary: Summary) -> Summary {
         assert!(self.patches_locked);
-        lock(&self.locked, &self.patches_available, summary)
+        lock(
+            &self.locked,
+            &self.patches_available,
+            summary,
+            &self.version_prefs,
+        )
     }
 
     fn warn_bad_override(
@@ -734,7 +755,12 @@ impl<'gctx> Registry for PackageRegistry<'gctx> {
                         }
                     }
                     let summary = summary.into_summary();
-                    f(IndexSummary::Candidate(lock(locked, all_patches, summary)))
+                    f(IndexSummary::Candidate(lock(
+                        locked,
+                        all_patches,
+                        summary,
+                        &self.version_prefs,
+                    )))
                 };
                 return source.query(dep, kind, callback);
             }
@@ -799,13 +825,22 @@ impl<'gctx> Registry for PackageRegistry<'gctx> {
         }
         Ok(())
     }
+
+    fn version_prefs(&self) -> &VersionPreferences {
+        &self.version_prefs
+    }
+
+    fn set_version_prefs(&mut self, version_prefs: VersionPreferences) {
+        self.version_prefs = version_prefs;
+    }
 }
 
 /// See [`PackageRegistry::lock`].
 fn lock(
     locked: &LockedMap,
     patches: &HashMap<CanonicalUrl, Vec<PackageId>>,
     summary: Summary,
+    version_prefs: &VersionPreferences,
 ) -> Summary {
     let pair = locked
         .get(&(summary.source_id(), summary.name()))
@@ -892,12 +927,30 @@ fn lock(
             }
         }
 
-        // If this dependency did not have a locked version, then we query
-        // all known locked packages to see if they match this dependency.
-        // If anything does then we lock it to that and move on.
-        let v = locked
-            .get(&(dep.source_id(), dep.package_name()))
-            .and_then(|vec| vec.iter().find(|&&(id, _)| dep.matches_id(id)));
+        // If this dependency did not have a locked version, we check if multiple
+        // versions of the dependency already exists in the lockfile. In this case we want
+        // to do the full resolving instead of locking a specific version.
+        // Otherwise, we query all known locked packages to see if they match this dependency,
+        // and if anything does then we lock it to that and move on.
+
+        let compatible_versions_count = version_prefs
+            .get_try_to_use()
+            .iter()
+            .filter(|&&id| {
+                id.source_id() == dep.source_id()
+                    && id.name() == dep.package_name()
+                    && dep.matches_id(id)
+            })
+            .count();
+
+        let v = if compatible_versions_count > 1 {
+            None
+        } else {
+            locked
+                .get(&(dep.source_id(), dep.package_name()))
+                .and_then(|vec| vec.iter().find(|&&(id, _)| dep.matches_id(id)))
+        };
+
         if let Some(&(id, _)) = v {
             trace!("\tsecond hit on {}", id);
             let mut dep = dep;

src/cargo/core/resolver/dep_cache.rs

@@ -14,7 +14,6 @@ use crate::core::resolver::errors::describe_path_in_context;
 use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet};
 use crate::core::resolver::{
     ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, VersionOrdering,
-    VersionPreferences,
 };
 use crate::core::{
     Dependency, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, Registry, Summary,
@@ -32,7 +31,6 @@ use tracing::debug;
 pub struct RegistryQueryer<'a> {
     pub registry: &'a mut (dyn Registry + 'a),
     replacements: &'a [(PackageIdSpec, Dependency)],
-    version_prefs: &'a VersionPreferences,
     /// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_version`)
     registry_cache: HashMap<(Dependency, Option<VersionOrdering>), Poll<Rc<Vec<Summary>>>>,
     /// a cache of `Dependency`s that are required for a `Summary`
@@ -52,12 +50,10 @@ impl<'a> RegistryQueryer<'a> {
     pub fn new(
         registry: &'a mut dyn Registry,
         replacements: &'a [(PackageIdSpec, Dependency)],
-        version_prefs: &'a VersionPreferences,
     ) -> Self {
         RegistryQueryer {
             registry,
             replacements,
-            version_prefs,
             registry_cache: HashMap::new(),
             summary_cache: HashMap::new(),
             used_replacements: HashMap::new(),
@@ -208,7 +204,9 @@ impl<'a> RegistryQueryer<'a> {
         }
 
         let first_version = first_version;
-        self.version_prefs.sort_summaries(&mut ret, first_version);
+        self.registry
+            .version_prefs()
+            .sort_summaries(&mut ret, first_version);
 
         let out = Poll::Ready(Rc::new(ret));
 

src/cargo/core/resolver/mod.rs

@@ -126,7 +126,6 @@ pub fn resolve(
     summaries: &[(Summary, ResolveOpts)],
     replacements: &[(PackageIdSpec, Dependency)],
     registry: &mut dyn Registry,
-    version_prefs: &VersionPreferences,
     resolve_version: ResolveVersion,
     gctx: Option<&GlobalContext>,
 ) -> CargoResult<Resolve> {
@@ -136,7 +135,7 @@ pub fn resolve(
         }
         _ => None,
     };
-    let mut registry = RegistryQueryer::new(registry, replacements, version_prefs);
+    let mut registry = RegistryQueryer::new(registry, replacements);
     let resolver_ctx = loop {
         let resolver_ctx = ResolverContext::new();
         let resolver_ctx =

src/cargo/core/resolver/version_prefs.rs

@@ -32,6 +32,10 @@ pub enum VersionOrdering {
 }
 
 impl VersionPreferences {
+    pub fn get_try_to_use(&self) -> &HashSet<PackageId> {
+        &self.try_to_use
+    }
+
     /// Indicate that the given package (specified as a [`PackageId`]) should be preferred.
     pub fn prefer_package_id(&mut self, pkg_id: PackageId) {
         self.try_to_use.insert(pkg_id);

src/cargo/ops/lockfile.rs

@@ -35,7 +35,7 @@ pub fn resolve_to_string(ws: &Workspace<'_>, resolve: &Resolve) -> CargoResult<S
     Ok(out)
 }
 
-/// Ensure the resolve result is written to fisk
+/// Ensure the resolve result is written to disk
 ///
 /// Returns `true` if the lockfile changed
 #[tracing::instrument(skip_all)]

src/cargo/ops/resolve.rs

@@ -64,14 +64,14 @@ use crate::core::resolver::{
     self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion, VersionOrdering, VersionPreferences,
 };
 use crate::core::summary::Summary;
-use crate::core::Dependency;
 use crate::core::GitReference;
 use crate::core::PackageId;
 use crate::core::PackageIdSpec;
 use crate::core::PackageIdSpecQuery;
 use crate::core::PackageSet;
 use crate::core::SourceId;
 use crate::core::Workspace;
+use crate::core::{Dependency, Registry};
 use crate::ops;
 use crate::sources::RecursivePathSource;
 use crate::util::cache_lock::CacheLockMode;
@@ -398,6 +398,8 @@ pub fn resolve_with_previous<'gctx>(
         registry.lock_patches();
     }
 
+    registry.set_version_prefs(version_prefs);
+
     let summaries: Vec<(Summary, ResolveOpts)> = {
         let _span = tracing::span!(tracing::Level::TRACE, "registry.lock").entered();
         ws.members_with_features(specs, cli_features)?
@@ -421,7 +423,6 @@ pub fn resolve_with_previous<'gctx>(
         &summaries,
         &replace,
         registry,
-        &version_prefs,
         ResolveVersion::with_rust_version(ws.rust_version()),
         Some(ws.gctx()),
     )?;

tests/testsuite/update.rs

@@ -421,7 +421,7 @@ dependencies = [
 name = "foo"
 version = "0.0.1"
 dependencies = [
- "log 0.1.0",
+ "log 0.4.0",
 ]"#
     ));
 
@@ -434,7 +434,7 @@ dependencies = [
 name = "foo"
 version = "0.0.1"
 dependencies = [
- "log 0.2.0",
+ "log 0.1.0",
 ]"#
     ));
 
@@ -458,7 +458,7 @@ dependencies = [
 name = "foo"
 version = "0.0.1"
 dependencies = [
- "log 0.2.0",
+ "log 0.4.0",
 ]"#
     ));
 }