Auto merge of #9847 - weihanglo:issue-9840, r=Eh2406

Distinguish lockfile version req from normal dep in resolver error message

Resolves #9840

This PR adds a new variant `OptVersionReq::Locked` as #9840 described.
The new variant represents as a locked version requirement that contains
an exact locked version and the original version requirement.

Previously we use exact version req to synthesize locked version req.
Now we make those need a truly locked to be `OptVersionReq::Locked`,
including:

- `[patch]`: previous used exact version req to emulate locked version,
  and it only need to lock dep version but not source ID, so we make
  an extra method `Dependency::lock_version` for it.
- Dependencies from lock files: No need to change the call site.

Author: bors

src/cargo/core/dependency.rs

@@ -320,7 +320,6 @@ impl Dependency {
     /// Locks this dependency to depending on the specified package ID.
     pub fn lock_to(&mut self, id: PackageId) -> &mut Dependency {
         assert_eq!(self.inner.source_id, id.source_id());
-        assert!(self.inner.req.matches(id.version()));
         trace!(
             "locking dep from `{}` with `{}` at {} to {}",
             self.package_name(),
@@ -329,7 +328,7 @@ impl Dependency {
             id
         );
         let me = Rc::make_mut(&mut self.inner);
-        me.req = OptVersionReq::exact(id.version());
+        me.req.lock_to(id.version());
 
         // Only update the `precise` of this source to preserve other
         // information about dependency's source which may not otherwise be
@@ -340,10 +339,20 @@ impl Dependency {
         self
     }
 
-    /// Returns `true` if this is a "locked" dependency, basically whether it has
-    /// an exact version req.
+    /// Locks this dependency to a specified version.
+    ///
+    /// Mainly used in dependency patching like `[patch]` or `[replace]`, which
+    /// doesn't need to lock the entire dependency to a specific [`PackageId`].
+    pub fn lock_version(&mut self, version: &semver::Version) -> &mut Dependency {
+        let me = Rc::make_mut(&mut self.inner);
+        me.req.lock_to(version);
+        self
+    }
+
+    /// Returns `true` if this is a "locked" dependency. Basically a locked
+    /// dependency has an exact version req, but not vice versa.
     pub fn is_locked(&self) -> bool {
-        self.inner.req.is_exact()
+        self.inner.req.is_locked()
     }
 
     /// Returns `false` if the dependency is only used to build the local package.

src/cargo/core/registry.rs

@@ -5,10 +5,9 @@ use crate::core::{Dependency, PackageId, Source, SourceId, SourceMap, Summary};
 use crate::sources::config::SourceConfigMap;
 use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
-use crate::util::{profile, CanonicalUrl, Config, VersionReqExt};
+use crate::util::{profile, CanonicalUrl, Config};
 use anyhow::{bail, Context as _};
 use log::{debug, trace};
-use semver::VersionReq;
 use url::Url;
 
 /// Source of information about a group of packages.
@@ -765,8 +764,7 @@ fn lock(
                 if locked.source_id() == dep.source_id() {
                     dep.lock_to(locked);
                 } else {
-                    let req = VersionReq::exact(locked.version());
-                    dep.set_version_req(req);
+                    dep.lock_version(locked.version());
                 }
                 return dep;
             }

src/cargo/core/resolver/errors.rs

@@ -387,10 +387,16 @@ pub(crate) fn describe_path<'a>(
             } else {
                 dep.name_in_toml().to_string()
             };
+            let locked_version = dep
+                .version_req()
+                .locked_version()
+                .map(|v| format!("(locked to {}) ", v))
+                .unwrap_or_default();
+
             write!(
                 dep_path_desc,
-                "\n    ... which satisfies {}dependency `{}` of package `{}`",
-                source_kind, requirement, pkg
+                "\n    ... which satisfies {}dependency `{}` {}of package `{}`",
+                source_kind, requirement, locked_version, pkg
             )
             .unwrap();
         }

src/cargo/ops/cargo_install.rs

@@ -681,7 +681,7 @@ fn installed_exact_package<T>(
 where
     T: Source,
 {
-    if !dep.is_locked() {
+    if !dep.version_req().is_exact() {
         // If the version isn't exact, we may need to update the registry and look for a newer
         // version - we can't know if the package is installed without doing so.
         return Ok(None);

src/cargo/util/semver_ext.rs

@@ -5,6 +5,8 @@ use std::fmt::{self, Display};
 pub enum OptVersionReq {
     Any,
     Req(VersionReq),
+    /// The exact locked version and the original version requirement.
+    Locked(Version, VersionReq),
 }
 
 pub trait VersionExt {
@@ -49,22 +51,53 @@ impl OptVersionReq {
                     cmp.op == Op::Exact && cmp.minor.is_some() && cmp.patch.is_some()
                 }
             }
+            OptVersionReq::Locked(..) => true,
+        }
+    }
+
+    pub fn lock_to(&mut self, version: &Version) {
+        assert!(self.matches(version), "cannot lock {} to {}", self, version);
+        use OptVersionReq::*;
+        let version = version.clone();
+        *self = match self {
+            Any => Locked(version, VersionReq::STAR),
+            Req(req) => Locked(version, req.clone()),
+            Locked(_, req) => Locked(version, req.clone()),
+        };
+    }
+
+    pub fn is_locked(&self) -> bool {
+        matches!(self, OptVersionReq::Locked(..))
+    }
+
+    /// Gets the version to which this req is locked, if any.
+    pub fn locked_version(&self) -> Option<&Version> {
+        match self {
+            OptVersionReq::Locked(version, _) => Some(version),
+            _ => None,
         }
     }
 
     pub fn matches(&self, version: &Version) -> bool {
         match self {
             OptVersionReq::Any => true,
             OptVersionReq::Req(req) => req.matches(version),
+            OptVersionReq::Locked(v, _) => {
+                v.major == version.major
+                    && v.minor == version.minor
+                    && v.patch == version.patch
+                    && v.pre == version.pre
+            }
         }
     }
 }
 
 impl Display for OptVersionReq {
-    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            OptVersionReq::Any => formatter.write_str("*"),
-            OptVersionReq::Req(req) => Display::fmt(req, formatter),
+            OptVersionReq::Any => f.write_str("*"),
+            OptVersionReq::Req(req) => Display::fmt(req, f),
+            OptVersionReq::Locked(_, req) => Display::fmt(req, f),
         }
     }
 }
@@ -74,3 +107,40 @@ impl From<VersionReq> for OptVersionReq {
         OptVersionReq::Req(req)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn locked_has_the_same_with_exact() {
+        fn test_versions(target_ver: &str, vers: &[&str]) {
+            let ver = Version::parse(target_ver).unwrap();
+            let exact = OptVersionReq::exact(&ver);
+            let mut locked = exact.clone();
+            locked.lock_to(&ver);
+            for v in vers {
+                let v = Version::parse(v).unwrap();
+                assert_eq!(exact.matches(&v), locked.matches(&v));
+            }
+        }
+
+        test_versions(
+            "1.0.0",
+            &["1.0.0", "1.0.1", "0.9.9", "0.10.0", "0.1.0", "1.0.0-pre"],
+        );
+        test_versions("0.9.0", &["0.9.0", "0.9.1", "1.9.0", "0.0.9", "0.9.0-pre"]);
+        test_versions("0.0.2", &["0.0.2", "0.0.1", "0.0.3", "0.0.2-pre"]);
+        test_versions(
+            "0.1.0-beta2.a",
+            &[
+                "0.1.0-beta2.a",
+                "0.9.1",
+                "0.1.0",
+                "0.1.1-beta2.a",
+                "0.1.0-beta2",
+            ],
+        );
+        test_versions("0.1.0+meta", &["0.1.0", "0.1.0+meta", "0.1.0+any"]);
+    }
+}

src/cargo/util/toml/mod.rs

@@ -1553,16 +1553,15 @@ impl TomlManifest {
             }
 
             let mut dep = replacement.to_dependency(spec.name().as_str(), cx, None)?;
-            {
-                let version = spec.version().ok_or_else(|| {
-                    anyhow!(
-                        "replacements must specify a version \
-                         to replace, but `{}` does not",
-                        spec
-                    )
-                })?;
-                dep.set_version_req(VersionReq::exact(version));
-            }
+            let version = spec.version().ok_or_else(|| {
+                anyhow!(
+                    "replacements must specify a version \
+                     to replace, but `{}` does not",
+                    spec
+                )
+            })?;
+            dep.set_version_req(VersionReq::exact(version))
+                .lock_version(version);
             replace.push((spec, dep));
         }
         Ok(replace)

tests/testsuite/build_script.rs

@@ -1000,7 +1000,7 @@ fn links_duplicates_old_registry() {
     but a native library can be linked only once
 
 package `bar v0.1.0`
-    ... which satisfies dependency `bar = \"=0.1.0\"` of package `foo v0.1.0 ([..]foo)`
+    ... which satisfies dependency `bar = \"^0.1\"` (locked to 0.1.0) of package `foo v0.1.0 ([..]foo)`
 links to native library `a`
 
 package `foo v0.1.0 ([..]foo)`