Skip to content

Commit 0f3f47e

Browse files
borsbors
committed
Auto merge of #9188 - weihanglo:issue-9041, r=ehuss
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>
2 parents e78f1c8 + c5d2304 commit 0f3f47e

File tree

6 files changed

+76
-46
lines changed

6 files changed

+76
-46
lines changed

‎src/cargo/core/package_id_spec.rs‎

Lines changed: 25 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -38,27 +38,31 @@ impl PackageIdSpec {
3838
/// use cargo::core::PackageIdSpec;
3939
///
4040
/// let specs = vec![
41+
/// "https://crates.io/foo",
4142
/// "https://crates.io/foo#1.2.3",
4243
/// "https://crates.io/foo#bar:1.2.3",
43-
/// "crates.io/foo",
44-
/// "crates.io/foo#1.2.3",
45-
/// "crates.io/foo#bar",
46-
/// "crates.io/foo#bar:1.2.3",
4744
/// "foo",
4845
/// "foo:1.2.3",
4946
/// ];
5047
/// for spec in specs {
5148
/// assert!(PackageIdSpec::parse(spec).is_ok());
5249
/// }
5350
pub fn parse(spec: &str) -> CargoResult<PackageIdSpec> {
54-
if spec.contains('/') {
51+
if spec.contains("://") {
5552
if let Ok(url) = spec.into_url() {
5653
return PackageIdSpec::from_url(url);
5754
}
58-
if !spec.contains("://") {
59-
if let Ok(url) = Url::parse(&format!("cargo://{}", spec)) {
60-
return PackageIdSpec::from_url(url);
61-
}
55+
} else if spec.contains('/') || spec.contains('\\') {
56+
let abs = std::env::current_dir().unwrap_or_default().join(spec);
57+
if abs.exists() {
58+
let maybe_url = Url::from_file_path(abs)
59+
.map_or_else(|_| "a file:// URL".to_string(), |url| url.to_string());
60+
bail!(
61+
"package ID specification `{}` looks like a file path, \
62+
maybe try {}",
63+
spec,
64+
maybe_url
65+
);
6266
}
6367
}
6468
let mut parts = spec.splitn(2, ':');
@@ -80,8 +84,11 @@ impl PackageIdSpec {
8084
where
8185
I: IntoIterator<Item = PackageId>,
8286
{
83-
let spec = PackageIdSpec::parse(spec)
84-
.chain_err(|| anyhow::format_err!("invalid package ID specification: `{}`", spec))?;
87+
let i: Vec<_> = i.into_iter().collect();
88+
let spec = PackageIdSpec::parse(spec).chain_err(|| {
89+
let suggestion = lev_distance::closest_msg(spec, i.iter(), |id| id.name().as_str());
90+
anyhow::format_err!("invalid package ID specification: `{}`{}", spec, suggestion)
91+
})?;
8592
spec.query(i)
8693
}
8794

@@ -275,11 +282,7 @@ impl fmt::Display for PackageIdSpec {
275282
let mut printed_name = false;
276283
match self.url {
277284
Some(ref url) => {
278-
if url.scheme() == "cargo" {
279-
write!(f, "{}{}", url.host().unwrap(), url.path())?;
280-
} else {
281-
write!(f, "{}", url)?;
282-
}
285+
write!(f, "{}", url)?;
283286
if url.path_segments().unwrap().next_back().unwrap() != &*self.name {
284287
printed_name = true;
285288
write!(f, "#{}", self.name)?;
@@ -333,51 +336,27 @@ mod tests {
333336
}
334337

335338
ok(
336-
"https://crates.io/foo#1.2.3",
337-
PackageIdSpec {
338-
name: InternedString::new("foo"),
339-
version: Some("1.2.3".to_semver().unwrap()),
340-
url: Some(Url::parse("https://crates.io/foo").unwrap()),
341-
},
342-
);
343-
ok(
344-
"https://crates.io/foo#bar:1.2.3",
345-
PackageIdSpec {
346-
name: InternedString::new("bar"),
347-
version: Some("1.2.3".to_semver().unwrap()),
348-
url: Some(Url::parse("https://crates.io/foo").unwrap()),
349-
},
350-
);
351-
ok(
352-
"crates.io/foo",
339+
"https://crates.io/foo",
353340
PackageIdSpec {
354341
name: InternedString::new("foo"),
355342
version: None,
356-
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
343+
url: Some(Url::parse("https://crates.io/foo").unwrap()),
357344
},
358345
);
359346
ok(
360-
"crates.io/foo#1.2.3",
347+
"https://crates.io/foo#1.2.3",
361348
PackageIdSpec {
362349
name: InternedString::new("foo"),
363350
version: Some("1.2.3".to_semver().unwrap()),
364-
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
365-
},
366-
);
367-
ok(
368-
"crates.io/foo#bar",
369-
PackageIdSpec {
370-
name: InternedString::new("bar"),
371-
version: None,
372-
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
351+
url: Some(Url::parse("https://crates.io/foo").unwrap()),
373352
},
374353
);
375354
ok(
376-
"crates.io/foo#bar:1.2.3",
355+
"https://crates.io/foo#bar:1.2.3",
377356
PackageIdSpec {
378357
name: InternedString::new("bar"),
379358
version: Some("1.2.3".to_semver().unwrap()),
380-
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
359+
url: Some(Url::parse("https://crates.io/foo").unwrap()),
381360
},
382361
);
383362
ok(

‎src/doc/man/cargo-pkgid.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,9 @@ Get the package ID for the given package instead of the current package.
8181

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

84+
4. Retrieve package specification for `foo` from a local package:
85+
86+
cargo pkgid file:///path/to/local/package#foo
87+
8488
## SEE ALSO
8589
{{man "cargo" 1}}, {{man "cargo-generate-lockfile" 1}}, {{man "cargo-metadata" 1}}

‎src/doc/man/generated_txt/cargo-pkgid.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ EXAMPLES
137137

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

140+
4. Retrieve package specification for foo from a local package:
141+
142+
cargo pkgid file:///path/to/local/package#foo
143+
140144
SEE ALSO
141145
cargo(1), cargo-generate-lockfile(1), cargo-metadata(1)
142146

‎src/doc/src/commands/cargo-pkgid.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,5 +163,9 @@ details on environment variables that Cargo reads.
163163

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

166+
4. Retrieve package specification for `foo` from a local package:
167+
168+
cargo pkgid file:///path/to/local/package#foo
169+
166170
## SEE ALSO
167171
[cargo(1)](cargo.html), [cargo-generate-lockfile(1)](cargo-generate-lockfile.html), [cargo-metadata(1)](cargo-metadata.html)

‎src/etc/man/cargo-pkgid.1‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,5 +207,15 @@ cargo pkgid https://github.com/rust\-lang/crates.io\-index#foo
207207
.fi
208208
.RE
209209
.RE
210+
.sp
211+
.RS 4
212+
\h'-04' 4.\h'+01'Retrieve package specification for \fBfoo\fR from a local package:
213+
.sp
214+
.RS 4
215+
.nf
216+
cargo pkgid file:///path/to/local/package#foo
217+
.fi
218+
.RE
219+
.RE
210220
.SH "SEE ALSO"
211221
\fBcargo\fR(1), \fBcargo\-generate\-lockfile\fR(1), \fBcargo\-metadata\fR(1)

‎tests/testsuite/pkgid.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ fn suggestion_bad_pkgid() {
5454
"#,
5555
)
5656
.file("src/lib.rs", "")
57+
.file("cratesio", "")
5758
.build();
5859

5960
p.cargo("generate-lockfile").run();
@@ -93,6 +94,34 @@ Did you mean one of these?
9394
9495
two-ver:0.1.0
9596
two-ver:0.2.0
97+
",
98+
)
99+
.run();
100+
101+
// Bad file URL.
102+
p.cargo("pkgid ./Cargo.toml")
103+
.with_status(101)
104+
.with_stderr(
105+
"\
106+
error: invalid package ID specification: `./Cargo.toml`
107+
108+
Caused by:
109+
package ID specification `./Cargo.toml` looks like a file path, maybe try file://[..]/Cargo.toml
110+
",
111+
)
112+
.run();
113+
114+
// Bad file URL with simliar name.
115+
p.cargo("pkgid './cratesio'")
116+
.with_status(101)
117+
.with_stderr(
118+
"\
119+
error: invalid package ID specification: `./cratesio`
120+
121+
<tab>Did you mean `crates-io`?
122+
123+
Caused by:
124+
package ID specification `./cratesio` looks like a file path, maybe try file://[..]/cratesio
96125
",
97126
)
98127
.run();

0 commit comments

Comments
 (0)