From d7bcc0cd26983b00daf7de5f00e175a4dc4ef0b6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Dec 2023 12:49:07 -0600 Subject: [PATCH 1/5] refactor(spec): Move queries to a extension trait This is a step towards moving `PackageIdSpec` into `schema` as manifests need it as part of #12801. While I didn't take this approach in #13080, that was largely how core these functions are / how pervasive their use is. --- src/bin/cargo/commands/remove.rs | 1 + src/cargo/core/mod.rs | 2 +- src/cargo/core/package_id_spec.rs | 161 +++++++++++++---------- src/cargo/core/profiles.rs | 4 +- src/cargo/core/resolver/dep_cache.rs | 4 +- src/cargo/core/resolver/resolve.rs | 2 +- src/cargo/core/workspace.rs | 4 +- src/cargo/ops/cargo_clean.rs | 2 +- src/cargo/ops/cargo_generate_lockfile.rs | 2 +- src/cargo/ops/cargo_pkgid.rs | 2 +- src/cargo/ops/cargo_uninstall.rs | 2 +- src/cargo/ops/registry/publish.rs | 1 + src/cargo/ops/resolve.rs | 4 +- src/cargo/ops/tree/mod.rs | 2 +- 14 files changed, 109 insertions(+), 84 deletions(-) diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index 8344d38d236..b7abb171510 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -1,5 +1,6 @@ use cargo::core::dependency::DepKind; use cargo::core::PackageIdSpec; +use cargo::core::PackageIdSpecQuery; use cargo::core::Resolve; use cargo::core::Workspace; use cargo::ops::cargo_remove::remove; diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index eea910b6651..41a5a315162 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -4,7 +4,7 @@ pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::manifest::{Manifest, Target, TargetKind}; pub use self::package::{Package, PackageSet}; pub use self::package_id::PackageId; -pub use self::package_id_spec::PackageIdSpec; +pub use self::package_id_spec::{PackageIdSpec, PackageIdSpecQuery}; pub use self::registry::Registry; pub use self::resolver::{Resolve, ResolveVersion}; pub use self::shell::{Shell, Verbosity}; diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index b25cb826c5f..ca4b9427af0 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -85,19 +85,6 @@ impl PackageIdSpec { }) } - /// Roughly equivalent to `PackageIdSpec::parse(spec)?.query(i)` - pub fn query_str(spec: &str, i: I) -> CargoResult - where - I: IntoIterator, - { - let i: Vec<_> = i.into_iter().collect(); - let spec = PackageIdSpec::parse(spec).with_context(|| { - let suggestion = edit_distance::closest_msg(spec, i.iter(), |id| id.name().as_str()); - format!("invalid package ID specification: `{}`{}", spec, suggestion) - })?; - spec.query(i) - } - /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` /// fields filled in. pub fn from_package_id(package_id: PackageId) -> PackageIdSpec { @@ -221,9 +208,94 @@ impl PackageIdSpec { pub fn set_kind(&mut self, kind: SourceKind) { self.kind = Some(kind); } +} + +fn strip_url_protocol(url: &Url) -> Url { + // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https + let raw = url.to_string(); + raw.split_once('+').unwrap().1.parse().unwrap() +} + +impl fmt::Display for PackageIdSpec { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut printed_name = false; + match self.url { + Some(ref url) => { + if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { + write!(f, "{protocol}+")?; + } + write!(f, "{}", url)?; + if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { + if let Some(pretty) = git_ref.pretty_ref(true) { + write!(f, "?{}", pretty)?; + } + } + if url.path_segments().unwrap().next_back().unwrap() != &*self.name { + printed_name = true; + write!(f, "#{}", self.name)?; + } + } + None => { + printed_name = true; + write!(f, "{}", self.name)?; + } + } + if let Some(ref v) = self.version { + write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; + } + Ok(()) + } +} + +impl ser::Serialize for PackageIdSpec { + fn serialize(&self, s: S) -> Result + where + S: ser::Serializer, + { + self.to_string().serialize(s) + } +} + +impl<'de> de::Deserialize<'de> for PackageIdSpec { + fn deserialize(d: D) -> Result + where + D: de::Deserializer<'de>, + { + let string = String::deserialize(d)?; + PackageIdSpec::parse(&string).map_err(de::Error::custom) + } +} + +pub trait PackageIdSpecQuery { + /// Roughly equivalent to `PackageIdSpec::parse(spec)?.query(i)` + fn query_str(spec: &str, i: I) -> CargoResult + where + I: IntoIterator; /// Checks whether the given `PackageId` matches the `PackageIdSpec`. - pub fn matches(&self, package_id: PackageId) -> bool { + fn matches(&self, package_id: PackageId) -> bool; + + /// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or + /// more are found, then this returns an error. + fn query(&self, i: I) -> CargoResult + where + I: IntoIterator; +} + +impl PackageIdSpecQuery for PackageIdSpec { + fn query_str(spec: &str, i: I) -> CargoResult + where + I: IntoIterator, + { + let i: Vec<_> = i.into_iter().collect(); + let spec = PackageIdSpec::parse(spec).with_context(|| { + let suggestion = edit_distance::closest_msg(spec, i.iter(), |id| id.name().as_str()); + format!("invalid package ID specification: `{}`{}", spec, suggestion) + })?; + spec.query(i) + } + + fn matches(&self, package_id: PackageId) -> bool { if self.name() != package_id.name().as_str() { return false; } @@ -249,9 +321,7 @@ impl PackageIdSpec { true } - /// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or - /// more are found, then this returns an error. - pub fn query(&self, i: I) -> CargoResult + fn query(&self, i: I) -> CargoResult where I: IntoIterator, { @@ -342,65 +412,10 @@ impl PackageIdSpec { } } -fn strip_url_protocol(url: &Url) -> Url { - // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https - let raw = url.to_string(); - raw.split_once('+').unwrap().1.parse().unwrap() -} - -impl fmt::Display for PackageIdSpec { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut printed_name = false; - match self.url { - Some(ref url) => { - if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { - write!(f, "{protocol}+")?; - } - write!(f, "{}", url)?; - if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { - if let Some(pretty) = git_ref.pretty_ref(true) { - write!(f, "?{}", pretty)?; - } - } - if url.path_segments().unwrap().next_back().unwrap() != &*self.name { - printed_name = true; - write!(f, "#{}", self.name)?; - } - } - None => { - printed_name = true; - write!(f, "{}", self.name)?; - } - } - if let Some(ref v) = self.version { - write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; - } - Ok(()) - } -} - -impl ser::Serialize for PackageIdSpec { - fn serialize(&self, s: S) -> Result - where - S: ser::Serializer, - { - self.to_string().serialize(s) - } -} - -impl<'de> de::Deserialize<'de> for PackageIdSpec { - fn deserialize(d: D) -> Result - where - D: de::Deserializer<'de>, - { - let string = String::deserialize(d)?; - PackageIdSpec::parse(&string).map_err(de::Error::custom) - } -} - #[cfg(test)] mod tests { use super::PackageIdSpec; + use super::PackageIdSpecQuery; use crate::core::{GitReference, PackageId, SourceId, SourceKind}; use url::Url; diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 34365008ee5..4d2a23f5060 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -25,7 +25,9 @@ use crate::core::compiler::{CompileKind, CompileTarget, Unit}; use crate::core::dependency::Artifact; use crate::core::resolver::features::FeaturesFor; use crate::core::Feature; -use crate::core::{PackageId, PackageIdSpec, Resolve, Shell, Target, Workspace}; +use crate::core::{ + PackageId, PackageIdSpec, PackageIdSpecQuery, Resolve, Shell, Target, Workspace, +}; use crate::util::interning::InternedString; use crate::util::toml::validate_profile; use crate::util::{closest_msg, config, CargoResult, Config}; diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 00a269482b7..9e8ffd3510c 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -16,7 +16,9 @@ use crate::core::resolver::{ ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, VersionOrdering, VersionPreferences, }; -use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary}; +use crate::core::{ + Dependency, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, Registry, Summary, +}; use crate::sources::source::QueryKind; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index b401e923275..02f112166dc 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -1,6 +1,6 @@ use super::encode::Metadata; use crate::core::dependency::DepKind; -use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target}; +use crate::core::{Dependency, PackageId, PackageIdSpec, PackageIdSpecQuery, Summary, Target}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::Graph; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 3467fe18ee8..a47d994cff1 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -15,7 +15,9 @@ use crate::core::features::Features; use crate::core::registry::PackageRegistry; use crate::core::resolver::features::CliFeatures; use crate::core::resolver::ResolveBehavior; -use crate::core::{Dependency, Edition, FeatureValue, PackageId, PackageIdSpec}; +use crate::core::{ + Dependency, Edition, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, +}; use crate::core::{EitherManifest, Package, SourceId, VirtualManifest}; use crate::ops; use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 923b2decdc4..4add5d86326 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -1,6 +1,6 @@ use crate::core::compiler::{CompileKind, CompileMode, Layout, RustcTargetData}; use crate::core::profiles::Profiles; -use crate::core::{PackageIdSpec, TargetKind, Workspace}; +use crate::core::{PackageIdSpec, PackageIdSpecQuery, TargetKind, Workspace}; use crate::ops; use crate::util::edit_distance; use crate::util::errors::CargoResult; diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 03c38630e92..1bba64925ec 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -1,6 +1,6 @@ use crate::core::registry::PackageRegistry; use crate::core::resolver::features::{CliFeatures, HasDevUnits}; -use crate::core::{PackageId, PackageIdSpec}; +use crate::core::{PackageId, PackageIdSpec, PackageIdSpecQuery}; use crate::core::{Resolve, SourceId, Workspace}; use crate::ops; use crate::util::cache_lock::CacheLockMode; diff --git a/src/cargo/ops/cargo_pkgid.rs b/src/cargo/ops/cargo_pkgid.rs index bbae154a736..2324e25b44e 100644 --- a/src/cargo/ops/cargo_pkgid.rs +++ b/src/cargo/ops/cargo_pkgid.rs @@ -1,4 +1,4 @@ -use crate::core::{PackageIdSpec, Workspace}; +use crate::core::{PackageIdSpec, PackageIdSpecQuery, Workspace}; use crate::ops; use crate::util::CargoResult; diff --git a/src/cargo/ops/cargo_uninstall.rs b/src/cargo/ops/cargo_uninstall.rs index 7b9fdccd4cb..7b45a69b4a6 100644 --- a/src/cargo/ops/cargo_uninstall.rs +++ b/src/cargo/ops/cargo_uninstall.rs @@ -1,5 +1,5 @@ use crate::core::PackageId; -use crate::core::{PackageIdSpec, SourceId}; +use crate::core::{PackageIdSpec, PackageIdSpecQuery, SourceId}; use crate::ops::common_for_install_and_uninstall::*; use crate::sources::PathSource; use crate::util::errors::CargoResult; diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 201907bb268..2313792c8cd 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -21,6 +21,7 @@ use crate::core::manifest::ManifestMetadata; use crate::core::resolver::CliFeatures; use crate::core::Dependency; use crate::core::Package; +use crate::core::PackageIdSpecQuery; use crate::core::SourceId; use crate::core::Workspace; use crate::ops; diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index c3ae6b2def8..06716a2b4cf 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -64,7 +64,9 @@ use crate::core::resolver::{ self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion, VersionOrdering, VersionPreferences, }; use crate::core::summary::Summary; -use crate::core::{GitReference, PackageId, PackageIdSpec, PackageSet, SourceId, Workspace}; +use crate::core::{ + GitReference, PackageId, PackageIdSpec, PackageIdSpecQuery, PackageSet, SourceId, Workspace, +}; use crate::ops; use crate::sources::PathSource; use crate::util::cache_lock::CacheLockMode; diff --git a/src/cargo/ops/tree/mod.rs b/src/cargo/ops/tree/mod.rs index ce3bae8ccd0..6928ec5f94b 100644 --- a/src/cargo/ops/tree/mod.rs +++ b/src/cargo/ops/tree/mod.rs @@ -4,7 +4,7 @@ use self::format::Pattern; use crate::core::compiler::{CompileKind, RustcTargetData}; use crate::core::dependency::DepKind; use crate::core::resolver::{features::CliFeatures, ForceAllTargets, HasDevUnits}; -use crate::core::{Package, PackageId, PackageIdSpec, Workspace}; +use crate::core::{Package, PackageId, PackageIdSpec, PackageIdSpecQuery, Workspace}; use crate::ops::{self, Packages}; use crate::util::{CargoResult, Config}; use crate::{drop_print, drop_println}; From f3999d544f271b49d7db08ac97e3577386d92417 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Dec 2023 15:31:07 -0600 Subject: [PATCH 2/5] refactor(schema): Pull SourceKind out into schemas --- src/cargo/core/mod.rs | 3 +- src/cargo/core/source_id.rs | 203 +-------------------- src/cargo/util_schemas/core/mod.rs | 4 + src/cargo/util_schemas/core/source_kind.rs | 201 ++++++++++++++++++++ src/cargo/util_schemas/mod.rs | 1 + 5 files changed, 210 insertions(+), 202 deletions(-) create mode 100644 src/cargo/util_schemas/core/mod.rs create mode 100644 src/cargo/util_schemas/core/source_kind.rs diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 41a5a315162..85b961de6d9 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -8,12 +8,13 @@ pub use self::package_id_spec::{PackageIdSpec, PackageIdSpecQuery}; pub use self::registry::Registry; pub use self::resolver::{Resolve, ResolveVersion}; pub use self::shell::{Shell, Verbosity}; -pub use self::source_id::{GitReference, SourceId, SourceKind}; +pub use self::source_id::SourceId; pub use self::summary::{FeatureMap, FeatureValue, Summary}; pub use self::workspace::{ find_workspace_root, resolve_relative_path, MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig, }; +pub use crate::util_schemas::core::{GitReference, SourceKind}; pub mod compiler; pub mod dependency; diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs index cde204377b0..3b1cad94211 100644 --- a/src/cargo/core/source_id.rs +++ b/src/cargo/core/source_id.rs @@ -1,4 +1,6 @@ +use crate::core::GitReference; use crate::core::PackageId; +use crate::core::SourceKind; use crate::sources::registry::CRATES_IO_HTTP_INDEX; use crate::sources::source::Source; use crate::sources::{DirectorySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; @@ -82,38 +84,6 @@ impl fmt::Display for Precise { } } -/// The possible kinds of code source. -/// Along with [`SourceIdInner`], this fully defines the source. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum SourceKind { - /// A git repository. - Git(GitReference), - /// A local path. - Path, - /// A remote registry. - Registry, - /// A sparse registry. - SparseRegistry, - /// A local filesystem-based registry. - LocalRegistry, - /// A directory-based registry. - Directory, -} - -/// Information to find a specific commit in a Git repository. -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub enum GitReference { - /// From a tag. - Tag(String), - /// From a branch. - Branch(String), - /// From a specific revision. Can be a commit hash (either short or full), - /// or a named reference like `refs/pull/493/head`. - Rev(String), - /// The default branch of the repository, the reference named `HEAD`. - DefaultBranch, -} - /// Where the remote source key is defined. /// /// The purpose of this is to provide better diagnostics for different sources of keys. @@ -746,108 +716,6 @@ 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, see `SourceId::new` - 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 { - Some(self.cmp(other)) - } -} - -/// Note that this is specifically not derived on `SourceKind` although the -/// implementation here is very similar to what it might look like if it were -/// otherwise derived. -/// -/// The reason for this is somewhat obtuse. First of all the hash value of -/// `SourceKind` makes its way into `~/.cargo/registry/index/github.com-XXXX` -/// which means that changes to the hash means that all Rust users need to -/// redownload the crates.io index and all their crates. If possible we strive -/// to not change this to make this redownloading behavior happen as little as -/// possible. How is this connected to `Ord` you might ask? That's a good -/// question! -/// -/// Since the beginning of time `SourceKind` has had `#[derive(Hash)]`. It for -/// the longest time *also* derived the `Ord` and `PartialOrd` traits. In #8522, -/// however, the implementation of `Ord` changed. This handwritten implementation -/// forgot to sync itself with the originally derived implementation, namely -/// placing git dependencies as sorted after all other dependencies instead of -/// first as before. -/// -/// This regression in #8522 (Rust 1.47) went unnoticed. When we switched back -/// to a derived implementation in #9133 (Rust 1.52 beta) we only then ironically -/// saw an issue (#9334). In #9334 it was observed that stable Rust at the time -/// (1.51) was sorting git dependencies last, whereas Rust 1.52 beta would sort -/// git dependencies first. This is because the `PartialOrd` implementation in -/// 1.51 used #8522, the buggy implementation, which put git deps last. In 1.52 -/// it was (unknowingly) restored to the pre-1.47 behavior with git dependencies -/// first. -/// -/// Because the breakage was only witnessed after the original breakage, this -/// trait implementation is preserving the "broken" behavior. Put a different way: -/// -/// * Rust pre-1.47 sorted git deps first. -/// * Rust 1.47 to Rust 1.51 sorted git deps last, a breaking change (#8522) that -/// was never noticed. -/// * Rust 1.52 restored the pre-1.47 behavior (#9133, without knowing it did -/// so), and breakage was witnessed by actual users due to difference with -/// 1.51. -/// * Rust 1.52 (the source as it lives now) was fixed to match the 1.47-1.51 -/// behavior (#9383), which is now considered intentionally breaking from the -/// pre-1.47 behavior. -/// -/// Note that this was all discovered when Rust 1.53 was in nightly and 1.52 was -/// in beta. #9133 was in both beta and nightly at the time of discovery. For -/// 1.52 #9383 reverted #9133, meaning 1.52 is the same as 1.51. On nightly -/// (1.53) #9397 was created to fix the regression introduced by #9133 relative -/// to the current stable (1.51). -/// -/// That's all a long winded way of saying "it's weird that git deps hash first -/// and are sorted last, but it's the way it is right now". The author of this -/// comment chose to handwrite the `Ord` implementation instead of the `Hash` -/// implementation, but it's only required that at most one of them is -/// hand-written because the other can be derived. Perhaps one day in -/// the future someone can figure out how to remove this behavior. -impl Ord for SourceKind { - fn cmp(&self, other: &SourceKind) -> Ordering { - match (self, other) { - (SourceKind::Path, SourceKind::Path) => Ordering::Equal, - (SourceKind::Path, _) => Ordering::Less, - (_, SourceKind::Path) => Ordering::Greater, - - (SourceKind::Registry, SourceKind::Registry) => Ordering::Equal, - (SourceKind::Registry, _) => Ordering::Less, - (_, SourceKind::Registry) => Ordering::Greater, - - (SourceKind::SparseRegistry, SourceKind::SparseRegistry) => Ordering::Equal, - (SourceKind::SparseRegistry, _) => Ordering::Less, - (_, SourceKind::SparseRegistry) => Ordering::Greater, - - (SourceKind::LocalRegistry, SourceKind::LocalRegistry) => Ordering::Equal, - (SourceKind::LocalRegistry, _) => Ordering::Less, - (_, SourceKind::LocalRegistry) => Ordering::Greater, - - (SourceKind::Directory, SourceKind::Directory) => Ordering::Equal, - (SourceKind::Directory, _) => Ordering::Less, - (_, SourceKind::Directory) => Ordering::Greater, - - (SourceKind::Git(a), SourceKind::Git(b)) => a.cmp(b), - } - } -} - /// A `Display`able view into a `SourceId` that will write it as a url pub struct SourceIdAsUrl<'a> { inner: &'a SourceIdInner, @@ -877,73 +745,6 @@ 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> { - match self { - GitReference::DefaultBranch => None, - _ => Some(PrettyRef { - inner: self, - url_encoded, - }), - } - } -} - -/// A git reference that can be `Display`ed -pub struct PrettyRef<'a> { - inner: &'a GitReference, - url_encoded: bool, -} - -impl<'a> fmt::Display for PrettyRef<'a> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let value: &str; - match self.inner { - GitReference::Branch(s) => { - write!(f, "branch=")?; - value = s; - } - GitReference::Tag(s) => { - write!(f, "tag=")?; - value = s; - } - GitReference::Rev(s) => { - write!(f, "rev=")?; - value = s; - } - GitReference::DefaultBranch => unreachable!(), - } - if self.url_encoded { - for value in url::form_urlencoded::byte_serialize(value.as_bytes()) { - write!(f, "{value}")?; - } - } else { - write!(f, "{value}")?; - } - Ok(()) - } -} - impl KeyOf { /// Gets the underlying key. fn key(&self) -> &str { diff --git a/src/cargo/util_schemas/core/mod.rs b/src/cargo/util_schemas/core/mod.rs new file mode 100644 index 00000000000..cb959a15759 --- /dev/null +++ b/src/cargo/util_schemas/core/mod.rs @@ -0,0 +1,4 @@ +mod source_kind; + +pub use source_kind::GitReference; +pub use source_kind::SourceKind; diff --git a/src/cargo/util_schemas/core/source_kind.rs b/src/cargo/util_schemas/core/source_kind.rs new file mode 100644 index 00000000000..7b2ecaeec8c --- /dev/null +++ b/src/cargo/util_schemas/core/source_kind.rs @@ -0,0 +1,201 @@ +use std::cmp::Ordering; + +/// The possible kinds of code source. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum SourceKind { + /// A git repository. + Git(GitReference), + /// A local path. + Path, + /// A remote registry. + Registry, + /// A sparse registry. + SparseRegistry, + /// A local filesystem-based registry. + LocalRegistry, + /// A directory-based registry. + Directory, +} + +impl SourceKind { + pub 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, see `SourceId::new` + SourceKind::SparseRegistry => None, + SourceKind::LocalRegistry => Some("local-registry"), + SourceKind::Directory => Some("directory"), + } + } +} + +/// Note that this is specifically not derived on `SourceKind` although the +/// implementation here is very similar to what it might look like if it were +/// otherwise derived. +/// +/// The reason for this is somewhat obtuse. First of all the hash value of +/// `SourceKind` makes its way into `~/.cargo/registry/index/github.com-XXXX` +/// which means that changes to the hash means that all Rust users need to +/// redownload the crates.io index and all their crates. If possible we strive +/// to not change this to make this redownloading behavior happen as little as +/// possible. How is this connected to `Ord` you might ask? That's a good +/// question! +/// +/// Since the beginning of time `SourceKind` has had `#[derive(Hash)]`. It for +/// the longest time *also* derived the `Ord` and `PartialOrd` traits. In #8522, +/// however, the implementation of `Ord` changed. This handwritten implementation +/// forgot to sync itself with the originally derived implementation, namely +/// placing git dependencies as sorted after all other dependencies instead of +/// first as before. +/// +/// This regression in #8522 (Rust 1.47) went unnoticed. When we switched back +/// to a derived implementation in #9133 (Rust 1.52 beta) we only then ironically +/// saw an issue (#9334). In #9334 it was observed that stable Rust at the time +/// (1.51) was sorting git dependencies last, whereas Rust 1.52 beta would sort +/// git dependencies first. This is because the `PartialOrd` implementation in +/// 1.51 used #8522, the buggy implementation, which put git deps last. In 1.52 +/// it was (unknowingly) restored to the pre-1.47 behavior with git dependencies +/// first. +/// +/// Because the breakage was only witnessed after the original breakage, this +/// trait implementation is preserving the "broken" behavior. Put a different way: +/// +/// * Rust pre-1.47 sorted git deps first. +/// * Rust 1.47 to Rust 1.51 sorted git deps last, a breaking change (#8522) that +/// was never noticed. +/// * Rust 1.52 restored the pre-1.47 behavior (#9133, without knowing it did +/// so), and breakage was witnessed by actual users due to difference with +/// 1.51. +/// * Rust 1.52 (the source as it lives now) was fixed to match the 1.47-1.51 +/// behavior (#9383), which is now considered intentionally breaking from the +/// pre-1.47 behavior. +/// +/// Note that this was all discovered when Rust 1.53 was in nightly and 1.52 was +/// in beta. #9133 was in both beta and nightly at the time of discovery. For +/// 1.52 #9383 reverted #9133, meaning 1.52 is the same as 1.51. On nightly +/// (1.53) #9397 was created to fix the regression introduced by #9133 relative +/// to the current stable (1.51). +/// +/// That's all a long winded way of saying "it's weird that git deps hash first +/// and are sorted last, but it's the way it is right now". The author of this +/// comment chose to handwrite the `Ord` implementation instead of the `Hash` +/// implementation, but it's only required that at most one of them is +/// hand-written because the other can be derived. Perhaps one day in +/// the future someone can figure out how to remove this behavior. +impl Ord for SourceKind { + fn cmp(&self, other: &SourceKind) -> Ordering { + match (self, other) { + (SourceKind::Path, SourceKind::Path) => Ordering::Equal, + (SourceKind::Path, _) => Ordering::Less, + (_, SourceKind::Path) => Ordering::Greater, + + (SourceKind::Registry, SourceKind::Registry) => Ordering::Equal, + (SourceKind::Registry, _) => Ordering::Less, + (_, SourceKind::Registry) => Ordering::Greater, + + (SourceKind::SparseRegistry, SourceKind::SparseRegistry) => Ordering::Equal, + (SourceKind::SparseRegistry, _) => Ordering::Less, + (_, SourceKind::SparseRegistry) => Ordering::Greater, + + (SourceKind::LocalRegistry, SourceKind::LocalRegistry) => Ordering::Equal, + (SourceKind::LocalRegistry, _) => Ordering::Less, + (_, SourceKind::LocalRegistry) => Ordering::Greater, + + (SourceKind::Directory, SourceKind::Directory) => Ordering::Equal, + (SourceKind::Directory, _) => Ordering::Less, + (_, SourceKind::Directory) => Ordering::Greater, + + (SourceKind::Git(a), SourceKind::Git(b)) => a.cmp(b), + } + } +} + +/// Forwards to `Ord` +impl PartialOrd for SourceKind { + fn partial_cmp(&self, other: &SourceKind) -> Option { + Some(self.cmp(other)) + } +} + +/// Information to find a specific commit in a Git repository. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum GitReference { + /// From a tag. + Tag(String), + /// From a branch. + Branch(String), + /// From a specific revision. Can be a commit hash (either short or full), + /// or a named reference like `refs/pull/493/head`. + Rev(String), + /// The default branch of the repository, the reference named `HEAD`. + DefaultBranch, +} + +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> { + match self { + GitReference::DefaultBranch => None, + _ => Some(PrettyRef { + inner: self, + url_encoded, + }), + } + } +} + +/// A git reference that can be `Display`ed +pub struct PrettyRef<'a> { + inner: &'a GitReference, + url_encoded: bool, +} + +impl<'a> std::fmt::Display for PrettyRef<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let value: &str; + match self.inner { + GitReference::Branch(s) => { + write!(f, "branch=")?; + value = s; + } + GitReference::Tag(s) => { + write!(f, "tag=")?; + value = s; + } + GitReference::Rev(s) => { + write!(f, "rev=")?; + value = s; + } + GitReference::DefaultBranch => unreachable!(), + } + if self.url_encoded { + for value in url::form_urlencoded::byte_serialize(value.as_bytes()) { + write!(f, "{value}")?; + } + } else { + write!(f, "{value}")?; + } + Ok(()) + } +} diff --git a/src/cargo/util_schemas/mod.rs b/src/cargo/util_schemas/mod.rs index dd0e15b0af4..a2d0a0736a8 100644 --- a/src/cargo/util_schemas/mod.rs +++ b/src/cargo/util_schemas/mod.rs @@ -5,4 +5,5 @@ //! Any logic for getting final semantics from these will likely need other tools to process, like //! `cargo metadata`. +pub mod core; pub mod manifest; From 294c3851e9ccaeb2084c0163a2477131df8f5e76 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Dec 2023 15:56:08 -0600 Subject: [PATCH 3/5] refactor: Pull out constructor for PackageIdSpec --- src/cargo/core/package_id_spec.rs | 60 ++++++++++++++++++------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index ca4b9427af0..8cfbace8772 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -32,6 +32,30 @@ pub struct PackageIdSpec { } impl PackageIdSpec { + pub fn new(name: String) -> Self { + Self { + name, + version: None, + url: None, + kind: None, + } + } + + pub fn with_version(mut self, version: PartialVersion) -> Self { + self.version = Some(version); + self + } + + pub fn with_url(mut self, url: Url) -> Self { + self.url = Some(url); + self + } + + pub fn with_kind(mut self, kind: SourceKind) -> Self { + self.kind = Some(kind); + self + } + /// Parses a spec string and returns a `PackageIdSpec` if the string was valid. /// /// # Examples @@ -88,12 +112,10 @@ impl PackageIdSpec { /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` /// fields filled in. pub fn from_package_id(package_id: PackageId) -> PackageIdSpec { - PackageIdSpec { - name: String::from(package_id.name().as_str()), - version: Some(package_id.version().clone().into()), - url: Some(package_id.source_id().url().clone()), - kind: Some(package_id.source_id().kind().clone()), - } + PackageIdSpec::new(String::from(package_id.name().as_str())) + .with_version(package_id.version().clone().into()) + .with_url(package_id.source_id().url().clone()) + .with_kind(package_id.source_id().kind().clone()) } /// Tries to convert a valid `Url` to a `PackageIdSpec`. @@ -341,26 +363,16 @@ impl PackageIdSpecQuery for PackageIdSpec { } }; if self.url.is_some() { - try_spec( - PackageIdSpec { - name: self.name.clone(), - version: self.version.clone(), - url: None, - kind: None, - }, - &mut suggestion, - ); + let spec = PackageIdSpec::new(self.name.clone()); + let spec = if let Some(version) = self.version.clone() { + spec.with_version(version) + } else { + spec + }; + try_spec(spec, &mut suggestion); } if suggestion.is_empty() && self.version.is_some() { - try_spec( - PackageIdSpec { - name: self.name.clone(), - version: None, - url: None, - kind: None, - }, - &mut suggestion, - ); + try_spec(PackageIdSpec::new(self.name.clone()), &mut suggestion); } if suggestion.is_empty() { suggestion.push_str(&edit_distance::closest_msg( From 33d1d78a72b697e5eac7ff139503e39c8b6258fe Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Dec 2023 16:09:34 -0600 Subject: [PATCH 4/5] refactor: Move Id to Spec converter --- src/cargo/core/package_id.rs | 10 ++++++++++ src/cargo/core/package_id_spec.rs | 11 +---------- src/cargo/ops/cargo_compile/packages.rs | 10 +++++----- src/cargo/ops/cargo_install.rs | 6 ++---- src/cargo/ops/cargo_pkgid.rs | 2 +- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index f8859a1ce5b..37b367218f8 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -10,6 +10,7 @@ use std::sync::OnceLock; use serde::de; use serde::ser; +use crate::core::PackageIdSpec; use crate::core::SourceId; use crate::util::interning::InternedString; use crate::util::CargoResult; @@ -186,6 +187,15 @@ impl PackageId { pub fn tarball_name(&self) -> String { format!("{}-{}.crate", self.name(), self.version()) } + + /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` + /// fields filled in. + pub fn to_spec(&self) -> PackageIdSpec { + PackageIdSpec::new(String::from(self.name().as_str())) + .with_version(self.version().clone().into()) + .with_url(self.source_id().url().clone()) + .with_kind(self.source_id().kind().clone()) + } } pub struct PackageIdStableHash<'a>(PackageId, &'a Path); diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 8cfbace8772..cd3947211d9 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -109,15 +109,6 @@ impl PackageIdSpec { }) } - /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` - /// fields filled in. - pub fn from_package_id(package_id: PackageId) -> PackageIdSpec { - PackageIdSpec::new(String::from(package_id.name().as_str())) - .with_version(package_id.version().clone().into()) - .with_url(package_id.source_id().url().clone()) - .with_kind(package_id.source_id().kind().clone()) - } - /// Tries to convert a valid `Url` to a `PackageIdSpec`. fn from_url(mut url: Url) -> CargoResult { let mut kind = None; @@ -417,7 +408,7 @@ impl PackageIdSpecQuery for PackageIdSpec { if version_cnt[id.version()] == 1 { msg.push_str(&format!("\n {}@{}", spec.name(), id.version())); } else { - msg.push_str(&format!("\n {}", PackageIdSpec::from_package_id(*id))); + msg.push_str(&format!("\n {}", id.to_spec())); } } } diff --git a/src/cargo/ops/cargo_compile/packages.rs b/src/cargo/ops/cargo_compile/packages.rs index 2d14d60a69c..192bfcb3a3f 100644 --- a/src/cargo/ops/cargo_compile/packages.rs +++ b/src/cargo/ops/cargo_compile/packages.rs @@ -47,7 +47,7 @@ impl Packages { Packages::All => ws .members() .map(Package::package_id) - .map(PackageIdSpec::from_package_id) + .map(|id| id.to_spec()) .collect(), Packages::OptOut(opt_out) => { let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?; @@ -57,7 +57,7 @@ impl Packages { !names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns) }) .map(Package::package_id) - .map(PackageIdSpec::from_package_id) + .map(|id| id.to_spec()) .collect(); let warn = |e| ws.config().shell().warn(e); emit_package_not_found(ws, names, true).or_else(warn)?; @@ -65,7 +65,7 @@ impl Packages { specs } Packages::Packages(packages) if packages.is_empty() => { - vec![PackageIdSpec::from_package_id(ws.current()?.package_id())] + vec![ws.current()?.package_id().to_spec()] } Packages::Packages(opt_in) => { let (mut patterns, packages) = opt_patterns_and_names(opt_in)?; @@ -78,7 +78,7 @@ impl Packages { .members() .filter(|pkg| match_patterns(pkg, &mut patterns)) .map(Package::package_id) - .map(PackageIdSpec::from_package_id); + .map(|id| id.to_spec()); specs.extend(matched_pkgs); } emit_pattern_not_found(ws, patterns, false)?; @@ -87,7 +87,7 @@ impl Packages { Packages::Default => ws .default_members() .map(Package::package_id) - .map(PackageIdSpec::from_package_id) + .map(|id| id.to_spec()) .collect(), }; if specs.is_empty() { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 16027233edb..0d7836bc254 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -4,9 +4,7 @@ use std::sync::Arc; use std::{env, fs}; use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput}; -use crate::core::{ - Dependency, Edition, Package, PackageId, PackageIdSpec, SourceId, Target, Workspace, -}; +use crate::core::{Dependency, Edition, Package, PackageId, SourceId, Target, Workspace}; use crate::ops::{common_for_install_and_uninstall::*, FilterRule}; use crate::ops::{CompileFilter, Packages}; use crate::sources::source::Source; @@ -206,7 +204,7 @@ impl<'cfg> InstallablePackage<'cfg> { // For cargo install tracking, we retain the source git url in `pkg`, but for the build spec // we need to unconditionally use `ws.current()` to correctly address the path where we // locally cloned that repo. - let pkgidspec = PackageIdSpec::from_package_id(ws.current()?.package_id()); + let pkgidspec = ws.current()?.package_id().to_spec(); opts.spec = Packages::Packages(vec![pkgidspec.to_string()]); if from_cwd { diff --git a/src/cargo/ops/cargo_pkgid.rs b/src/cargo/ops/cargo_pkgid.rs index 2324e25b44e..4e81e741f78 100644 --- a/src/cargo/ops/cargo_pkgid.rs +++ b/src/cargo/ops/cargo_pkgid.rs @@ -11,5 +11,5 @@ pub fn pkgid(ws: &Workspace<'_>, spec: Option<&str>) -> CargoResult PackageIdSpec::query_str(spec, resolve.iter())?, None => ws.current()?.package_id(), }; - Ok(PackageIdSpec::from_package_id(pkgid)) + Ok(pkgid.to_spec()) } From d1b1cb184c4442e12a9ebd8f363120e586b1b564 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Dec 2023 16:26:41 -0600 Subject: [PATCH 5/5] refactor: Pull PackageIdSpec into schema --- src/cargo/core/mod.rs | 4 +- src/cargo/core/package_id_spec.rs | 603 +----------------- src/cargo/util_schemas/core/mod.rs | 2 + .../util_schemas/core/package_id_spec.rs | 601 +++++++++++++++++ src/cargo/util_schemas/manifest.rs | 2 +- 5 files changed, 619 insertions(+), 593 deletions(-) create mode 100644 src/cargo/util_schemas/core/package_id_spec.rs diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 85b961de6d9..f3b3142fa73 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -4,7 +4,7 @@ pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::manifest::{Manifest, Target, TargetKind}; pub use self::package::{Package, PackageSet}; pub use self::package_id::PackageId; -pub use self::package_id_spec::{PackageIdSpec, PackageIdSpecQuery}; +pub use self::package_id_spec::PackageIdSpecQuery; pub use self::registry::Registry; pub use self::resolver::{Resolve, ResolveVersion}; pub use self::shell::{Shell, Verbosity}; @@ -14,7 +14,7 @@ pub use self::workspace::{ find_workspace_root, resolve_relative_path, MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig, }; -pub use crate::util_schemas::core::{GitReference, SourceKind}; +pub use crate::util_schemas::core::{GitReference, PackageIdSpec, SourceKind}; pub mod compiler; pub mod dependency; diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index cd3947211d9..35c5437aebc 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -1,283 +1,11 @@ use std::collections::HashMap; -use std::fmt; use anyhow::{bail, Context as _}; -use semver::Version; -use serde::{de, ser}; -use url::Url; -use crate::core::GitReference; use crate::core::PackageId; -use crate::core::SourceKind; +use crate::core::PackageIdSpec; use crate::util::edit_distance; use crate::util::errors::CargoResult; -use crate::util::{validate_package_name, IntoUrl}; -use crate::util_semver::PartialVersion; - -/// Some or all of the data required to identify a package: -/// -/// 1. the package name (a `String`, required) -/// 2. the package version (a `Version`, optional) -/// 3. the package source (a `Url`, optional) -/// -/// If any of the optional fields are omitted, then the package ID may be ambiguous, there may be -/// more than one package/version/url combo that will match. However, often just the name is -/// sufficient to uniquely define a package ID. -#[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] -pub struct PackageIdSpec { - name: String, - version: Option, - url: Option, - kind: Option, -} - -impl PackageIdSpec { - pub fn new(name: String) -> Self { - Self { - name, - version: None, - url: None, - kind: None, - } - } - - pub fn with_version(mut self, version: PartialVersion) -> Self { - self.version = Some(version); - self - } - - pub fn with_url(mut self, url: Url) -> Self { - self.url = Some(url); - self - } - - pub fn with_kind(mut self, kind: SourceKind) -> Self { - self.kind = Some(kind); - self - } - - /// Parses a spec string and returns a `PackageIdSpec` if the string was valid. - /// - /// # Examples - /// Some examples of valid strings - /// - /// ``` - /// 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", - /// "https://crates.io/foo#bar@1.2.3", - /// "foo", - /// "foo:1.2.3", - /// "foo@1.2.3", - /// ]; - /// for spec in specs { - /// assert!(PackageIdSpec::parse(spec).is_ok()); - /// } - pub fn parse(spec: &str) -> CargoResult { - if spec.contains("://") { - if let Ok(url) = spec.into_url() { - 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, [':', '@']); - let name = parts.next().unwrap(); - let version = match parts.next() { - Some(version) => Some(version.parse::()?), - None => None, - }; - validate_package_name(name, "pkgid", "")?; - Ok(PackageIdSpec { - name: String::from(name), - version, - url: None, - kind: None, - }) - } - - /// Tries to convert a valid `Url` to a `PackageIdSpec`. - fn from_url(mut url: Url) -> CargoResult { - let mut kind = None; - if let Some((kind_str, scheme)) = url.scheme().split_once('+') { - match kind_str { - "git" => { - let git_ref = GitReference::from_query(url.query_pairs()); - url.set_query(None); - kind = Some(SourceKind::Git(git_ref)); - url = strip_url_protocol(&url); - } - "registry" => { - if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") - } - kind = Some(SourceKind::Registry); - url = strip_url_protocol(&url); - } - "sparse" => { - if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") - } - kind = Some(SourceKind::SparseRegistry); - // Leave `sparse` as part of URL, see `SourceId::new` - // url = strip_url_protocol(&url); - } - "path" => { - if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") - } - if scheme != "file" { - anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported"); - } - kind = Some(SourceKind::Path); - url = strip_url_protocol(&url); - } - kind => anyhow::bail!("unsupported source protocol: {kind}"), - } - } else { - if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") - } - } - - let frag = url.fragment().map(|s| s.to_owned()); - url.set_fragment(None); - - let (name, version) = { - let mut path = url - .path_segments() - .ok_or_else(|| anyhow::format_err!("pkgid urls must have a path: {}", url))?; - let path_name = path.next_back().ok_or_else(|| { - anyhow::format_err!( - "pkgid urls must have at least one path \ - component: {}", - url - ) - })?; - match frag { - Some(fragment) => match fragment.split_once([':', '@']) { - Some((name, part)) => { - let version = part.parse::()?; - (String::from(name), Some(version)) - } - None => { - if fragment.chars().next().unwrap().is_alphabetic() { - (String::from(fragment.as_str()), None) - } else { - let version = fragment.parse::()?; - (String::from(path_name), Some(version)) - } - } - }, - None => (String::from(path_name), None), - } - }; - Ok(PackageIdSpec { - name, - version, - url: Some(url), - kind, - }) - } - - pub fn name(&self) -> &str { - self.name.as_str() - } - - /// Full `semver::Version`, if present - pub fn version(&self) -> Option { - self.version.as_ref().and_then(|v| v.to_version()) - } - - pub fn partial_version(&self) -> Option<&PartialVersion> { - self.version.as_ref() - } - - pub fn url(&self) -> Option<&Url> { - self.url.as_ref() - } - - pub fn set_url(&mut self, url: Url) { - self.url = Some(url); - } - - pub fn kind(&self) -> Option<&SourceKind> { - self.kind.as_ref() - } - - pub fn set_kind(&mut self, kind: SourceKind) { - self.kind = Some(kind); - } -} - -fn strip_url_protocol(url: &Url) -> Url { - // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https - let raw = url.to_string(); - raw.split_once('+').unwrap().1.parse().unwrap() -} - -impl fmt::Display for PackageIdSpec { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut printed_name = false; - match self.url { - Some(ref url) => { - if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { - write!(f, "{protocol}+")?; - } - write!(f, "{}", url)?; - if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { - if let Some(pretty) = git_ref.pretty_ref(true) { - write!(f, "?{}", pretty)?; - } - } - if url.path_segments().unwrap().next_back().unwrap() != &*self.name { - printed_name = true; - write!(f, "#{}", self.name)?; - } - } - None => { - printed_name = true; - write!(f, "{}", self.name)?; - } - } - if let Some(ref v) = self.version { - write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; - } - Ok(()) - } -} - -impl ser::Serialize for PackageIdSpec { - fn serialize(&self, s: S) -> Result - where - S: ser::Serializer, - { - self.to_string().serialize(s) - } -} - -impl<'de> de::Deserialize<'de> for PackageIdSpec { - fn deserialize(d: D) -> Result - where - D: de::Deserializer<'de>, - { - let string = String::deserialize(d)?; - PackageIdSpec::parse(&string).map_err(de::Error::custom) - } -} pub trait PackageIdSpecQuery { /// Roughly equivalent to `PackageIdSpec::parse(spec)?.query(i)` @@ -313,20 +41,20 @@ impl PackageIdSpecQuery for PackageIdSpec { return false; } - if let Some(ref v) = self.version { + if let Some(ref v) = self.partial_version() { if !v.matches(package_id.version()) { return false; } } - if let Some(u) = &self.url { - if u != package_id.source_id().url() { + if let Some(u) = &self.url() { + if *u != package_id.source_id().url() { return false; } } - if let Some(k) = &self.kind { - if k != package_id.source_id().kind() { + if let Some(k) = &self.kind() { + if *k != package_id.source_id().kind() { return false; } } @@ -353,21 +81,21 @@ impl PackageIdSpecQuery for PackageIdSpec { minimize(suggestion, &try_matches, self); } }; - if self.url.is_some() { - let spec = PackageIdSpec::new(self.name.clone()); - let spec = if let Some(version) = self.version.clone() { + if self.url().is_some() { + let spec = PackageIdSpec::new(self.name().to_owned()); + let spec = if let Some(version) = self.partial_version().cloned() { spec.with_version(version) } else { spec }; try_spec(spec, &mut suggestion); } - if suggestion.is_empty() && self.version.is_some() { - try_spec(PackageIdSpec::new(self.name.clone()), &mut suggestion); + if suggestion.is_empty() && self.version().is_some() { + try_spec(PackageIdSpec::new(self.name().to_owned()), &mut suggestion); } if suggestion.is_empty() { suggestion.push_str(&edit_distance::closest_msg( - &self.name, + self.name(), all_ids.iter(), |id| id.name().as_str(), )); @@ -419,314 +147,9 @@ impl PackageIdSpecQuery for PackageIdSpec { mod tests { use super::PackageIdSpec; use super::PackageIdSpecQuery; - use crate::core::{GitReference, PackageId, SourceId, SourceKind}; + use crate::core::{PackageId, SourceId}; use url::Url; - #[test] - fn good_parsing() { - #[track_caller] - fn ok(spec: &str, expected: PackageIdSpec, expected_rendered: &str) { - let parsed = PackageIdSpec::parse(spec).unwrap(); - assert_eq!(parsed, expected); - let rendered = parsed.to_string(); - assert_eq!(rendered, expected_rendered); - let reparsed = PackageIdSpec::parse(&rendered).unwrap(); - assert_eq!(reparsed, expected); - } - - ok( - "https://crates.io/foo", - PackageIdSpec { - name: String::from("foo"), - version: None, - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: None, - }, - "https://crates.io/foo", - ); - ok( - "https://crates.io/foo#1.2.3", - PackageIdSpec { - name: String::from("foo"), - version: Some("1.2.3".parse().unwrap()), - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: None, - }, - "https://crates.io/foo#1.2.3", - ); - ok( - "https://crates.io/foo#1.2", - PackageIdSpec { - name: String::from("foo"), - version: Some("1.2".parse().unwrap()), - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: None, - }, - "https://crates.io/foo#1.2", - ); - ok( - "https://crates.io/foo#bar:1.2.3", - PackageIdSpec { - name: String::from("bar"), - version: Some("1.2.3".parse().unwrap()), - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: None, - }, - "https://crates.io/foo#bar@1.2.3", - ); - ok( - "https://crates.io/foo#bar@1.2.3", - PackageIdSpec { - name: String::from("bar"), - version: Some("1.2.3".parse().unwrap()), - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: None, - }, - "https://crates.io/foo#bar@1.2.3", - ); - ok( - "https://crates.io/foo#bar@1.2", - PackageIdSpec { - name: String::from("bar"), - version: Some("1.2".parse().unwrap()), - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: None, - }, - "https://crates.io/foo#bar@1.2", - ); - ok( - "registry+https://crates.io/foo#bar@1.2", - PackageIdSpec { - name: String::from("bar"), - version: Some("1.2".parse().unwrap()), - url: Some(Url::parse("https://crates.io/foo").unwrap()), - kind: Some(SourceKind::Registry), - }, - "registry+https://crates.io/foo#bar@1.2", - ); - ok( - "sparse+https://crates.io/foo#bar@1.2", - PackageIdSpec { - name: String::from("bar"), - version: Some("1.2".parse().unwrap()), - url: Some(Url::parse("sparse+https://crates.io/foo").unwrap()), - kind: Some(SourceKind::SparseRegistry), - }, - "sparse+https://crates.io/foo#bar@1.2", - ); - ok( - "foo", - PackageIdSpec { - name: String::from("foo"), - version: None, - url: None, - kind: None, - }, - "foo", - ); - ok( - "foo:1.2.3", - PackageIdSpec { - name: String::from("foo"), - version: Some("1.2.3".parse().unwrap()), - url: None, - kind: None, - }, - "foo@1.2.3", - ); - ok( - "foo@1.2.3", - PackageIdSpec { - name: String::from("foo"), - version: Some("1.2.3".parse().unwrap()), - url: None, - kind: None, - }, - "foo@1.2.3", - ); - ok( - "foo@1.2", - PackageIdSpec { - name: String::from("foo"), - version: Some("1.2".parse().unwrap()), - url: None, - kind: None, - }, - "foo@1.2", - ); - - // pkgid-spec.md - ok( - "regex", - PackageIdSpec { - name: String::from("regex"), - version: None, - url: None, - kind: None, - }, - "regex", - ); - ok( - "regex@1.4", - PackageIdSpec { - name: String::from("regex"), - version: Some("1.4".parse().unwrap()), - url: None, - kind: None, - }, - "regex@1.4", - ); - ok( - "regex@1.4.3", - PackageIdSpec { - name: String::from("regex"), - version: Some("1.4.3".parse().unwrap()), - url: None, - kind: 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()), - kind: None, - }, - "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()), - kind: None, - }, - "https://github.com/rust-lang/crates.io-index#regex@1.4.3", - ); - ok( - "sparse+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("sparse+https://github.com/rust-lang/crates.io-index").unwrap(), - ), - kind: Some(SourceKind::SparseRegistry), - }, - "sparse+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()), - kind: None, - }, - "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()), - kind: None, - }, - "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()), - kind: None, - }, - "ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", - ); - ok( - "git+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()), - kind: Some(SourceKind::Git(GitReference::DefaultBranch)), - }, - "git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", - ); - ok( - "git+ssh://git@github.com/rust-lang/regex.git?branch=dev#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()), - kind: Some(SourceKind::Git(GitReference::Branch("dev".to_owned()))), - }, - "git+ssh://git@github.com/rust-lang/regex.git?branch=dev#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()), - kind: None, - }, - "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()), - kind: None, - }, - "file:///path/to/my/project/foo#1.1.8", - ); - ok( - "path+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()), - kind: Some(SourceKind::Path), - }, - "path+file:///path/to/my/project/foo#1.1.8", - ); - } - - #[test] - fn bad_parsing() { - assert!(PackageIdSpec::parse("baz:").is_err()); - assert!(PackageIdSpec::parse("baz:*").is_err()); - assert!(PackageIdSpec::parse("baz@").is_err()); - assert!(PackageIdSpec::parse("baz@*").is_err()); - 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("foobar+https://github.com/rust-lang/crates.io-index").is_err() - ); - assert!(PackageIdSpec::parse("path+https://github.com/rust-lang/crates.io-index").is_err()); - - // Only `git+` can use `?` - assert!(PackageIdSpec::parse("file:///path/to/my/project/foo?branch=dev").is_err()); - assert!(PackageIdSpec::parse("path+file:///path/to/my/project/foo?branch=dev").is_err()); - assert!(PackageIdSpec::parse( - "registry+https://github.com/rust-lang/cargo#0.52.0?branch=dev" - ) - .is_err()); - assert!(PackageIdSpec::parse( - "sparse+https://github.com/rust-lang/cargo#0.52.0?branch=dev" - ) - .is_err()); - } - #[test] fn matching() { let url = Url::parse("https://example.com").unwrap(); diff --git a/src/cargo/util_schemas/core/mod.rs b/src/cargo/util_schemas/core/mod.rs index cb959a15759..2001a6bc7ee 100644 --- a/src/cargo/util_schemas/core/mod.rs +++ b/src/cargo/util_schemas/core/mod.rs @@ -1,4 +1,6 @@ +mod package_id_spec; mod source_kind; +pub use package_id_spec::PackageIdSpec; pub use source_kind::GitReference; pub use source_kind::SourceKind; diff --git a/src/cargo/util_schemas/core/package_id_spec.rs b/src/cargo/util_schemas/core/package_id_spec.rs new file mode 100644 index 00000000000..cc3f70ff852 --- /dev/null +++ b/src/cargo/util_schemas/core/package_id_spec.rs @@ -0,0 +1,601 @@ +use std::fmt; + +use anyhow::bail; +use semver::Version; +use serde::{de, ser}; +use url::Url; + +use crate::core::GitReference; +use crate::core::PackageId; +use crate::core::SourceKind; +use crate::util::errors::CargoResult; +use crate::util::{validate_package_name, IntoUrl}; +use crate::util_semver::PartialVersion; + +/// Some or all of the data required to identify a package: +/// +/// 1. the package name (a `String`, required) +/// 2. the package version (a `Version`, optional) +/// 3. the package source (a `Url`, optional) +/// +/// If any of the optional fields are omitted, then the package ID may be ambiguous, there may be +/// more than one package/version/url combo that will match. However, often just the name is +/// sufficient to uniquely define a package ID. +#[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] +pub struct PackageIdSpec { + name: String, + version: Option, + url: Option, + kind: Option, +} + +impl PackageIdSpec { + pub fn new(name: String) -> Self { + Self { + name, + version: None, + url: None, + kind: None, + } + } + + pub fn with_version(mut self, version: PartialVersion) -> Self { + self.version = Some(version); + self + } + + pub fn with_url(mut self, url: Url) -> Self { + self.url = Some(url); + self + } + + pub fn with_kind(mut self, kind: SourceKind) -> Self { + self.kind = Some(kind); + self + } + + /// Parses a spec string and returns a `PackageIdSpec` if the string was valid. + /// + /// # Examples + /// Some examples of valid strings + /// + /// ``` + /// 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", + /// "https://crates.io/foo#bar@1.2.3", + /// "foo", + /// "foo:1.2.3", + /// "foo@1.2.3", + /// ]; + /// for spec in specs { + /// assert!(PackageIdSpec::parse(spec).is_ok()); + /// } + pub fn parse(spec: &str) -> CargoResult { + if spec.contains("://") { + if let Ok(url) = spec.into_url() { + 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, [':', '@']); + let name = parts.next().unwrap(); + let version = match parts.next() { + Some(version) => Some(version.parse::()?), + None => None, + }; + validate_package_name(name, "pkgid", "")?; + Ok(PackageIdSpec { + name: String::from(name), + version, + url: None, + kind: None, + }) + } + + /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` + /// fields filled in. + pub fn from_package_id(package_id: PackageId) -> PackageIdSpec { + PackageIdSpec { + name: String::from(package_id.name().as_str()), + version: Some(package_id.version().clone().into()), + url: Some(package_id.source_id().url().clone()), + kind: Some(package_id.source_id().kind().clone()), + } + } + + /// Tries to convert a valid `Url` to a `PackageIdSpec`. + fn from_url(mut url: Url) -> CargoResult { + let mut kind = None; + if let Some((kind_str, scheme)) = url.scheme().split_once('+') { + match kind_str { + "git" => { + let git_ref = GitReference::from_query(url.query_pairs()); + url.set_query(None); + kind = Some(SourceKind::Git(git_ref)); + url = strip_url_protocol(&url); + } + "registry" => { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } + kind = Some(SourceKind::Registry); + url = strip_url_protocol(&url); + } + "sparse" => { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } + kind = Some(SourceKind::SparseRegistry); + // Leave `sparse` as part of URL, see `SourceId::new` + // url = strip_url_protocol(&url); + } + "path" => { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } + if scheme != "file" { + anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported"); + } + kind = Some(SourceKind::Path); + url = strip_url_protocol(&url); + } + kind => anyhow::bail!("unsupported source protocol: {kind}"), + } + } else { + if url.query().is_some() { + bail!("cannot have a query string in a pkgid: {url}") + } + } + + let frag = url.fragment().map(|s| s.to_owned()); + url.set_fragment(None); + + let (name, version) = { + let mut path = url + .path_segments() + .ok_or_else(|| anyhow::format_err!("pkgid urls must have a path: {}", url))?; + let path_name = path.next_back().ok_or_else(|| { + anyhow::format_err!( + "pkgid urls must have at least one path \ + component: {}", + url + ) + })?; + match frag { + Some(fragment) => match fragment.split_once([':', '@']) { + Some((name, part)) => { + let version = part.parse::()?; + (String::from(name), Some(version)) + } + None => { + if fragment.chars().next().unwrap().is_alphabetic() { + (String::from(fragment.as_str()), None) + } else { + let version = fragment.parse::()?; + (String::from(path_name), Some(version)) + } + } + }, + None => (String::from(path_name), None), + } + }; + Ok(PackageIdSpec { + name, + version, + url: Some(url), + kind, + }) + } + + pub fn name(&self) -> &str { + self.name.as_str() + } + + /// Full `semver::Version`, if present + pub fn version(&self) -> Option { + self.version.as_ref().and_then(|v| v.to_version()) + } + + pub fn partial_version(&self) -> Option<&PartialVersion> { + self.version.as_ref() + } + + pub fn url(&self) -> Option<&Url> { + self.url.as_ref() + } + + pub fn set_url(&mut self, url: Url) { + self.url = Some(url); + } + + pub fn kind(&self) -> Option<&SourceKind> { + self.kind.as_ref() + } + + pub fn set_kind(&mut self, kind: SourceKind) { + self.kind = Some(kind); + } +} + +fn strip_url_protocol(url: &Url) -> Url { + // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https + let raw = url.to_string(); + raw.split_once('+').unwrap().1.parse().unwrap() +} + +impl fmt::Display for PackageIdSpec { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut printed_name = false; + match self.url { + Some(ref url) => { + if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { + write!(f, "{protocol}+")?; + } + write!(f, "{}", url)?; + if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { + if let Some(pretty) = git_ref.pretty_ref(true) { + write!(f, "?{}", pretty)?; + } + } + if url.path_segments().unwrap().next_back().unwrap() != &*self.name { + printed_name = true; + write!(f, "#{}", self.name)?; + } + } + None => { + printed_name = true; + write!(f, "{}", self.name)?; + } + } + if let Some(ref v) = self.version { + write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; + } + Ok(()) + } +} + +impl ser::Serialize for PackageIdSpec { + fn serialize(&self, s: S) -> Result + where + S: ser::Serializer, + { + self.to_string().serialize(s) + } +} + +impl<'de> de::Deserialize<'de> for PackageIdSpec { + fn deserialize(d: D) -> Result + where + D: de::Deserializer<'de>, + { + let string = String::deserialize(d)?; + PackageIdSpec::parse(&string).map_err(de::Error::custom) + } +} + +#[cfg(test)] +mod tests { + use super::PackageIdSpec; + use crate::util_schemas::core::{GitReference, SourceKind}; + use url::Url; + + #[test] + fn good_parsing() { + #[track_caller] + fn ok(spec: &str, expected: PackageIdSpec, expected_rendered: &str) { + let parsed = PackageIdSpec::parse(spec).unwrap(); + assert_eq!(parsed, expected); + let rendered = parsed.to_string(); + assert_eq!(rendered, expected_rendered); + let reparsed = PackageIdSpec::parse(&rendered).unwrap(); + assert_eq!(reparsed, expected); + } + + ok( + "https://crates.io/foo", + PackageIdSpec { + name: String::from("foo"), + version: None, + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, + }, + "https://crates.io/foo", + ); + ok( + "https://crates.io/foo#1.2.3", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.2.3".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, + }, + "https://crates.io/foo#1.2.3", + ); + ok( + "https://crates.io/foo#1.2", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, + }, + "https://crates.io/foo#1.2", + ); + ok( + "https://crates.io/foo#bar:1.2.3", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2.3".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, + }, + "https://crates.io/foo#bar@1.2.3", + ); + ok( + "https://crates.io/foo#bar@1.2.3", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2.3".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, + }, + "https://crates.io/foo#bar@1.2.3", + ); + ok( + "https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: None, + }, + "https://crates.io/foo#bar@1.2", + ); + ok( + "registry+https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + kind: Some(SourceKind::Registry), + }, + "registry+https://crates.io/foo#bar@1.2", + ); + ok( + "sparse+https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: String::from("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("sparse+https://crates.io/foo").unwrap()), + kind: Some(SourceKind::SparseRegistry), + }, + "sparse+https://crates.io/foo#bar@1.2", + ); + ok( + "foo", + PackageIdSpec { + name: String::from("foo"), + version: None, + url: None, + kind: None, + }, + "foo", + ); + ok( + "foo:1.2.3", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.2.3".parse().unwrap()), + url: None, + kind: None, + }, + "foo@1.2.3", + ); + ok( + "foo@1.2.3", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.2.3".parse().unwrap()), + url: None, + kind: None, + }, + "foo@1.2.3", + ); + ok( + "foo@1.2", + PackageIdSpec { + name: String::from("foo"), + version: Some("1.2".parse().unwrap()), + url: None, + kind: None, + }, + "foo@1.2", + ); + + // pkgid-spec.md + ok( + "regex", + PackageIdSpec { + name: String::from("regex"), + version: None, + url: None, + kind: None, + }, + "regex", + ); + ok( + "regex@1.4", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4".parse().unwrap()), + url: None, + kind: None, + }, + "regex@1.4", + ); + ok( + "regex@1.4.3", + PackageIdSpec { + name: String::from("regex"), + version: Some("1.4.3".parse().unwrap()), + url: None, + kind: 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()), + kind: None, + }, + "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()), + kind: None, + }, + "https://github.com/rust-lang/crates.io-index#regex@1.4.3", + ); + ok( + "sparse+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("sparse+https://github.com/rust-lang/crates.io-index").unwrap(), + ), + kind: Some(SourceKind::SparseRegistry), + }, + "sparse+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()), + kind: None, + }, + "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()), + kind: None, + }, + "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()), + kind: None, + }, + "ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", + ); + ok( + "git+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()), + kind: Some(SourceKind::Git(GitReference::DefaultBranch)), + }, + "git+ssh://git@github.com/rust-lang/regex.git#regex@1.4.3", + ); + ok( + "git+ssh://git@github.com/rust-lang/regex.git?branch=dev#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()), + kind: Some(SourceKind::Git(GitReference::Branch("dev".to_owned()))), + }, + "git+ssh://git@github.com/rust-lang/regex.git?branch=dev#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()), + kind: None, + }, + "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()), + kind: None, + }, + "file:///path/to/my/project/foo#1.1.8", + ); + ok( + "path+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()), + kind: Some(SourceKind::Path), + }, + "path+file:///path/to/my/project/foo#1.1.8", + ); + } + + #[test] + fn bad_parsing() { + assert!(PackageIdSpec::parse("baz:").is_err()); + assert!(PackageIdSpec::parse("baz:*").is_err()); + assert!(PackageIdSpec::parse("baz@").is_err()); + assert!(PackageIdSpec::parse("baz@*").is_err()); + 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("foobar+https://github.com/rust-lang/crates.io-index").is_err() + ); + assert!(PackageIdSpec::parse("path+https://github.com/rust-lang/crates.io-index").is_err()); + + // Only `git+` can use `?` + assert!(PackageIdSpec::parse("file:///path/to/my/project/foo?branch=dev").is_err()); + assert!(PackageIdSpec::parse("path+file:///path/to/my/project/foo?branch=dev").is_err()); + assert!(PackageIdSpec::parse( + "registry+https://github.com/rust-lang/cargo#0.52.0?branch=dev" + ) + .is_err()); + assert!(PackageIdSpec::parse( + "sparse+https://github.com/rust-lang/cargo#0.52.0?branch=dev" + ) + .is_err()); + } +} diff --git a/src/cargo/util_schemas/manifest.rs b/src/cargo/util_schemas/manifest.rs index 588dca678cf..c36cbc7e2c9 100644 --- a/src/cargo/util_schemas/manifest.rs +++ b/src/cargo/util_schemas/manifest.rs @@ -15,8 +15,8 @@ use serde::ser; use serde::{Deserialize, Serialize}; use serde_untagged::UntaggedEnumVisitor; -use crate::core::PackageIdSpec; use crate::util::RustVersion; +use crate::util_schemas::core::PackageIdSpec; /// This type is used to deserialize `Cargo.toml` files. #[derive(Debug, Deserialize, Serialize)]