Skip to content

Commit

Permalink
Extend pkgid syntax with @ support
Browse files Browse the repository at this point in the history
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 rust-lang#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
  • Loading branch information
epage authored and Hezuikn committed Sep 22, 2022
1 parent eedeeff commit 7a59f0b
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 41 deletions.
8 changes: 4 additions & 4 deletions src/cargo/core/compiler/future_incompat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ fn render_report(per_package_reports: &[FutureIncompatReportPackage]) -> BTreeMa
let mut report: BTreeMap<String, String> = BTreeMap::new();
for per_package in per_package_reports {
let package_spec = format!(
"{}:{}",
"{}@{}",
per_package.package_id.name(),
per_package.package_id.version()
);
Expand Down Expand Up @@ -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
Expand Down
45 changes: 38 additions & 7 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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()?),
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)));
}
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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(
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -366,6 +381,7 @@ mod tests {
version: None,
url: None,
},
"foo",
);
ok(
"foo:1.2.3",
Expand All @@ -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",
);
}

Expand All @@ -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());
}
Expand All @@ -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));
}
}
9 changes: 3 additions & 6 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,20 +896,17 @@ 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 {}",
registry.host()
)
})?;
} 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()))?;
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ fn yank() {
.with_stderr(
"\
[UPDATING] [..]
[YANK] foo:0.1.0
[YANK] foo@0.1.0
",
)
.run();
Expand Down
10 changes: 5 additions & 5 deletions tests/testsuite/future_incompat_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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[..]")
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1750,8 +1750,8 @@ fn update_ambiguous() {
is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the \
following:
bar:0.[..].0
bar:0.[..].0
bar@0.[..].0
bar@0.[..].0
",
)
.run();
Expand Down
10 changes: 5 additions & 5 deletions tests/testsuite/pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/profile_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/profile_overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down
10 changes: 5 additions & 5 deletions tests/testsuite/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
",
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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] [..]
",
)
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <spec>` where `<spec>` is one of the following:
dep:1.0.0
dep:2.0.0
dep@1.0.0
dep@2.0.0
",
)
.with_status(101)
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 7a59f0b

Please # to comment.