Skip to content

Commit

Permalink
Auto merge of #9188 - weihanglo:issue-9041, r=ehuss
Browse files Browse the repository at this point in the history
Package ID specification urls must contain a host

Resolves #9041

Not sure which commit breaks this. Cargo shipped with rust 1.46 didn't unwrap on a `None` but 1.47 did. Even checkouted to 149022b (cargo of rust 1.46), it still unwrap unexpectedly.

So I ended up following the [Specification grammer](https://doc.rust-lang.org/cargo/reference/pkgid-spec.html#specification-grammar) to make sure there is a `host` in the pkgid urls.

<details>
<summary>See console output</summary>

cargo of rust 1.46
```console
$ cargo +1.46 -vV
cargo 1.46.0 (149022b 2020-07-17)
release: 1.46.0
commit-hash: 149022b
commit-date: 2020-07-17

$ cargo +1.46 pkgid /path
error: package ID specification `/path` matched no packages
```

cargo of rust 1.47

```console
$ cargo +1.47 -vV
cargo 1.47.0 (f3c7e06 2020-08-28)
release: 1.47.0
commit-hash: f3c7e06
commit-date: 2020-08-28

$ cargo +1.47 pkgid /path
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/tools/cargo/src/cargo/core/package_id_spec.rs:234:50
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

cargo on commit 149022b

```console
$ git checkout 149022b
$ cargo run -- pkgid /path
   Compiling cargo-platform v0.1.1 ([..]/cargo/crates/cargo-platform)
   Compiling crates-io v0.31.1 ([..]/cargo/crates/crates-io)
   Compiling cargo v0.47.0 ([..]/cargo)
    Finished dev [unoptimized + debuginfo] target(s) in 22.90s
     Running `target/debug/cargo pkgid /path`
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/cargo/core/package_id_spec.rs:234:50
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
</details>
  • Loading branch information
bors committed Mar 9, 2021
2 parents e78f1c8 + c5d2304 commit 0f3f47e
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 46 deletions.
71 changes: 25 additions & 46 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,31 @@ impl PackageIdSpec {
/// use cargo::core::PackageIdSpec;
///
/// let specs = vec![
/// "https://crates.io/foo",
/// "https://crates.io/foo#1.2.3",
/// "https://crates.io/foo#bar:1.2.3",
/// "crates.io/foo",
/// "crates.io/foo#1.2.3",
/// "crates.io/foo#bar",
/// "crates.io/foo#bar:1.2.3",
/// "foo",
/// "foo:1.2.3",
/// ];
/// for spec in specs {
/// assert!(PackageIdSpec::parse(spec).is_ok());
/// }
pub fn parse(spec: &str) -> CargoResult<PackageIdSpec> {
if spec.contains('/') {
if spec.contains("://") {
if let Ok(url) = spec.into_url() {
return PackageIdSpec::from_url(url);
}
if !spec.contains("://") {
if let Ok(url) = Url::parse(&format!("cargo://{}", spec)) {
return PackageIdSpec::from_url(url);
}
} else if spec.contains('/') || spec.contains('\\') {
let abs = std::env::current_dir().unwrap_or_default().join(spec);
if abs.exists() {
let maybe_url = Url::from_file_path(abs)
.map_or_else(|_| "a file:// URL".to_string(), |url| url.to_string());
bail!(
"package ID specification `{}` looks like a file path, \
maybe try {}",
spec,
maybe_url
);
}
}
let mut parts = spec.splitn(2, ':');
Expand All @@ -80,8 +84,11 @@ impl PackageIdSpec {
where
I: IntoIterator<Item = PackageId>,
{
let spec = PackageIdSpec::parse(spec)
.chain_err(|| anyhow::format_err!("invalid package ID specification: `{}`", spec))?;
let i: Vec<_> = i.into_iter().collect();
let spec = PackageIdSpec::parse(spec).chain_err(|| {
let suggestion = lev_distance::closest_msg(spec, i.iter(), |id| id.name().as_str());
anyhow::format_err!("invalid package ID specification: `{}`{}", spec, suggestion)
})?;
spec.query(i)
}

Expand Down Expand Up @@ -275,11 +282,7 @@ impl fmt::Display for PackageIdSpec {
let mut printed_name = false;
match self.url {
Some(ref url) => {
if url.scheme() == "cargo" {
write!(f, "{}{}", url.host().unwrap(), url.path())?;
} else {
write!(f, "{}", url)?;
}
write!(f, "{}", url)?;
if url.path_segments().unwrap().next_back().unwrap() != &*self.name {
printed_name = true;
write!(f, "#{}", self.name)?;
Expand Down Expand Up @@ -333,51 +336,27 @@ mod tests {
}

ok(
"https://crates.io/foo#1.2.3",
PackageIdSpec {
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
);
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()),
},
);
ok(
"crates.io/foo",
"https://crates.io/foo",
PackageIdSpec {
name: InternedString::new("foo"),
version: None,
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo#1.2.3",
"https://crates.io/foo#1.2.3",
PackageIdSpec {
name: InternedString::new("foo"),
version: Some("1.2.3".to_semver().unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo#bar",
PackageIdSpec {
name: InternedString::new("bar"),
version: None,
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
);
ok(
"crates.io/foo#bar:1.2.3",
"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("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("https://crates.io/foo").unwrap()),
},
);
ok(
Expand Down
4 changes: 4 additions & 0 deletions src/doc/man/cargo-pkgid.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,9 @@ Get the package ID for the given package instead of the current package.

cargo pkgid https://github.com/rust-lang/crates.io-index#foo

4. Retrieve package specification for `foo` from a local package:

cargo pkgid file:///path/to/local/package#foo

## SEE ALSO
{{man "cargo" 1}}, {{man "cargo-generate-lockfile" 1}}, {{man "cargo-metadata" 1}}
4 changes: 4 additions & 0 deletions src/doc/man/generated_txt/cargo-pkgid.txt
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ EXAMPLES

cargo pkgid https://github.com/rust-lang/crates.io-index#foo

4. Retrieve package specification for foo from a local package:

cargo pkgid file:///path/to/local/package#foo

SEE ALSO
cargo(1), cargo-generate-lockfile(1), cargo-metadata(1)

4 changes: 4 additions & 0 deletions src/doc/src/commands/cargo-pkgid.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,9 @@ details on environment variables that Cargo reads.

cargo pkgid https://github.com/rust-lang/crates.io-index#foo

4. Retrieve package specification for `foo` from a local package:

cargo pkgid file:///path/to/local/package#foo

## SEE ALSO
[cargo(1)](cargo.html), [cargo-generate-lockfile(1)](cargo-generate-lockfile.html), [cargo-metadata(1)](cargo-metadata.html)
10 changes: 10 additions & 0 deletions src/etc/man/cargo-pkgid.1
Original file line number Diff line number Diff line change
Expand Up @@ -207,5 +207,15 @@ cargo pkgid https://github.com/rust\-lang/crates.io\-index#foo
.fi
.RE
.RE
.sp
.RS 4
\h'-04' 4.\h'+01'Retrieve package specification for \fBfoo\fR from a local package:
.sp
.RS 4
.nf
cargo pkgid file:///path/to/local/package#foo
.fi
.RE
.RE
.SH "SEE ALSO"
\fBcargo\fR(1), \fBcargo\-generate\-lockfile\fR(1), \fBcargo\-metadata\fR(1)
29 changes: 29 additions & 0 deletions tests/testsuite/pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ fn suggestion_bad_pkgid() {
"#,
)
.file("src/lib.rs", "")
.file("cratesio", "")
.build();

p.cargo("generate-lockfile").run();
Expand Down Expand Up @@ -93,6 +94,34 @@ 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 simliar name.
p.cargo("pkgid './cratesio'")
.with_status(101)
.with_stderr(
"\
error: invalid package ID specification: `./cratesio`
<tab>Did you mean `crates-io`?
Caused by:
package ID specification `./cratesio` looks like a file path, maybe try file://[..]/cratesio
",
)
.run();
Expand Down

0 comments on commit 0f3f47e

Please sign in to comment.