From 3138f91d4de2501f9358f67712a86ed21ddc0396 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 10:29:09 -0500 Subject: [PATCH 1/7] test(pkgid): Include test for ambiguous spec --- tests/testsuite/pkgid.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 3e3e4692a63..5163476a59f 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -80,6 +80,19 @@ Did you mean one of these? error: package ID specification `crates_io` did not match any packages Did you mean `crates-io`? +", + ) + .run(); + + // Ambiguous. + p.cargo("pkgid two-ver") + .with_status(101) + .with_stderr( + "\ +error: There are multiple `two-ver` packages in your project, and the specification `two-ver` is ambiguous. +Please re-run this command with `-p ` where `` is one of the following: + two-ver@0.1.0 + two-ver@0.2.0 ", ) .run(); From 018b7589e8721f4ca3202df6b90aa0770e167f09 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 10:36:36 -0500 Subject: [PATCH 2/7] test(pkgid): Focus tests on use cases, rather than success/failure I assume the reason these aren't all individual tests is test-time but if we divide by success/failure, I'll need to duplicate things to handle partial versions. Overall, I feel like this makes the tests make more sense. --- tests/testsuite/pkgid.rs | 116 +++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 41 deletions(-) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 5163476a59f..f9f42edd2da 100644 --- 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#bar@0.1.0") + // 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` + +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#crates-io@0.1.0") + .run(); + // Bad URL. p.cargo("pkgid https://example.com/crates-io") .with_status(101) @@ -83,6 +116,35 @@ error: package ID specification `crates_io` did not match any packages ", ) .run(); +} + +#[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#two-ver@0.2.0") + .run(); // Ambiguous. p.cargo("pkgid two-ver") @@ -107,34 +169,6 @@ Did you mean one of these? two-ver@0.1.0 two-ver@0.2.0 -", - ) - .run(); - - // 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 './cratesio'") - .with_status(101) - .with_stderr( - "\ -error: invalid package ID specification: `./cratesio` - -Did you mean `crates-io`? - -Caused by: - package ID specification `./cratesio` looks like a file path, maybe try file://[..]/cratesio ", ) .run(); From 3d2f3717372ce6276e8eeb4d0413aa2833e23776 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 10:40:47 -0500 Subject: [PATCH 3/7] test(pkgid): Add cases for partial versions --- tests/testsuite/pkgid.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index f9f42edd2da..7cb633485f2 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -146,6 +146,34 @@ fn multiple_versions() { .with_stdout("https://github.com/rust-lang/crates.io-index#two-ver@0.2.0") .run(); + // Incomplete version. + p.cargo("pkgid two-ver@0") + .with_status(101) + .with_stderr( + "\ +error: invalid package ID specification: `two-ver@0` + +Did you mean `two-ver`? + +Caused by: + cannot parse '0' as a semver +", + ) + .run(); + + // Incomplete version. + p.cargo("pkgid two-ver@0.2") + .with_status(101) + .with_stderr( + "\ +error: invalid package ID specification: `two-ver@0.2` + +Caused by: + cannot parse '0.2' as a semver +", + ) + .run(); + // Ambiguous. p.cargo("pkgid two-ver") .with_status(101) From 4238f689f54c92e455e0c5b12618ffa380dacd55 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 10:49:02 -0500 Subject: [PATCH 4/7] fix(install): More consistently refer to SemVer --- src/bin/cargo/commands/install.rs | 2 +- tests/testsuite/install_upgrade.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 4ec3f6951e6..b4937fa6f6f 100644 --- 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 { // requirement, add a note to the warning if v.parse::().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/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index 100fe1fde11..5c1818da743 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -232,7 +232,7 @@ fn ambiguous_version_no_longer_allowed() { "\ [ERROR] invalid value '1.0' for '--version ': cannot parse '1.0' as a semver - 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'. ", From c88137cd620729abbb272a5e083231c0774fd8d4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 10:49:09 -0500 Subject: [PATCH 5/7] fix(pkgid): More consistently refer to SemVer --- src/cargo/util/to_semver.rs | 5 ++++- tests/testsuite/install_upgrade.rs | 2 +- tests/testsuite/pkgid.rs | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/to_semver.rs b/src/cargo/util/to_semver.rs index 25da9dfb975..3cc9e5706ac 100644 --- 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 { 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 index 5c1818da743..580117f5cdc 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -230,7 +230,7 @@ fn ambiguous_version_no_longer_allowed() { cargo_process("install foo --version=1.0") .with_stderr( "\ -[ERROR] invalid value '1.0' for '--version ': cannot parse '1.0' as a semver +[ERROR] invalid value '1.0' for '--version ': cannot parse '1.0' as a SemVer version tip: if you want to specify SemVer range, add an explicit qualifier, like '^1.0' diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 7cb633485f2..1857f43b185 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -156,7 +156,7 @@ error: invalid package ID specification: `two-ver@0` Did you mean `two-ver`? Caused by: - cannot parse '0' as a semver + cannot parse '0' as a SemVer version ", ) .run(); @@ -169,7 +169,7 @@ Caused by: error: invalid package ID specification: `two-ver@0.2` Caused by: - cannot parse '0.2' as a semver + cannot parse '0.2' as a SemVer version ", ) .run(); From 9fb8128d9e9eaa0a564a43324f8c1b9009348ad0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 20:29:13 -0500 Subject: [PATCH 6/7] test(manifest): Verify error on good pre-release --- tests/testsuite/rust_version.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 4f65dcdc6c8..0f4df2c440c 100644 --- 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\" + | ^^^^^^^^^^^^^^^ + expected a version like \"1.32\"", + ) + .run(); +} + #[cargo_test] fn rust_version_bad_pre_release() { project() From a78bba7a92c26d05329d19ea2d067e03e1dd83c8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 29 Aug 2023 20:31:40 -0500 Subject: [PATCH 7/7] fix(manifest): Improve error on good pre-release --- src/cargo/util/semver_ext.rs | 4 ++-- tests/testsuite/rust_version.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index 9e0d3e50efe..5ec3f58a1d3 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -142,10 +142,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/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 0f4df2c440c..21321b7c579 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -85,7 +85,7 @@ Caused by: | 6 | rust-version = \"1.43.0-beta.1\" | ^^^^^^^^^^^^^^^ - expected a version like \"1.32\"", + unexpected prerelease field, expected a version like \"1.32\"", ) .run(); }