Auto merge of #12806 - epage:replace, r=Eh2406

fix(replace): Partial-version spec support

### What does this PR try to resolve?

#12614 changed package ID specs to allow fields in the version number to be optional.  This earliest branch with this change is `rust-1.74.0` (beta).  While `@Eh2406` was investigating version metadata issues in #12772, problems with the partial version change were found
- `replace`s that specify version metadata were ignored **(fixed with this PR)**
  - This also extends out to any other place a PackageIDSpec may show up, like `cargo check -p <name>`@<spec>``
  - We explicitly kept the same semantics of version requirements that pre-releases require opt-in.  If nothing else, this gives us more room to change semantics in the future if we ever fix the semantics for pre-release.
- `replace`s that don't specify version metadata when the `Cargo.lock` contained a version metadata, it would previously be ignored (with a warning) but now match **(unchanged with this PR)**
  - When the version metadata in `Cargo.lock` differed from the overriding `Cargo.toml`, cargo would panic **(now an error in this PR)**

With this PR, we are acknowledging that we changed behavior in taking ignored replaces (because of differences with version metadata) and applying them.  Seeing as version metadata is relatively rare, replaces are relatively rare, and differences in it for registries is unsupported, the impact seems very small.

The questions before us are
- Do we revert #12614 in `master` and `rust-1.74.0` or merge this PR into `master`
- If we merge this PR into `master`, do we cherry-pick this into `rust-1.74.0` or revert #12614, giving ourselves more time to find problems

### How should we test and review this PR?

The initial commit adds tests that pass as of #12614.  Prior to #12614, these tests would have warned that the `replace` was unused and failed because `bar::bar` didn't exist.  Each commit then changes the behavior (or not) and updates the corresponding test.

### Additional information

src/cargo/core/package_id_spec.rs

@@ -176,8 +176,7 @@ impl PackageIdSpec {
         }
 
         if let Some(ref v) = self.version {
-            let req = v.exact_req();
-            if !req.matches(package_id.version()) {
+            if !v.matches(package_id.version()) {
                 return false;
             }
         }
@@ -444,15 +443,50 @@ mod tests {
     fn matching() {
         let url = Url::parse("https://example.com").unwrap();
         let sid = SourceId::for_registry(&url).unwrap();
-        let foo = PackageId::new("foo", "1.2.3", sid).unwrap();
-        let bar = PackageId::new("bar", "1.2.3", sid).unwrap();
 
+        let foo = PackageId::new("foo", "1.2.3", sid).unwrap();
         assert!(PackageIdSpec::parse("foo").unwrap().matches(foo));
-        assert!(!PackageIdSpec::parse("foo").unwrap().matches(bar));
+        assert!(!PackageIdSpec::parse("bar").unwrap().matches(foo));
         assert!(PackageIdSpec::parse("foo:1.2.3").unwrap().matches(foo));
         assert!(!PackageIdSpec::parse("foo:1.2.2").unwrap().matches(foo));
         assert!(PackageIdSpec::parse("foo@1.2.3").unwrap().matches(foo));
         assert!(!PackageIdSpec::parse("foo@1.2.2").unwrap().matches(foo));
         assert!(PackageIdSpec::parse("foo@1.2").unwrap().matches(foo));
+
+        let meta = PackageId::new("meta", "1.2.3+hello", sid).unwrap();
+        assert!(PackageIdSpec::parse("meta").unwrap().matches(meta));
+        assert!(PackageIdSpec::parse("meta@1").unwrap().matches(meta));
+        assert!(PackageIdSpec::parse("meta@1.2").unwrap().matches(meta));
+        assert!(PackageIdSpec::parse("meta@1.2.3").unwrap().matches(meta));
+        assert!(!PackageIdSpec::parse("meta@1.2.3-alpha.0")
+            .unwrap()
+            .matches(meta));
+        assert!(PackageIdSpec::parse("meta@1.2.3+hello")
+            .unwrap()
+            .matches(meta));
+        assert!(!PackageIdSpec::parse("meta@1.2.3+bye")
+            .unwrap()
+            .matches(meta));
+
+        let pre = PackageId::new("pre", "1.2.3-alpha.0", sid).unwrap();
+        assert!(PackageIdSpec::parse("pre").unwrap().matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1").unwrap().matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1.2").unwrap().matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1.2.3").unwrap().matches(pre));
+        assert!(PackageIdSpec::parse("pre@1.2.3-alpha.0")
+            .unwrap()
+            .matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1.2.3-alpha.1")
+            .unwrap()
+            .matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1.2.3-beta.0")
+            .unwrap()
+            .matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1.2.3+hello")
+            .unwrap()
+            .matches(pre));
+        assert!(!PackageIdSpec::parse("pre@1.2.3-alpha.0+hello")
+            .unwrap()
+            .matches(pre));
     }
 }

src/cargo/core/resolver/dep_cache.rs

@@ -173,11 +173,20 @@ impl<'a> RegistryQueryer<'a> {
                 )));
             }
 
-            // The dependency should be hard-coded to have the same name and an
-            // exact version requirement, so both of these assertions should
-            // never fail.
-            assert_eq!(s.version(), summary.version());
-            assert_eq!(s.name(), summary.name());
+            assert_eq!(
+                s.name(),
+                summary.name(),
+                "dependency should be hard coded to have the same name"
+            );
+            if s.version() != summary.version() {
+                return Poll::Ready(Err(anyhow::anyhow!(
+                    "replacement specification `{}` matched {} and tried to override it with {}\n\
+                     avoid matching unrelated packages by being more specific",
+                    spec,
+                    summary.version(),
+                    s.version(),
+                )));
+            }
 
             let replace = if s.source_id() == summary.source_id() {
                 debug!("Preventing\n{:?}\nfrom replacing\n{:?}", summary, s);

src/cargo/util/semver_ext.rs

@@ -182,16 +182,25 @@ impl PartialVersion {
         }
     }
 
-    pub fn exact_req(&self) -> VersionReq {
-        VersionReq {
-            comparators: vec![Comparator {
-                op: semver::Op::Exact,
-                major: self.major,
-                minor: self.minor,
-                patch: self.patch,
-                pre: self.pre.as_ref().cloned().unwrap_or_default(),
-            }],
+    /// Check if this matches a version, including build metadata
+    ///
+    /// Build metadata does not affect version precedence but may be necessary for uniquely
+    /// identifying a package.
+    pub fn matches(&self, version: &Version) -> bool {
+        if !version.pre.is_empty() && self.pre.is_none() {
+            // Pre-release versions must be explicitly opted into, if for no other reason than to
+            // give us room to figure out and define the semantics
+            return false;
         }
+        self.major == version.major
+            && self.minor.map(|f| f == version.minor).unwrap_or(true)
+            && self.patch.map(|f| f == version.patch).unwrap_or(true)
+            && self.pre.as_ref().map(|f| f == &version.pre).unwrap_or(true)
+            && self
+                .build
+                .as_ref()
+                .map(|f| f == &version.build)
+                .unwrap_or(true)
     }
 }
 

tests/testsuite/replace.rs

@@ -1298,3 +1298,157 @@ fn override_plus_dep() {
         .with_stderr_contains("error: cyclic package dependency: [..]")
         .run();
 }
+
+#[cargo_test]
+fn override_generic_matching_other_versions() {
+    Package::new("bar", "0.1.0+a").publish();
+
+    let bar = git::repo(&paths::root().join("override"))
+        .file("Cargo.toml", &basic_manifest("bar", "0.1.0"))
+        .file("src/lib.rs", "pub fn bar() {}")
+        .build();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [package]
+                    name = "foo"
+                    version = "0.0.1"
+                    authors = []
+
+                    [dependencies]
+                    bar = "0.1.0"
+
+                    [replace]
+                    "bar:0.1.0" = {{ git = '{}' }}
+                "#,
+                bar.url()
+            ),
+        )
+        .file(
+            "src/lib.rs",
+            "extern crate bar; pub fn foo() { bar::bar(); }",
+        )
+        .build();
+
+    p.cargo("check")
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+[UPDATING] git repository `[..]`
+[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 ([..]/foo)`
+
+Caused by:
+  replacement specification `https://github.com/rust-lang/crates.io-index#bar@0.1.0` matched 0.1.0+a and tried to override it with 0.1.0
+  avoid matching unrelated packages by being more specific
+",
+        )
+        .with_status(101)
+        .run();
+}
+
+#[cargo_test]
+fn override_respects_spec_metadata() {
+    Package::new("bar", "0.1.0+a").publish();
+
+    let bar = git::repo(&paths::root().join("override"))
+        .file("Cargo.toml", &basic_manifest("bar", "0.1.0+a"))
+        .file("src/lib.rs", "pub fn bar() {}")
+        .build();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [package]
+                    name = "foo"
+                    version = "0.0.1"
+                    authors = []
+
+                    [dependencies]
+                    bar = "0.1.0"
+
+                    [replace]
+                    "bar:0.1.0+notTheBuild" = {{ git = '{}' }}
+                "#,
+                bar.url()
+            ),
+        )
+        .file(
+            "src/lib.rs",
+            "extern crate bar; pub fn foo() { bar::bar(); }",
+        )
+        .build();
+
+    p.cargo("check")
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+[WARNING] package replacement is not used: https://github.com/rust-lang/crates.io-index#bar@0.1.0+notTheBuild
+[DOWNLOADING] crates ...
+[DOWNLOADED] bar v0.1.0+a (registry `dummy-registry`)
+[CHECKING] bar v0.1.0+a
+[CHECKING] foo v0.0.1 ([..]/foo)
+[..]
+[..]
+[..]
+[..]
+[..]
+[..]
+[..]
+error: could not compile `foo` (lib) due to previous error
+",
+        )
+        .with_status(101)
+        .run();
+}
+
+#[cargo_test]
+fn override_spec_metadata_is_optional() {
+    Package::new("bar", "0.1.0+a").publish();
+
+    let bar = git::repo(&paths::root().join("override"))
+        .file("Cargo.toml", &basic_manifest("bar", "0.1.0+a"))
+        .file("src/lib.rs", "pub fn bar() {}")
+        .build();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [package]
+                    name = "foo"
+                    version = "0.0.1"
+                    authors = []
+
+                    [dependencies]
+                    bar = "0.1.0"
+
+                    [replace]
+                    "bar:0.1.0" = {{ git = '{}' }}
+                "#,
+                bar.url()
+            ),
+        )
+        .file(
+            "src/lib.rs",
+            "extern crate bar; pub fn foo() { bar::bar(); }",
+        )
+        .build();
+
+    p.cargo("check")
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+[UPDATING] git repository `[..]`
+[CHECKING] bar v0.1.0+a (file://[..])
+[CHECKING] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+}