Skip to content

Commit

Permalink
Auto merge of #10582 - epage:pkgid, r=ehuss
Browse files Browse the repository at this point in the history
Extend pkgid syntax with `@` support

In addition to `foo:1.2.3`, we now support `[email protected]` for pkgids.  We
are also making it the default way of rendering pkgid's for the user.

### What does this PR try to resolve?

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.  #10472 served
as the change proposal for this

See also
- https://internals.rust-lang.org/t/feedback-on-cargo-add-before-its-merged/16024/26?u=epage
- https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Multiple.20ways.20of.20specifying.20versions

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

The focus of the testing is on the parsers unit tests and on the end-to-end output.  We are not explicitly testing end-to-end input in this PR, assuming the unit tests are sufficient.

### Additional information

This only focuses on places we already accept pkgids.  Looking into supporting `[email protected]` in `cargo install` and `cargo yank` is being left for a future PR.
  • Loading branch information
bors committed May 6, 2022
2 parents acaf8e2 + 709f1be commit 0f75da6
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 61 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#[email protected]",
/// "foo",
/// "foo:1.2.3",
/// "[email protected]",
/// ];
/// 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#[email protected]",
);
ok(
"https://crates.io/foo#[email protected]",
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#[email protected]",
);
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,
},
"[email protected]",
);
ok(
"[email protected]",
PackageIdSpec {
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
url: None,
},
"[email protected]",
);
}

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("[email protected]").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("[email protected]").unwrap().matches(foo));
assert!(!PackageIdSpec::parse("[email protected]").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
6 changes: 3 additions & 3 deletions src/doc/man/cargo-pkgid.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ following:
SPEC Structure | Example SPEC
---------------------------|--------------
_name_ | `bitflags`
_name_`:`_version_ | `bitflags:1.0.4`
_name_`@`_version_ | `bitflags@1.0.4`
_url_ | `https://github.com/rust-lang/cargo`
_url_`#`_version_ | `https://github.com/rust-lang/cargo#0.33.0`
_url_`#`_name_ | `https://github.com/rust-lang/crates.io-index#bitflags`
_url_`#`_name_`:`_version_ | `https://github.com/rust-lang/cargo#crates-io:0.21.0`
_url_`#`_name_`:`_version_ | `https://github.com/rust-lang/cargo#crates-io@0.21.0`

## OPTIONS

Expand Down Expand Up @@ -75,7 +75,7 @@ Get the package ID for the given package instead of the current package.

2. Retrieve package specification for version 1.0.0 of `foo`:

cargo pkgid foo:1.0.0
cargo pkgid foo@1.0.0

3. Retrieve package specification for `foo` from crates.io:

Expand Down
6 changes: 3 additions & 3 deletions src/doc/man/generated_txt/cargo-pkgid.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ DESCRIPTION
+-----------------+--------------------------------------------------+
| name | bitflags |
+-----------------+--------------------------------------------------+
| name:version | bitflags:1.0.4 |
| name@version | bitflags@1.0.4 |
+-----------------+--------------------------------------------------+
| url | https://github.com/rust-lang/cargo |
+-----------------+--------------------------------------------------+
Expand All @@ -36,7 +36,7 @@ DESCRIPTION
| | https://github.com/rust-lang/crates.io-index#bitflags |
+-----------------+--------------------------------------------------+
| | |
| url#name:version | https://github.com/rust-lang/cargo#crates-io:0.21.0 |
| url#name:version | https://github.com/rust-lang/cargo#crates-io@0.21.0 |
+-----------------+--------------------------------------------------+

OPTIONS
Expand Down Expand Up @@ -133,7 +133,7 @@ EXAMPLES

2. Retrieve package specification for version 1.0.0 of foo:

cargo pkgid foo:1.0.0
cargo pkgid foo@1.0.0

3. Retrieve package specification for foo from crates.io:

Expand Down
6 changes: 3 additions & 3 deletions src/doc/src/commands/cargo-pkgid.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ following:
SPEC Structure | Example SPEC
---------------------------|--------------
_name_ | `bitflags`
_name_`:`_version_ | `bitflags:1.0.4`
_name_`@`_version_ | `bitflags@1.0.4`
_url_ | `https://github.com/rust-lang/cargo`
_url_`#`_version_ | `https://github.com/rust-lang/cargo#0.33.0`
_url_`#`_name_ | `https://github.com/rust-lang/crates.io-index#bitflags`
_url_`#`_name_`:`_version_ | `https://github.com/rust-lang/cargo#crates-io:0.21.0`
_url_`#`_name_`:`_version_ | `https://github.com/rust-lang/cargo#crates-io@0.21.0`

## OPTIONS

Expand Down Expand Up @@ -159,7 +159,7 @@ details on environment variables that Cargo reads.

2. Retrieve package specification for version 1.0.0 of `foo`:

cargo pkgid foo:1.0.0
cargo pkgid foo@1.0.0

3. Retrieve package specification for `foo` from crates.io:

Expand Down
14 changes: 7 additions & 7 deletions src/doc/src/reference/pkgid-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ is a string which is used to uniquely refer to one package within a graph of
packages.

The specification may be fully qualified, such as
`https://github.com/rust-lang/crates.io-index#regex:1.4.3` or it may be
`https://github.com/rust-lang/crates.io-index#regex@1.4.3` or it may be
abbreviated, such as `regex`. The abbreviated form may be used as long as it
uniquely identifies a single package in the dependency graph. If there is
ambiguity, additional qualifiers can be added to make it unique. For example,
if there are two versions of the `regex` package in the graph, then it can be
qualified with a version to make it unique, such as `regex:1.4.3`.
qualified with a version to make it unique, such as `regex@1.4.3`.

#### Specification grammar

Expand All @@ -23,7 +23,7 @@ The formal grammar for a Package Id Specification is:
```notrust
spec := pkgname
| proto "://" hostname-and-path [ "#" ( pkgname | semver ) ]
pkgname := name [ ":" semver ]
pkgname := name [ ("@" | ":" ) semver ]
proto := "http" | "git" | ...
```
Expand All @@ -40,17 +40,17 @@ The following are references to the `regex` package on `crates.io`:
| Spec | Name | Version |
|:------------------------------------------------------------|:-------:|:-------:|
| `regex` | `regex` | `*` |
| `regex:1.4.3` | `regex` | `1.4.3` |
| `regex@1.4.3` | `regex` | `1.4.3` |
| `https://github.com/rust-lang/crates.io-index#regex` | `regex` | `*` |
| `https://github.com/rust-lang/crates.io-index#regex:1.4.3` | `regex` | `1.4.3` |
| `https://github.com/rust-lang/crates.io-index#regex@1.4.3` | `regex` | `1.4.3` |

The following are some examples of specs for several different git dependencies:

| Spec | Name | Version |
|:----------------------------------------------------------|:----------------:|:--------:|
| `https://github.com/rust-lang/cargo#0.52.0` | `cargo` | `0.52.0` |
| `https://github.com/rust-lang/cargo#cargo-platform:0.1.2` | <nobr>`cargo-platform`</nobr> | `0.1.2` |
| `ssh://[email protected]/rust-lang/regex.git#regex:1.4.3` | `regex` | `1.4.3` |
| `https://github.com/rust-lang/cargo#cargo-platform@0.1.2` | <nobr>`cargo-platform`</nobr> | `0.1.2` |
| `ssh://[email protected]/rust-lang/regex.git#regex@1.4.3` | `regex` | `1.4.3` |

Local packages on the filesystem can use `file://` URLs to reference them:

Expand Down
8 changes: 4 additions & 4 deletions src/etc/man/cargo-pkgid.1
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ T}:T{
\fBbitflags\fR
T}
T{
\fIname\fR\fB:\fR\fIversion\fR
\fIname\fR\fB@\fR\fIversion\fR
T}:T{
\fBbitflags:1.0.4\fR
\fBbitflags@1.0.4\fR
T}
T{
\fIurl\fR
Expand All @@ -58,7 +58,7 @@ T}
T{
\fIurl\fR\fB#\fR\fIname\fR\fB:\fR\fIversion\fR
T}:T{
\fBhttps://github.com/rust\-lang/cargo#crates\-io:0.21.0\fR
\fBhttps://github.com/rust\-lang/cargo#crates\-io@0.21.0\fR
T}
.TE
.sp
Expand Down Expand Up @@ -195,7 +195,7 @@ cargo pkgid foo
.sp
.RS 4
.nf
cargo pkgid foo:1.0.0
cargo pkgid foo@1.0.0
.fi
.RE
.RE
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
Loading

0 comments on commit 0f75da6

Please sign in to comment.