From 6fb8bc169f8cab621872ab090f606c92fdfe0d8e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 16:40:07 -0600 Subject: [PATCH 1/6] test(spec): Ensure we can parse what we render --- src/cargo/core/package_id_spec.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index d3f0abc465e..7b531902e5f 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -331,7 +331,10 @@ mod tests { fn ok(spec: &str, expected: PackageIdSpec, expected_rendered: &str) { let parsed = PackageIdSpec::parse(spec).unwrap(); assert_eq!(parsed, expected); - assert_eq!(parsed.to_string(), expected_rendered); + let rendered = parsed.to_string(); + assert_eq!(rendered, expected_rendered); + let reparsed = PackageIdSpec::parse(&rendered).unwrap(); + assert_eq!(reparsed, expected); } ok( From c4685c7b787bf750b3bd883034625a76acab8c45 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 16:24:37 -0600 Subject: [PATCH 2/6] test(spec): Check matches with URLs --- src/cargo/core/package_id_spec.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 7b531902e5f..07c8efc7484 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -453,6 +453,12 @@ mod tests { 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").unwrap().matches(foo)); + assert!(PackageIdSpec::parse("https://example.com#foo@1.2") + .unwrap() + .matches(foo)); + assert!(!PackageIdSpec::parse("https://bob.com#foo@1.2") + .unwrap() + .matches(foo)); let meta = PackageId::new("meta", "1.2.3+hello", sid).unwrap(); assert!(PackageIdSpec::parse("meta").unwrap().matches(meta)); From 91d6ecc6340986d3f0880e9567d0904622ac09ed Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 16:57:22 -0600 Subject: [PATCH 3/6] test(spec): Verify all examples --- src/cargo/core/package_id_spec.rs | 92 +++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 07c8efc7484..2b04ecb7678 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -427,6 +427,98 @@ mod tests { }, "foo@1.2", ); + + // pkgid-spec.md + ok( + "regex", + PackageIdSpec { + name: String::from("regex"), + version: None, + url: None, + }, + "regex", + ); + ok( + "regex@1.4", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4".parse().unwrap()), + url: None, + }, + "regex@1.4", + ); + ok( + "regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: None, + }, + "regex@1.4.3", + ); + ok( + "/~https://github.com/rust-lang/crates.io-index#regex", + PackageIdSpec { + name: String::from("regex"), + version: None, + url: Some(Url::parse("/~https://github.com/rust-lang/crates.io-index").unwrap()), + }, + "/~https://github.com/rust-lang/crates.io-index#regex", + ); + ok( + "/~https://github.com/rust-lang/crates.io-index#regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: Some(Url::parse("/~https://github.com/rust-lang/crates.io-index").unwrap()), + }, + "/~https://github.com/rust-lang/crates.io-index#regex@1.4.3", + ); + ok( + "/~https://github.com/rust-lang/cargo#0.52.0", + PackageIdSpec { + name: String::from("cargo"), + version: Some("0.52.0".parse().unwrap()), + url: Some(Url::parse("/~https://github.com/rust-lang/cargo").unwrap()), + }, + "/~https://github.com/rust-lang/cargo#0.52.0", + ); + ok( + "/~https://github.com/rust-lang/cargo#cargo-platform@0.1.2", + PackageIdSpec { + name: String::from("cargo-platform"), + version: Some("0.1.2".parse().unwrap()), + url: Some(Url::parse("/~https://github.com/rust-lang/cargo").unwrap()), + }, + "/~https://github.com/rust-lang/cargo#cargo-platform@0.1.2", + ); + ok( + "ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: Some(Url::parse("ssh://git@github.com/rust-lang/regex.git").unwrap()), + }, + "ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", + ); + ok( + "file:///path/to/my/project/foo", + PackageIdSpec { + name: String::from("foo"), + version: None, + url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()), + }, + "file:///path/to/my/project/foo", + ); + ok( + "file:///path/to/my/project/foo#1.1.8", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.1.8".parse().unwrap()), + url: Some(Url::parse("file:///path/to/my/project/foo").unwrap()), + }, + "file:///path/to/my/project/foo#1.1.8", + ); } #[test] From 55fb612e45e9679dee3b67616feed86583315799 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 15:14:35 -0600 Subject: [PATCH 4/6] refactor(spec): Make it easier to add more matches --- src/cargo/core/package_id_spec.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 2b04ecb7678..c617c1f7ab0 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -180,10 +180,13 @@ impl PackageIdSpec { } } - match self.url { - Some(ref u) => u == package_id.source_id().url(), - None => true, + if let Some(u) = &self.url { + if u != package_id.source_id().url() { + return false; + } } + + true } /// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or From e948bbb5e491cd2b34a086d79190ba49c37962e4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 15:54:32 -0600 Subject: [PATCH 5/6] refactor(source): Pull out protocol logic --- src/cargo/core/source_id.rs | 73 +++++++++++++++---------------------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs index a8c162fbf18..1e0c122849d 100644 --- a/src/cargo/core/source_id.rs +++ b/src/cargo/core/source_id.rs @@ -752,6 +752,20 @@ impl PartialEq for SourceIdInner { } } +impl SourceKind { + pub(crate) fn protocol(&self) -> Option<&str> { + match self { + SourceKind::Path => Some("path"), + SourceKind::Git(_) => Some("git"), + SourceKind::Registry => Some("registry"), + // Sparse registry URL already includes the `sparse+` prefix + SourceKind::SparseRegistry => None, + SourceKind::LocalRegistry => Some("local-registry"), + SourceKind::Directory => Some("directory"), + } + } +} + /// Forwards to `Ord` impl PartialOrd for SourceKind { fn partial_cmp(&self, other: &SourceKind) -> Option { @@ -848,53 +862,24 @@ pub struct SourceIdAsUrl<'a> { impl<'a> fmt::Display for SourceIdAsUrl<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self.inner { - SourceIdInner { - kind: SourceKind::Path, - ref url, - .. - } => write!(f, "path+{}", url), - SourceIdInner { - kind: SourceKind::Git(ref reference), - ref url, - ref precise, - .. - } => { - write!(f, "git+{}", url)?; - if let Some(pretty) = reference.pretty_ref(self.encoded) { - write!(f, "?{}", pretty)?; - } - if let Some(precise) = precise.as_ref() { - write!(f, "#{}", precise)?; - } - Ok(()) - } - SourceIdInner { - kind: SourceKind::Registry, - ref url, - .. - } => { - write!(f, "registry+{url}") + if let Some(protocol) = self.inner.kind.protocol() { + write!(f, "{protocol}+")?; + } + write!(f, "{}", self.inner.url)?; + if let SourceIdInner { + kind: SourceKind::Git(ref reference), + ref precise, + .. + } = *self.inner + { + if let Some(pretty) = reference.pretty_ref(self.encoded) { + write!(f, "?{}", pretty)?; } - SourceIdInner { - kind: SourceKind::SparseRegistry, - ref url, - .. - } => { - // Sparse registry URL already includes the `sparse+` prefix - write!(f, "{url}") + if let Some(precise) = precise.as_ref() { + write!(f, "#{}", precise)?; } - SourceIdInner { - kind: SourceKind::LocalRegistry, - ref url, - .. - } => write!(f, "local-registry+{}", url), - SourceIdInner { - kind: SourceKind::Directory, - ref url, - .. - } => write!(f, "directory+{}", url), } + Ok(()) } } From 9f511b69034e42f461fb95112d2cb68eceaf64b9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 7 Nov 2023 16:04:36 -0600 Subject: [PATCH 6/6] refactor(source): Pull out git ref logic --- src/cargo/core/source_id.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs index 1e0c122849d..e53b1704db0 100644 --- a/src/cargo/core/source_id.rs +++ b/src/cargo/core/source_id.rs @@ -188,17 +188,7 @@ impl SourceId { match kind { "git" => { let mut url = url.into_url()?; - let mut reference = GitReference::DefaultBranch; - for (k, v) in url.query_pairs() { - match &k[..] { - // Map older 'ref' to branch. - "branch" | "ref" => reference = GitReference::Branch(v.into_owned()), - - "rev" => reference = GitReference::Rev(v.into_owned()), - "tag" => reference = GitReference::Tag(v.into_owned()), - _ => {} - } - } + let reference = GitReference::from_query(url.query_pairs()); let precise = url.fragment().map(|s| s.to_owned()); url.set_fragment(None); url.set_query(None); @@ -884,6 +874,24 @@ impl<'a> fmt::Display for SourceIdAsUrl<'a> { } impl GitReference { + pub fn from_query( + query_pairs: impl Iterator, impl AsRef)>, + ) -> Self { + let mut reference = GitReference::DefaultBranch; + for (k, v) in query_pairs { + let v = v.as_ref(); + match k.as_ref() { + // Map older 'ref' to branch. + "branch" | "ref" => reference = GitReference::Branch(v.to_owned()), + + "rev" => reference = GitReference::Rev(v.to_owned()), + "tag" => reference = GitReference::Tag(v.to_owned()), + _ => {} + } + } + reference + } + /// Returns a `Display`able view of this git reference, or None if using /// the head of the default branch pub fn pretty_ref(&self, url_encoded: bool) -> Option> {