From 7a59f0b9b99f9eaaeacefbd514f51ae6fb8d8b16 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Apr 2022 16:00:15 -0500 Subject: [PATCH] Extend pkgid syntax with `@` support In addition to `foo:1.2.3`, we now support `foo@1.2.3` for pkgids. We are also making it the default way of rendering pkgid's for the user. With cargo-add in #10472, we've decided to only use `@` in it and to add it as an alternative to `:` in the rest of cargo. `cargo-add` originally used `@`. When preparing it for merge, I switched to `:` to be consistent with pkgids. When discussing this, it was felt `@` has precedence in too many tools to switch to `:` but that we should instead switch pkgid's to use `@`, in a backwards compatible way. See also https://internals.rust-lang.org/t/feedback-on-cargo-add-before-its-merged/16024/26?u=epage --- src/cargo/core/compiler/future_incompat.rs | 8 ++-- src/cargo/core/package_id_spec.rs | 45 ++++++++++++++++++---- src/cargo/ops/registry.rs | 9 ++--- tests/testsuite/credential_process.rs | 2 +- tests/testsuite/future_incompat_report.rs | 10 ++--- tests/testsuite/git.rs | 4 +- tests/testsuite/pkgid.rs | 10 ++--- tests/testsuite/profile_config.rs | 2 +- tests/testsuite/profile_overrides.rs | 4 +- tests/testsuite/replace.rs | 10 ++--- tests/testsuite/tree.rs | 4 +- tests/testsuite/yank.rs | 2 +- 12 files changed, 69 insertions(+), 41 deletions(-) diff --git a/src/cargo/core/compiler/future_incompat.rs b/src/cargo/core/compiler/future_incompat.rs index 23538b8656d..c814842dff7 100644 --- a/src/cargo/core/compiler/future_incompat.rs +++ b/src/cargo/core/compiler/future_incompat.rs @@ -236,7 +236,7 @@ fn render_report(per_package_reports: &[FutureIncompatReportPackage]) -> BTreeMa let mut report: BTreeMap = BTreeMap::new(); for per_package in per_package_reports { let package_spec = format!( - "{}:{}", + "{}@{}", per_package.package_id.name(), per_package.package_id.version() ); @@ -415,10 +415,10 @@ You may want to consider updating them to a newer version to see if the issue ha let manifest = bcx.packages.get_one(*package_id).unwrap().manifest(); format!( " - - {name} + - {package_spec} - Repository: {url} - - Detailed warning command: `cargo report future-incompatibilities --id {id} --package {name}`", - name = format!("{}:{}", package_id.name(), package_id.version()), + - Detailed warning command: `cargo report future-incompatibilities --id {id} --package {package_spec}`", + package_spec = format!("{}@{}", package_id.name(), package_id.version()), url = manifest .metadata() .repository diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 723b624e8bf..29043b96326 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -41,8 +41,10 @@ impl PackageIdSpec { /// "https://crates.io/foo", /// "https://crates.io/foo#1.2.3", /// "https://crates.io/foo#bar:1.2.3", + /// "https://crates.io/foo#bar@1.2.3", /// "foo", /// "foo:1.2.3", + /// "foo@1.2.3", /// ]; /// for spec in specs { /// assert!(PackageIdSpec::parse(spec).is_ok()); @@ -65,7 +67,7 @@ impl PackageIdSpec { ); } } - let mut parts = spec.splitn(2, ':'); + let mut parts = spec.splitn(2, [':', '@']); let name = parts.next().unwrap(); let version = match parts.next() { Some(version) => Some(version.to_semver()?), @@ -122,7 +124,7 @@ impl PackageIdSpec { })?; match frag { Some(fragment) => { - let mut parts = fragment.splitn(2, ':'); + let mut parts = fragment.splitn(2, [':', '@']); let name_or_version = parts.next().unwrap(); match parts.next() { Some(part) => { @@ -268,7 +270,7 @@ impl PackageIdSpec { } for id in ids { if version_cnt[id.version()] == 1 { - msg.push_str(&format!("\n {}:{}", spec.name(), id.version())); + msg.push_str(&format!("\n {}@{}", spec.name(), id.version())); } else { msg.push_str(&format!("\n {}", PackageIdSpec::from_package_id(*id))); } @@ -290,11 +292,11 @@ impl fmt::Display for PackageIdSpec { } None => { printed_name = true; - write!(f, "{}", self.name)? + write!(f, "{}", self.name)?; } } if let Some(ref v) = self.version { - write!(f, "{}{}", if printed_name { ":" } else { "#" }, v)?; + write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; } Ok(()) } @@ -329,10 +331,11 @@ mod tests { #[test] fn good_parsing() { - fn ok(spec: &str, expected: PackageIdSpec) { + #[track_caller] + fn ok(spec: &str, expected: PackageIdSpec, expected_rendered: &str) { let parsed = PackageIdSpec::parse(spec).unwrap(); assert_eq!(parsed, expected); - assert_eq!(parsed.to_string(), spec); + assert_eq!(parsed.to_string(), expected_rendered); } ok( @@ -342,6 +345,7 @@ mod tests { version: None, url: Some(Url::parse("https://crates.io/foo").unwrap()), }, + "https://crates.io/foo", ); ok( "https://crates.io/foo#1.2.3", @@ -350,6 +354,7 @@ mod tests { version: Some("1.2.3".to_semver().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, + "https://crates.io/foo#1.2.3", ); ok( "https://crates.io/foo#bar:1.2.3", @@ -358,6 +363,16 @@ mod tests { version: Some("1.2.3".to_semver().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, + "https://crates.io/foo#bar@1.2.3", + ); + ok( + "https://crates.io/foo#bar@1.2.3", + PackageIdSpec { + name: InternedString::new("bar"), + version: Some("1.2.3".to_semver().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + }, + "https://crates.io/foo#bar@1.2.3", ); ok( "foo", @@ -366,6 +381,7 @@ mod tests { version: None, url: None, }, + "foo", ); ok( "foo:1.2.3", @@ -374,6 +390,16 @@ mod tests { version: Some("1.2.3".to_semver().unwrap()), url: None, }, + "foo@1.2.3", + ); + ok( + "foo@1.2.3", + PackageIdSpec { + name: InternedString::new("foo"), + version: Some("1.2.3".to_semver().unwrap()), + url: None, + }, + "foo@1.2.3", ); } @@ -382,6 +408,9 @@ mod tests { assert!(PackageIdSpec::parse("baz:").is_err()); assert!(PackageIdSpec::parse("baz:*").is_err()); assert!(PackageIdSpec::parse("baz:1.0").is_err()); + assert!(PackageIdSpec::parse("baz@").is_err()); + assert!(PackageIdSpec::parse("baz@*").is_err()); + assert!(PackageIdSpec::parse("baz@1.0").is_err()); assert!(PackageIdSpec::parse("https://baz:1.0").is_err()); assert!(PackageIdSpec::parse("https://#baz:1.0").is_err()); } @@ -397,5 +426,7 @@ mod tests { assert!(!PackageIdSpec::parse("foo").unwrap().matches(bar)); 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)); } } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 5233572f130..68f19392e0c 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -896,10 +896,9 @@ pub fn yank( let (mut registry, _, _) = registry(config, token, index.as_deref(), reg.as_deref(), true, true)?; + let package_spec = format!("{}@{}", name, version); if undo { - config - .shell() - .status("Unyank", format!("{}:{}", name, version))?; + config.shell().status("Unyank", package_spec)?; registry.unyank(&name, &version).with_context(|| { format!( "failed to undo a yank from the registry at {}", @@ -907,9 +906,7 @@ pub fn yank( ) })?; } else { - config - .shell() - .status("Yank", format!("{}:{}", name, version))?; + config.shell().status("Yank", package_spec)?; registry .yank(&name, &version) .with_context(|| format!("failed to yank from the registry at {}", registry.host()))?; diff --git a/tests/testsuite/credential_process.rs b/tests/testsuite/credential_process.rs index 8f06b18de08..33a36ceaf1b 100644 --- a/tests/testsuite/credential_process.rs +++ b/tests/testsuite/credential_process.rs @@ -361,7 +361,7 @@ fn yank() { .with_stderr( "\ [UPDATING] [..] -[YANK] foo:0.1.0 +[YANK] foo@0.1.0 ", ) .run(); diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs index c6cb2ffd324..bd6e443b686 100644 --- a/tests/testsuite/future_incompat_report.rs +++ b/tests/testsuite/future_incompat_report.rs @@ -139,7 +139,7 @@ frequency = 'never' .env("RUSTFLAGS", "-Zfuture-incompat-test") .with_stderr_contains(FUTURE_OUTPUT) .with_stderr_contains("warning: the following packages contain code that will be rejected by a future version of Rust: foo v0.0.0 [..]") - .with_stderr_contains(" - foo:0.0.0[..]") + .with_stderr_contains(" - foo@0.0.0[..]") .run(); } } @@ -189,17 +189,17 @@ fn test_multi_crate() { p.cargo(command).arg("--future-incompat-report") .env("RUSTFLAGS", "-Zfuture-incompat-test") .with_stderr_contains("warning: the following packages contain code that will be rejected by a future version of Rust: first-dep v0.0.1, second-dep v0.0.2") - .with_stderr_contains(" - first-dep:0.0.1") - .with_stderr_contains(" - second-dep:0.0.2") + .with_stderr_contains(" - first-dep@0.0.1") + .with_stderr_contains(" - second-dep@0.0.2") .run(); - p.cargo("report future-incompatibilities").arg("--package").arg("first-dep:0.0.1") + p.cargo("report future-incompatibilities").arg("--package").arg("first-dep@0.0.1") .with_stdout_contains("The package `first-dep v0.0.1` currently triggers the following future incompatibility lints:") .with_stdout_contains(FUTURE_OUTPUT) .with_stdout_does_not_contain("[..]second-dep-0.0.2/src[..]") .run(); - p.cargo("report future-incompatibilities").arg("--package").arg("second-dep:0.0.2") + p.cargo("report future-incompatibilities").arg("--package").arg("second-dep@0.0.2") .with_stdout_contains("The package `second-dep v0.0.2` currently triggers the following future incompatibility lints:") .with_stdout_contains(FUTURE_OUTPUT) .with_stdout_does_not_contain("[..]first-dep-0.0.1/src[..]") diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index ae38aa6fdaf..16e7e22f99e 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -1750,8 +1750,8 @@ fn update_ambiguous() { is ambiguous. Please re-run this command with `-p ` where `` is one of the \ following: - bar:0.[..].0 - bar:0.[..].0 + bar@0.[..].0 + bar@0.[..].0 ", ) .run(); diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 5cd4cd41b17..8f2c2c4a765 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -29,7 +29,7 @@ fn simple() { .run(); p.cargo("pkgid bar") - .with_stdout("https://github.com/rust-lang/crates.io-index#bar:0.1.0") + .with_stdout("https://github.com/rust-lang/crates.io-index#bar@0.1.0") .run(); } @@ -67,7 +67,7 @@ fn suggestion_bad_pkgid() { error: package ID specification `https://example.com/crates-io` did not match any packages Did you mean one of these? - crates-io:0.1.0 + crates-io@0.1.0 ", ) .run(); @@ -89,11 +89,11 @@ error: package ID specification `crates_io` did not match any packages .with_status(101) .with_stderr( "\ -error: package ID specification `two-ver:0.3.0` did not match any packages +error: package ID specification `two-ver@0.3.0` did not match any packages Did you mean one of these? - two-ver:0.1.0 - two-ver:0.2.0 + two-ver@0.1.0 + two-ver@0.2.0 ", ) .run(); diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index 93e76d4c0c1..8830dd20f67 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -168,7 +168,7 @@ fn profile_config_override_spec_multiple() { .with_stderr( "\ [ERROR] multiple package overrides in profile `dev` match package `bar v0.5.0 ([..])` -found package specs: bar, bar:0.5.0", +found package specs: bar, bar@0.5.0", ) .run(); } diff --git a/tests/testsuite/profile_overrides.rs b/tests/testsuite/profile_overrides.rs index 661fada49c8..882a7fabd02 100644 --- a/tests/testsuite/profile_overrides.rs +++ b/tests/testsuite/profile_overrides.rs @@ -71,7 +71,7 @@ fn profile_override_warnings() { p.cargo("build") .with_stderr_contains( "\ -[WARNING] profile package spec `bar:1.2.3` in profile `dev` \ +[WARNING] profile package spec `bar@1.2.3` in profile `dev` \ has a version or URL that does not match any of the packages: \ bar v0.5.0 ([..]/foo/bar) [WARNING] profile package spec `bart` in profile `dev` did not match any packages @@ -262,7 +262,7 @@ fn profile_override_spec_multiple() { .with_stderr_contains( "\ [ERROR] multiple package overrides in profile `dev` match package `bar v0.5.0 ([..])` -found package specs: bar, bar:0.5.0", +found package specs: bar, bar@0.5.0", ) .run(); } diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index 363e6c54b3b..f71c8047d09 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -645,7 +645,7 @@ fn override_wrong_name() { [ERROR] failed to get `baz` as a dependency of package `foo v0.0.1 ([..])` Caused by: - no matching package for override `[..]baz:0.1.0` found + no matching package for override `[..]baz@0.1.0` found location searched: file://[..] version required: =0.1.0 ", @@ -729,7 +729,7 @@ fn override_wrong_version() { error: failed to parse manifest at `[..]` Caused by: - replacements cannot specify a version requirement, but found one for `[..]bar:0.1.0` + replacements cannot specify a version requirement, but found one for `[..]bar@0.1.0` ", ) .run(); @@ -826,8 +826,8 @@ fn test_override_dep() { "\ error: There are multiple `bar` packages in your project, and the [..] Please re-run this command with [..] - [..]#bar:0.1.0 - [..]#bar:0.1.0 + [..]#bar@0.1.0 + [..]#bar@0.1.0 ", ) .run(); @@ -1082,7 +1082,7 @@ fn overriding_nonexistent_no_spurious() { p.cargo("build") .with_stderr( "\ -[WARNING] package replacement is not used: [..]baz:0.1.0 +[WARNING] package replacement is not used: [..]baz@0.1.0 [FINISHED] [..] ", ) diff --git a/tests/testsuite/tree.rs b/tests/testsuite/tree.rs index 53c992d2d56..d053c473176 100644 --- a/tests/testsuite/tree.rs +++ b/tests/testsuite/tree.rs @@ -1569,8 +1569,8 @@ fn ambiguous_name() { "\ error: There are multiple `dep` packages in your project, and the specification `dep` is ambiguous. Please re-run this command with `-p ` where `` is one of the following: - dep:1.0.0 - dep:2.0.0 + dep@1.0.0 + dep@2.0.0 ", ) .with_status(101) diff --git a/tests/testsuite/yank.rs b/tests/testsuite/yank.rs index 27b542dca60..259c5946566 100644 --- a/tests/testsuite/yank.rs +++ b/tests/testsuite/yank.rs @@ -38,7 +38,7 @@ fn simple() { .with_status(101) .with_stderr( " Updating `[..]` index - Unyank foo:0.0.1 + Unyank foo@0.0.1 error: failed to undo a yank from the registry at file:///[..] Caused by: