Auto merge of #12591 - epage:spec, r=weihanglo

Prepare for partial-version package specs ### What does this PR try to resolve? These are refactorings, test expansions, and cleanups I saw as I was preparing to implement support for `[email protected]` as proposed in #12425. I figured these changes stand on their own so I separated them out. One further change I considered was that `foo@0` will suggest `foo` in a "did you mean" message. This is a big off *but* most likely any fix for this would be undone by the work to support `[email protected]`, so I held off on it. ### How should we test and review this PR? Each change is broken down into an individual commit
rust-lang · Aug 31, 2023 · 0de91c8 · 0de91c8
2 parents 5aca9af + a78bba7
commit 0de91c8
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 35 deletions.
diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs
@@ -261,7 +261,7 @@ fn parse_semver_flag(v: &str) -> CargoResult<VersionReq> {
                 // requirement, add a note to the warning
                 if v.parse::<VersionReq>().is_ok() {
                     msg.push_str(&format!(
-                        "\n\n  tip: if you want to specify semver range, \
+                        "\n\n  tip: if you want to specify SemVer range, \
                              add an explicit qualifier, like '^{}'",
                         v
                     ));

diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs
@@ -143,10 +143,10 @@ impl std::str::FromStr for PartialVersion {
         let version_req = match semver::VersionReq::parse(value) {
             // Exclude semver operators like `^` and pre-release identifiers
             Ok(req) if value.chars().all(|c| c.is_ascii_digit() || c == '.') => req,
-            Err(_) if value.contains('+') => {
+            _ if value.contains('+') => {
                 anyhow::bail!("unexpected build field, expected a version like \"1.32\"")
             }
-            Err(_) if value.contains('-') => {
+            _ if value.contains('-') => {
                 anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"")
             }
             _ => anyhow::bail!("expected a version like \"1.32\""),

diff --git a/src/cargo/util/to_semver.rs b/src/cargo/util/to_semver.rs
@@ -15,7 +15,10 @@ impl<'a> ToSemver for &'a str {
     fn to_semver(self) -> CargoResult<Version> {
         match Version::parse(self.trim()) {
             Ok(v) => Ok(v),
-            Err(..) => Err(anyhow::format_err!("cannot parse '{}' as a semver", self)),
+            Err(..) => Err(anyhow::format_err!(
+                "cannot parse '{}' as a SemVer version",
+                self
+            )),
         }
     }
 }

diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs
@@ -230,9 +230,9 @@ fn ambiguous_version_no_longer_allowed() {
     cargo_process("install foo --version=1.0")
         .with_stderr(
             "\
-[ERROR] invalid value '1.0' for '--version <VERSION>': cannot parse '1.0' as a semver
+[ERROR] invalid value '1.0' for '--version <VERSION>': cannot parse '1.0' as a SemVer version
 
-  tip: if you want to specify semver range, add an explicit qualifier, like '^1.0'
+  tip: if you want to specify SemVer range, add an explicit qualifier, like '^1.0'
 
 For more information, try '--help'.
 ",

diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs
@@ -4,22 +4,31 @@ use cargo_test_support::project;
 use cargo_test_support::registry::Package;
 
 #[cargo_test]
-fn simple() {
-    Package::new("bar", "0.1.0").publish();
+fn local() {
     let p = project()
         .file(
             "Cargo.toml",
             r#"
+                [workspace]
+                members = ["bar"]
+
                 [package]
                 name = "foo"
                 version = "0.1.0"
                 edition = "2018"
-
-                [dependencies]
-                bar = "0.1.0"
             "#,
         )
         .file("src/main.rs", "fn main() {}")
+        .file(
+            "bar/Cargo.toml",
+            r#"
+                [package]
+                name = "bar"
+                version = "0.1.0"
+                edition = "2018"
+            "#,
+        )
+        .file("bar/src/main.rs", "fn main() {}")
         .build();
 
     p.cargo("generate-lockfile").run();
@@ -28,16 +37,38 @@ fn simple() {
         .with_stdout(format!("file://[..]{}#0.1.0", p.root().to_str().unwrap()))
         .run();
 
-    p.cargo("pkgid bar")
-        .with_stdout("https://github.com/rust-lang/crates.io-index#[email protected]")
+    // Bad file URL.
+    p.cargo("pkgid ./Cargo.toml")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: invalid package ID specification: `./Cargo.toml`
+
+Caused by:
+  package ID specification `./Cargo.toml` looks like a file path, maybe try file://[..]/Cargo.toml
+",
+        )
+        .run();
+
+    // Bad file URL with similar name.
+    p.cargo("pkgid './bar'")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: invalid package ID specification: `./bar`
+
+<tab>Did you mean `bar`?
+
+Caused by:
+  package ID specification `./bar` looks like a file path, maybe try file://[..]/bar
+",
+        )
         .run();
 }
 
 #[cargo_test]
-fn suggestion_bad_pkgid() {
+fn registry() {
     Package::new("crates-io", "0.1.0").publish();
-    Package::new("two-ver", "0.1.0").publish();
-    Package::new("two-ver", "0.2.0").publish();
     let p = project()
         .file(
             "Cargo.toml",
@@ -49,16 +80,18 @@ fn suggestion_bad_pkgid() {
 
                 [dependencies]
                 crates-io = "0.1.0"
-                two-ver = "0.1.0"
-                two-ver2 = { package = "two-ver", version = "0.2.0" }
             "#,
         )
-        .file("src/lib.rs", "")
+        .file("src/main.rs", "fn main() {}")
         .file("cratesio", "")
         .build();
 
     p.cargo("generate-lockfile").run();
 
+    p.cargo("pkgid crates-io")
+        .with_stdout("https://github.com/rust-lang/crates.io-index#[email protected]")
+        .run();
+
     // Bad URL.
     p.cargo("pkgid https://example.com/crates-io")
         .with_status(101)
@@ -83,45 +116,87 @@ error: package ID specification `crates_io` did not match any packages
 ",
         )
         .run();
+}
 
-    // Bad version.
-    p.cargo("pkgid two-ver:0.3.0")
+#[cargo_test]
+fn multiple_versions() {
+    Package::new("two-ver", "0.1.0").publish();
+    Package::new("two-ver", "0.2.0").publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+                edition = "2018"
+
+                [dependencies]
+                two-ver = "0.1.0"
+                two-ver2 = { package = "two-ver", version = "0.2.0" }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file("cratesio", "")
+        .build();
+
+    p.cargo("generate-lockfile").run();
+
+    p.cargo("pkgid two-ver:0.2.0")
+        .with_stdout("https://github.com/rust-lang/crates.io-index#[email protected]")
+        .run();
+
+    // Incomplete version.
+    p.cargo("pkgid two-ver@0")
         .with_status(101)
         .with_stderr(
             "\
-error: package ID specification `[email protected]` did not match any packages
-Did you mean one of these?
+error: invalid package ID specification: `two-ver@0`
 
-  [email protected]
-  [email protected]
+<tab>Did you mean `two-ver`?
+
+Caused by:
+  cannot parse '0' as a SemVer version
 ",
         )
         .run();
 
-    // Bad file URL.
-    p.cargo("pkgid ./Cargo.toml")
+    // Incomplete version.
+    p.cargo("pkgid [email protected]")
         .with_status(101)
         .with_stderr(
             "\
-error: invalid package ID specification: `./Cargo.toml`
+error: invalid package ID specification: `[email protected]`
 
 Caused by:
-  package ID specification `./Cargo.toml` looks like a file path, maybe try file://[..]/Cargo.toml
+  cannot parse '0.2' as a SemVer version
 ",
         )
         .run();
 
-    // Bad file URL with similar name.
-    p.cargo("pkgid './cratesio'")
+    // Ambiguous.
+    p.cargo("pkgid two-ver")
         .with_status(101)
         .with_stderr(
             "\
-error: invalid package ID specification: `./cratesio`
+error: There are multiple `two-ver` packages in your project, and the specification `two-ver` is ambiguous.
+Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
+  [email protected]
+  [email protected]
+",
+        )
+        .run();
 
-<tab>Did you mean `crates-io`?
+    // Bad version.
+    p.cargo("pkgid two-ver:0.3.0")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: package ID specification `[email protected]` did not match any packages
+Did you mean one of these?
 
-Caused by:
-  package ID specification `./cratesio` looks like a file path, maybe try file://[..]/cratesio
+  [email protected]
+  [email protected]
 ",
         )
         .run();

diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs
@@ -57,6 +57,39 @@ Caused by:
         .run();
 }
 
+#[cargo_test]
+fn rust_version_good_pre_release() {
+    project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            rust-version = "1.43.0-beta.1"
+            [[bin]]
+            name = "foo"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build()
+        .cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: failed to parse manifest at `[..]`
+
+Caused by:
+  TOML parse error at line 6, column 28
+    |
+  6 |             rust-version = \"1.43.0-beta.1\"
+    |                            ^^^^^^^^^^^^^^^
+  unexpected prerelease field, expected a version like \"1.32\"",
+        )
+        .run();
+}
+
 #[cargo_test]
 fn rust_version_bad_pre_release() {
     project()