From 242d7df3c2eefbad2f85ca1fdd050e404f948ec5 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 19 Feb 2021 14:05:32 +0800 Subject: [PATCH 01/10] fix(pkgid): pkgid urls must have a host --- src/cargo/core/package_id_spec.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index daa3c73ee74..a301a5d576a 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -97,6 +97,9 @@ impl PackageIdSpec { /// Tries to convert a valid `Url` to a `PackageIdSpec`. fn from_url(mut url: Url) -> CargoResult { + if url.host().is_none() { + bail!("pkgid urls must have a host: {}", url) + } if url.query().is_some() { bail!("cannot have a query string in a pkgid: {}", url) } @@ -405,6 +408,8 @@ mod tests { 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()); + assert!(PackageIdSpec::parse("file:///baz").is_err()); + assert!(PackageIdSpec::parse("/baz").is_err()); } #[test] From cfd45dae2f6dc787f00e9e6448a7b1adc9867379 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 19 Feb 2021 14:06:31 +0800 Subject: [PATCH 02/10] fix(pkgid): fallback when `host` does not exist --- src/cargo/core/package_id_spec.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index a301a5d576a..14168968a80 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -279,7 +279,8 @@ impl fmt::Display for PackageIdSpec { match self.url { Some(ref url) => { if url.scheme() == "cargo" { - write!(f, "{}{}", url.host().unwrap(), url.path())?; + let host = url.host().unwrap_or(url::Host::Domain("")); + write!(f, "{}{}", host, url.path())?; } else { write!(f, "{}", url)?; } From 1ee261e4753ae2232b510f41edd32e54d08ef000 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 21 Feb 2021 22:33:33 +0800 Subject: [PATCH 03/10] fix(pkgid): let absolute local path fail to pase --- src/cargo/core/package_id_spec.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 14168968a80..496391a9b28 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -56,6 +56,15 @@ impl PackageIdSpec { return PackageIdSpec::from_url(url); } if !spec.contains("://") { + if spec.starts_with('/') { + // This is out of current pkgid spec. + // Only for fixing rust-lang/cargo#9041 + bail!( + "pkgid urls with local paths must be prefixed \ + with a `file://` scheme: {}", + spec + ) + } if let Ok(url) = Url::parse(&format!("cargo://{}", spec)) { return PackageIdSpec::from_url(url); } @@ -97,9 +106,6 @@ impl PackageIdSpec { /// Tries to convert a valid `Url` to a `PackageIdSpec`. fn from_url(mut url: Url) -> CargoResult { - if url.host().is_none() { - bail!("pkgid urls must have a host: {}", url) - } if url.query().is_some() { bail!("cannot have a query string in a pkgid: {}", url) } @@ -279,8 +285,7 @@ impl fmt::Display for PackageIdSpec { match self.url { Some(ref url) => { if url.scheme() == "cargo" { - let host = url.host().unwrap_or(url::Host::Domain("")); - write!(f, "{}{}", host, url.path())?; + write!(f, "{}{}", url.host().unwrap(), url.path())?; } else { write!(f, "{}", url)?; } @@ -409,7 +414,6 @@ mod tests { 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()); - assert!(PackageIdSpec::parse("file:///baz").is_err()); assert!(PackageIdSpec::parse("/baz").is_err()); } From 1bbc0569b9d2f16b6dda66ab00fe766b628fa99b Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 08:40:22 +0800 Subject: [PATCH 04/10] fix(pkgid): drop cargo:// support --- src/cargo/core/package_id_spec.rs | 69 +++++++++---------------------- 1 file changed, 19 insertions(+), 50 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 496391a9b28..3a220aa58a6 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -51,23 +51,21 @@ impl PackageIdSpec { /// assert!(PackageIdSpec::parse(spec).is_ok()); /// } pub fn parse(spec: &str) -> CargoResult { - if spec.contains('/') { + if spec.contains("://") { if let Ok(url) = spec.into_url() { return PackageIdSpec::from_url(url); } - if !spec.contains("://") { - if spec.starts_with('/') { - // This is out of current pkgid spec. - // Only for fixing rust-lang/cargo#9041 - bail!( - "pkgid urls with local paths must be prefixed \ - with a `file://` scheme: {}", - spec - ) - } - 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, ':'); @@ -284,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)?; @@ -342,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( @@ -414,7 +384,6 @@ mod tests { 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()); - assert!(PackageIdSpec::parse("/baz").is_err()); } #[test] From 2e3a74cc953bd397b19706be3ffb798ef38975da Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 08:40:59 +0800 Subject: [PATCH 05/10] test(pkgid): bad file URL suggestion --- tests/testsuite/pkgid.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 93ee9eeddfb..d3244d0aff8 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -93,6 +93,19 @@ 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(); From 1667b99fee09eedcf0785080bd4379ef47436518 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 10:46:41 +0800 Subject: [PATCH 06/10] doc(pkgid): example for local package spec retrival --- src/doc/man/cargo-pkgid.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/doc/man/cargo-pkgid.md b/src/doc/man/cargo-pkgid.md index 12f8cacc8c4..1fb9a60efc7 100644 --- a/src/doc/man/cargo-pkgid.md +++ b/src/doc/man/cargo-pkgid.md @@ -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}} From 36300520ebb95dba46479f113c181efe5e66b5bf Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 10:48:55 +0800 Subject: [PATCH 07/10] doc: build from build-man.sh --- src/doc/man/generated_txt/cargo-pkgid.txt | 4 ++++ src/doc/src/commands/cargo-pkgid.md | 4 ++++ src/etc/man/cargo-pkgid.1 | 10 ++++++++++ 3 files changed, 18 insertions(+) diff --git a/src/doc/man/generated_txt/cargo-pkgid.txt b/src/doc/man/generated_txt/cargo-pkgid.txt index eab8edebbea..cee20fc122b 100644 --- a/src/doc/man/generated_txt/cargo-pkgid.txt +++ b/src/doc/man/generated_txt/cargo-pkgid.txt @@ -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) diff --git a/src/doc/src/commands/cargo-pkgid.md b/src/doc/src/commands/cargo-pkgid.md index e899d9c9d84..c380738c943 100644 --- a/src/doc/src/commands/cargo-pkgid.md +++ b/src/doc/src/commands/cargo-pkgid.md @@ -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) diff --git a/src/etc/man/cargo-pkgid.1 b/src/etc/man/cargo-pkgid.1 index 27b3cd71a8c..4ee1046dd43 100644 --- a/src/etc/man/cargo-pkgid.1 +++ b/src/etc/man/cargo-pkgid.1 @@ -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) From a22b3ac207bdfb83f8623f19d8eb0496ee8e592a Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 16:17:57 +0800 Subject: [PATCH 08/10] feat(pkgid): suggestion for bad parsing --- src/cargo/core/package_id_spec.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 3a220aa58a6..304596a9210 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -87,8 +87,11 @@ impl PackageIdSpec { where I: IntoIterator, { - 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) } From ade33c20d1ed34e408ae33e27b300b049c3ad6f6 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 16:18:21 +0800 Subject: [PATCH 09/10] teset(pkgid): suggestion for bad parsing --- tests/testsuite/pkgid.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index d3244d0aff8..5cd4cd41b17 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -54,6 +54,7 @@ fn suggestion_bad_pkgid() { "#, ) .file("src/lib.rs", "") + .file("cratesio", "") .build(); p.cargo("generate-lockfile").run(); @@ -106,6 +107,21 @@ 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` + +Did you mean `crates-io`? + +Caused by: + package ID specification `./cratesio` looks like a file path, maybe try file://[..]/cratesio ", ) .run(); From c5d23046299e7087cce76a73ca397d4474b12893 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 2 Mar 2021 16:40:45 +0800 Subject: [PATCH 10/10] test(pkgid): scheme must be explicit named --- src/cargo/core/package_id_spec.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 304596a9210..6049e981fa9 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -38,12 +38,9 @@ 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", /// ];