From 4135f562b8567405d023e1628da638aa4675c380 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 31 Aug 2023 10:00:24 -0500 Subject: [PATCH 1/9] refactor(manifest): Clarify the specialized role of RustVersion --- crates/resolver-tests/src/lib.rs | 10 +++++----- src/cargo/core/manifest.rs | 10 +++++----- src/cargo/core/package.rs | 6 +++--- src/cargo/core/resolver/dep_cache.rs | 6 +++--- src/cargo/core/resolver/mod.rs | 4 ++-- src/cargo/core/resolver/version_prefs.rs | 4 ++-- src/cargo/core/summary.rs | 8 ++++---- src/cargo/core/workspace.rs | 4 ++-- src/cargo/ops/cargo_add/mod.rs | 6 +++--- src/cargo/ops/resolve.rs | 8 ++++---- src/cargo/sources/registry/index.rs | 4 ++-- src/cargo/util/mod.rs | 2 +- src/cargo/util/semver_ext.rs | 16 ++++++++-------- src/cargo/util/toml/mod.rs | 18 +++++++++--------- 14 files changed, 53 insertions(+), 53 deletions(-) diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index 12f6171b120..f6e93e65cde 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -17,7 +17,7 @@ use cargo::core::Resolve; use cargo::core::{Dependency, PackageId, Registry, Summary}; use cargo::core::{GitReference, SourceId}; use cargo::sources::source::QueryKind; -use cargo::util::{CargoResult, Config, Graph, IntoUrl, PartialVersion}; +use cargo::util::{CargoResult, Config, Graph, IntoUrl, RustVersion}; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; @@ -185,7 +185,7 @@ pub fn resolve_with_config_raw( deps, &BTreeMap::new(), None::<&String>, - None::, + None::, ) .unwrap(); let opts = ResolveOpts::everything(); @@ -588,7 +588,7 @@ pub fn pkg_dep(name: T, dep: Vec) -> Summary { dep, &BTreeMap::new(), link, - None::, + None::, ) .unwrap() } @@ -616,7 +616,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary { Vec::new(), &BTreeMap::new(), link, - None::, + None::, ) .unwrap() } @@ -630,7 +630,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { deps, &BTreeMap::new(), sum.links().map(|a| a.as_str()), - None::, + None::, ) .unwrap() } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index a6ccc07ce3a..3422bf8faf1 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -19,7 +19,7 @@ use crate::core::{Edition, Feature, Features, WorkspaceConfig}; use crate::util::errors::*; use crate::util::interning::InternedString; use crate::util::toml::{TomlManifest, TomlProfiles}; -use crate::util::{short_hash, Config, Filesystem, PartialVersion}; +use crate::util::{short_hash, Config, Filesystem, RustVersion}; pub enum EitherManifest { Real(Manifest), @@ -58,7 +58,7 @@ pub struct Manifest { original: Rc, unstable_features: Features, edition: Edition, - rust_version: Option, + rust_version: Option, im_a_teapot: Option, default_run: Option, metabuild: Option>, @@ -112,7 +112,7 @@ pub struct ManifestMetadata { pub documentation: Option, // URL pub badges: BTreeMap>, pub links: Option, - pub rust_version: Option, + pub rust_version: Option, } #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] @@ -401,7 +401,7 @@ impl Manifest { workspace: WorkspaceConfig, unstable_features: Features, edition: Edition, - rust_version: Option, + rust_version: Option, im_a_teapot: Option, default_run: Option, original: Rc, @@ -570,7 +570,7 @@ impl Manifest { self.edition } - pub fn rust_version(&self) -> Option { + pub fn rust_version(&self) -> Option { self.rust_version } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index a4a0eaa3470..b09d09cd47c 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -31,7 +31,7 @@ use crate::util::network::http::http_handle_and_timeout; use crate::util::network::http::HttpTimeout; use crate::util::network::retry::{Retry, RetryResult}; use crate::util::network::sleep::SleepTracker; -use crate::util::PartialVersion; +use crate::util::RustVersion; use crate::util::{self, internal, Config, Progress, ProgressStyle}; pub const MANIFEST_PREAMBLE: &str = "\ @@ -104,7 +104,7 @@ pub struct SerializedPackage { #[serde(skip_serializing_if = "Option::is_none")] metabuild: Option>, default_run: Option, - rust_version: Option, + rust_version: Option, } impl Package { @@ -178,7 +178,7 @@ impl Package { self.targets().iter().any(|target| target.proc_macro()) } /// Gets the package's minimum Rust version. - pub fn rust_version(&self) -> Option { + pub fn rust_version(&self) -> Option { self.manifest().rust_version() } diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index f4e1cfc126f..47ad629dca2 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -20,7 +20,7 @@ use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, use crate::sources::source::QueryKind; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::PartialVersion; +use crate::util::RustVersion; use anyhow::Context as _; use std::collections::{BTreeSet, HashMap, HashSet}; @@ -36,7 +36,7 @@ pub struct RegistryQueryer<'a> { /// versions first. That allows `cargo update -Z minimal-versions` which will /// specify minimum dependency versions to be used. minimal_versions: bool, - max_rust_version: Option, + max_rust_version: Option, /// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_minimal_version`) registry_cache: HashMap<(Dependency, bool), Poll>>>, /// a cache of `Dependency`s that are required for a `Summary` @@ -58,7 +58,7 @@ impl<'a> RegistryQueryer<'a> { replacements: &'a [(PackageIdSpec, Dependency)], version_prefs: &'a VersionPreferences, minimal_versions: bool, - max_rust_version: Option, + max_rust_version: Option, ) -> Self { RegistryQueryer { registry, diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 9e7d41f60cd..37a0563d9d5 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -71,7 +71,7 @@ use crate::util::config::Config; use crate::util::errors::CargoResult; use crate::util::network::PollExt; use crate::util::profile; -use crate::util::PartialVersion; +use crate::util::RustVersion; use self::context::Context; use self::dep_cache::RegistryQueryer; @@ -139,7 +139,7 @@ pub fn resolve( version_prefs: &VersionPreferences, config: Option<&Config>, check_public_visible_dependencies: bool, - mut max_rust_version: Option, + mut max_rust_version: Option, ) -> CargoResult { let _p = profile::start("resolving"); let minimal_versions = match config { diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index a61f7cac37b..28de77f118a 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -81,7 +81,7 @@ impl VersionPreferences { mod test { use super::*; use crate::core::SourceId; - use crate::util::PartialVersion; + use crate::util::RustVersion; use std::collections::BTreeMap; fn pkgid(name: &str, version: &str) -> PackageId { @@ -104,7 +104,7 @@ mod test { Vec::new(), &features, None::<&String>, - None::, + None::, ) .unwrap() } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 02007335830..7783832be15 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -1,7 +1,7 @@ use crate::core::{Dependency, PackageId, SourceId}; use crate::util::interning::InternedString; use crate::util::CargoResult; -use crate::util::PartialVersion; +use crate::util::RustVersion; use anyhow::bail; use semver::Version; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -26,7 +26,7 @@ struct Inner { features: Rc, checksum: Option, links: Option, - rust_version: Option, + rust_version: Option, } impl Summary { @@ -35,7 +35,7 @@ impl Summary { dependencies: Vec, features: &BTreeMap>, links: Option>, - rust_version: Option, + rust_version: Option, ) -> CargoResult { // ****CAUTION**** If you change anything here that may raise a new // error, be sure to coordinate that change with either the index @@ -88,7 +88,7 @@ impl Summary { self.inner.links } - pub fn rust_version(&self) -> Option { + pub fn rust_version(&self) -> Option { self.inner.rust_version } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 2d41e095625..a7767ec575e 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -23,7 +23,7 @@ use crate::util::edit_distance; use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; use crate::util::toml::{read_manifest, InheritableFields, TomlDependency, TomlProfiles}; -use crate::util::PartialVersion; +use crate::util::RustVersion; use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; use cargo_util::paths; use cargo_util::paths::normalize_path; @@ -598,7 +598,7 @@ impl<'cfg> Workspace<'cfg> { /// Get the lowest-common denominator `package.rust-version` within the workspace, if specified /// anywhere - pub fn rust_version(&self) -> Option { + pub fn rust_version(&self) -> Option { self.members().filter_map(|pkg| pkg.rust_version()).min() } diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index cdfe5881271..c91f50aa05e 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -32,7 +32,7 @@ use crate::util::toml_mut::dependency::Source; use crate::util::toml_mut::dependency::WorkspaceSource; use crate::util::toml_mut::manifest::DepTable; use crate::util::toml_mut::manifest::LocalManifest; -use crate::util::PartialVersion; +use crate::util::RustVersion; use crate::CargoResult; use crate::Config; use crate_spec::CrateSpec; @@ -564,7 +564,7 @@ fn get_latest_dependency( })?; if config.cli_unstable().msrv_policy && honor_rust_version { - fn parse_msrv(comp: PartialVersion) -> (u64, u64, u64) { + fn parse_msrv(comp: RustVersion) -> (u64, u64, u64) { (comp.major, comp.minor.unwrap_or(0), comp.patch.unwrap_or(0)) } @@ -624,7 +624,7 @@ fn get_latest_dependency( fn rust_version_incompat_error( dep: &str, - rust_version: PartialVersion, + rust_version: RustVersion, lowest_rust_version: Option<&Summary>, ) -> anyhow::Error { let mut error_msg = format!( diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 934eb3571d4..f3b8f8d9ae3 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -69,7 +69,7 @@ use crate::core::{GitReference, PackageId, PackageIdSpec, PackageSet, SourceId, use crate::ops; use crate::sources::PathSource; use crate::util::errors::CargoResult; -use crate::util::PartialVersion; +use crate::util::RustVersion; use crate::util::{profile, CanonicalUrl}; use anyhow::Context as _; use std::collections::{HashMap, HashSet}; @@ -133,7 +133,7 @@ pub fn resolve_ws_with_opts<'cfg>( specs: &[PackageIdSpec], has_dev_units: HasDevUnits, force_all_targets: ForceAllTargets, - max_rust_version: Option, + max_rust_version: Option, ) -> CargoResult> { let mut registry = PackageRegistry::new(ws.config())?; let mut add_patches = true; @@ -240,7 +240,7 @@ pub fn resolve_ws_with_opts<'cfg>( fn resolve_with_registry<'cfg>( ws: &Workspace<'cfg>, registry: &mut PackageRegistry<'cfg>, - max_rust_version: Option, + max_rust_version: Option, ) -> CargoResult { let prev = ops::load_pkg_lockfile(ws)?; let mut resolve = resolve_with_previous( @@ -285,7 +285,7 @@ pub fn resolve_with_previous<'cfg>( to_avoid: Option<&HashSet>, specs: &[PackageIdSpec], register_patches: bool, - max_rust_version: Option, + max_rust_version: Option, ) -> CargoResult { // We only want one Cargo at a time resolving a crate graph since this can // involve a lot of frobbing of the global caches. diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 4b6d5048be6..dc176fd800e 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -91,7 +91,7 @@ use crate::core::{PackageId, SourceId, Summary}; use crate::sources::registry::{LoadResponse, RegistryData}; use crate::util::interning::InternedString; use crate::util::IntoUrl; -use crate::util::{internal, CargoResult, Config, Filesystem, OptVersionReq, PartialVersion}; +use crate::util::{internal, CargoResult, Config, Filesystem, OptVersionReq, RustVersion}; use anyhow::bail; use cargo_util::{paths, registry::make_dep_path}; use semver::Version; @@ -358,7 +358,7 @@ pub struct IndexPackage<'a> { /// /// Added in 2023 (see ), /// can be `None` if published before then or if not set in the manifest. - rust_version: Option, + rust_version: Option, /// The schema version for this entry. /// /// If this is None, it defaults to version `1`. Entries with unknown diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index d74351cd016..cb9a518ed7e 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -22,7 +22,7 @@ pub use self::progress::{Progress, ProgressStyle}; pub use self::queue::Queue; pub use self::restricted_names::validate_package_name; pub use self::rustc::Rustc; -pub use self::semver_ext::{OptVersionReq, PartialVersion, VersionExt, VersionReqExt}; +pub use self::semver_ext::{OptVersionReq, RustVersion, VersionExt, VersionReqExt}; pub use self::to_semver::ToSemver; pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo}; pub use self::workspace::{ diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index cddaa97c9cd..44cfc0c114e 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -110,13 +110,13 @@ impl From for OptVersionReq { } #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone, Debug)] -pub struct PartialVersion { +pub struct RustVersion { pub major: u64, pub minor: Option, pub patch: Option, } -impl PartialVersion { +impl RustVersion { pub fn caret_req(&self) -> VersionReq { VersionReq { comparators: vec![Comparator { @@ -130,11 +130,11 @@ impl PartialVersion { } } -impl std::str::FromStr for PartialVersion { +impl std::str::FromStr for RustVersion { type Err = anyhow::Error; fn from_str(value: &str) -> Result { - // HACK: `PartialVersion` is a subset of the `VersionReq` syntax that only ever + // HACK: `RustVersion` is a subset of the `VersionReq` syntax that only ever // has one comparator with a required minor and optional patch, and uses no // other features. if is_req(value) { @@ -163,7 +163,7 @@ impl std::str::FromStr for PartialVersion { semver::Prerelease::EMPTY, "guarenteed by character check" ); - Ok(PartialVersion { + Ok(RustVersion { major: comp.major, minor: comp.minor, patch: comp.patch, @@ -171,7 +171,7 @@ impl std::str::FromStr for PartialVersion { } } -impl Display for PartialVersion { +impl Display for RustVersion { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let major = self.major; write!(f, "{major}")?; @@ -185,7 +185,7 @@ impl Display for PartialVersion { } } -impl serde::Serialize for PartialVersion { +impl serde::Serialize for RustVersion { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer, @@ -194,7 +194,7 @@ impl serde::Serialize for PartialVersion { } } -impl<'de> serde::Deserialize<'de> for PartialVersion { +impl<'de> serde::Deserialize<'de> for RustVersion { fn deserialize(deserializer: D) -> Result where D: serde::Deserializer<'de>, diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 34161542fb3..9a8f1aa05e1 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -30,7 +30,7 @@ use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; use crate::util::{ self, config::ConfigRelativePath, validate_package_name, Config, IntoUrl, OptVersionReq, - PartialVersion, + RustVersion, }; pub mod embedded; @@ -1182,8 +1182,8 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { } } -type MaybeWorkspacePartialVersion = MaybeWorkspace; -impl<'de> de::Deserialize<'de> for MaybeWorkspacePartialVersion { +type MaybeWorkspaceRustVersion = MaybeWorkspace; +impl<'de> de::Deserialize<'de> for MaybeWorkspaceRustVersion { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -1191,7 +1191,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspacePartialVersion { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspacePartialVersion; + type Value = MaybeWorkspaceRustVersion; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { f.write_str("a semver or workspace") @@ -1201,8 +1201,8 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspacePartialVersion { where E: de::Error, { - let value = value.parse::().map_err(|e| E::custom(e))?; - Ok(MaybeWorkspacePartialVersion::Defined(value)) + let value = value.parse::().map_err(|e| E::custom(e))?; + Ok(MaybeWorkspaceRustVersion::Defined(value)) } fn visit_map(self, map: V) -> Result @@ -1400,7 +1400,7 @@ impl WorkspaceInherit for TomlWorkspaceField { #[serde(rename_all = "kebab-case")] pub struct TomlPackage { edition: Option, - rust_version: Option, + rust_version: Option, name: InternedString, #[serde(deserialize_with = "version_trim_whitespace")] version: MaybeWorkspaceSemverVersion, @@ -1490,7 +1490,7 @@ pub struct InheritableFields { exclude: Option>, include: Option>, #[serde(rename = "rust-version")] - rust_version: Option, + rust_version: Option, // We use skip here since it will never be present when deserializing // and we don't want it present when serializing #[serde(skip)] @@ -1530,7 +1530,7 @@ impl InheritableFields { ("package.license", license -> String), ("package.publish", publish -> VecStringOrBool), ("package.repository", repository -> String), - ("package.rust-version", rust_version -> PartialVersion), + ("package.rust-version", rust_version -> RustVersion), ("package.version", version -> semver::Version), } From 0f0d0fb1afea67691b1140bccc1db4bc145ae92b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 31 Aug 2023 10:10:53 -0500 Subject: [PATCH 2/9] refactor(manifest): Extract a more general PartialVersion --- src/cargo/util/semver_ext.rs | 44 +++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index 44cfc0c114e..5dff740f3c2 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -109,14 +109,42 @@ impl From for OptVersionReq { } } +#[derive( + PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone, Debug, serde::Serialize, serde::Deserialize, +)] +#[serde(transparent)] +pub struct RustVersion(PartialVersion); + +impl std::ops::Deref for RustVersion { + type Target = PartialVersion; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::str::FromStr for RustVersion { + type Err = anyhow::Error; + + fn from_str(value: &str) -> Result { + value.parse::().map(Self) + } +} + +impl Display for RustVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone, Debug)] -pub struct RustVersion { +pub struct PartialVersion { pub major: u64, pub minor: Option, pub patch: Option, } -impl RustVersion { +impl PartialVersion { pub fn caret_req(&self) -> VersionReq { VersionReq { comparators: vec![Comparator { @@ -130,11 +158,11 @@ impl RustVersion { } } -impl std::str::FromStr for RustVersion { +impl std::str::FromStr for PartialVersion { type Err = anyhow::Error; fn from_str(value: &str) -> Result { - // HACK: `RustVersion` is a subset of the `VersionReq` syntax that only ever + // HACK: `PartialVersion` is a subset of the `VersionReq` syntax that only ever // has one comparator with a required minor and optional patch, and uses no // other features. if is_req(value) { @@ -163,7 +191,7 @@ impl std::str::FromStr for RustVersion { semver::Prerelease::EMPTY, "guarenteed by character check" ); - Ok(RustVersion { + Ok(PartialVersion { major: comp.major, minor: comp.minor, patch: comp.patch, @@ -171,7 +199,7 @@ impl std::str::FromStr for RustVersion { } } -impl Display for RustVersion { +impl Display for PartialVersion { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let major = self.major; write!(f, "{major}")?; @@ -185,7 +213,7 @@ impl Display for RustVersion { } } -impl serde::Serialize for RustVersion { +impl serde::Serialize for PartialVersion { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer, @@ -194,7 +222,7 @@ impl serde::Serialize for RustVersion { } } -impl<'de> serde::Deserialize<'de> for RustVersion { +impl<'de> serde::Deserialize<'de> for PartialVersion { fn deserialize(deserializer: D) -> Result where D: serde::Deserializer<'de>, From 2438559961bc33687ad16eed3a4594debdff6571 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 31 Aug 2023 19:42:03 -0500 Subject: [PATCH 3/9] refactor(manifest): Drop 'Copy' from 'PartialVersion' This will unblock handling of prerelease / metadata --- src/cargo/core/manifest.rs | 4 ++-- src/cargo/core/package.rs | 4 ++-- src/cargo/core/resolver/dep_cache.rs | 7 ++++--- src/cargo/core/resolver/mod.rs | 2 +- src/cargo/core/summary.rs | 4 ++-- src/cargo/core/workspace.rs | 2 +- src/cargo/ops/cargo_add/mod.rs | 4 ++-- src/cargo/ops/cargo_package.rs | 4 ++-- src/cargo/ops/resolve.rs | 6 +++--- src/cargo/util/semver_ext.rs | 4 ++-- src/cargo/util/toml/mod.rs | 9 +++++---- 11 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 3422bf8faf1..7886abec302 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -570,8 +570,8 @@ impl Manifest { self.edition } - pub fn rust_version(&self) -> Option { - self.rust_version + pub fn rust_version(&self) -> Option<&RustVersion> { + self.rust_version.as_ref() } pub fn custom_metadata(&self) -> Option<&toml::Value> { diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index b09d09cd47c..76f6c405bc3 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -178,7 +178,7 @@ impl Package { self.targets().iter().any(|target| target.proc_macro()) } /// Gets the package's minimum Rust version. - pub fn rust_version(&self) -> Option { + pub fn rust_version(&self) -> Option<&RustVersion> { self.manifest().rust_version() } @@ -263,7 +263,7 @@ impl Package { metabuild: self.manifest().metabuild().cloned(), publish: self.publish().as_ref().cloned(), default_run: self.manifest().default_run().map(|s| s.to_owned()), - rust_version: self.rust_version(), + rust_version: self.rust_version().cloned(), } } } diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 47ad629dca2..b230254cccd 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -58,14 +58,14 @@ impl<'a> RegistryQueryer<'a> { replacements: &'a [(PackageIdSpec, Dependency)], version_prefs: &'a VersionPreferences, minimal_versions: bool, - max_rust_version: Option, + max_rust_version: Option<&RustVersion>, ) -> Self { RegistryQueryer { registry, replacements, version_prefs, minimal_versions, - max_rust_version, + max_rust_version: max_rust_version.cloned(), registry_cache: HashMap::new(), summary_cache: HashMap::new(), used_replacements: HashMap::new(), @@ -115,7 +115,8 @@ impl<'a> RegistryQueryer<'a> { let mut ret = Vec::new(); let ready = self.registry.query(dep, QueryKind::Exact, &mut |s| { - if self.max_rust_version.is_none() || s.rust_version() <= self.max_rust_version { + if self.max_rust_version.is_none() || s.rust_version() <= self.max_rust_version.as_ref() + { ret.push(s); } })?; diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 37a0563d9d5..7d8e8acd406 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -139,7 +139,7 @@ pub fn resolve( version_prefs: &VersionPreferences, config: Option<&Config>, check_public_visible_dependencies: bool, - mut max_rust_version: Option, + mut max_rust_version: Option<&RustVersion>, ) -> CargoResult { let _p = profile::start("resolving"); let minimal_versions = match config { diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 7783832be15..128c0db9cb1 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -88,8 +88,8 @@ impl Summary { self.inner.links } - pub fn rust_version(&self) -> Option { - self.inner.rust_version + pub fn rust_version(&self) -> Option<&RustVersion> { + self.inner.rust_version.as_ref() } pub fn override_id(mut self, id: PackageId) -> Summary { diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index a7767ec575e..db379d78073 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -598,7 +598,7 @@ impl<'cfg> Workspace<'cfg> { /// Get the lowest-common denominator `package.rust-version` within the workspace, if specified /// anywhere - pub fn rust_version(&self) -> Option { + pub fn rust_version(&self) -> Option<&RustVersion> { self.members().filter_map(|pkg| pkg.rust_version()).min() } diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index c91f50aa05e..6bab52e4075 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -564,7 +564,7 @@ fn get_latest_dependency( })?; if config.cli_unstable().msrv_policy && honor_rust_version { - fn parse_msrv(comp: RustVersion) -> (u64, u64, u64) { + fn parse_msrv(comp: &RustVersion) -> (u64, u64, u64) { (comp.major, comp.minor.unwrap_or(0), comp.patch.unwrap_or(0)) } @@ -624,7 +624,7 @@ fn get_latest_dependency( fn rust_version_incompat_error( dep: &str, - rust_version: RustVersion, + rust_version: &RustVersion, lowest_rust_version: Option<&Summary>, ) -> anyhow::Error { let mut error_msg = format!( diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 1a82cbd6b5b..b6423aa9684 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -423,7 +423,7 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult { TomlManifest::to_real_manifest(&toml_manifest, false, source_id, package_root, config)?; let new_pkg = Package::new(manifest, orig_pkg.manifest_path()); - let max_rust_version = new_pkg.rust_version(); + let max_rust_version = new_pkg.rust_version().cloned(); // Regenerate Cargo.lock using the old one as a guide. let tmp_ws = Workspace::ephemeral(new_pkg, ws.config(), None, true)?; @@ -437,7 +437,7 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult { None, &[], true, - max_rust_version, + max_rust_version.as_ref(), )?; let pkg_set = ops::get_resolved_packages(&new_resolve, tmp_reg)?; diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index f3b8f8d9ae3..053098d5559 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -133,7 +133,7 @@ pub fn resolve_ws_with_opts<'cfg>( specs: &[PackageIdSpec], has_dev_units: HasDevUnits, force_all_targets: ForceAllTargets, - max_rust_version: Option, + max_rust_version: Option<&RustVersion>, ) -> CargoResult> { let mut registry = PackageRegistry::new(ws.config())?; let mut add_patches = true; @@ -240,7 +240,7 @@ pub fn resolve_ws_with_opts<'cfg>( fn resolve_with_registry<'cfg>( ws: &Workspace<'cfg>, registry: &mut PackageRegistry<'cfg>, - max_rust_version: Option, + max_rust_version: Option<&RustVersion>, ) -> CargoResult { let prev = ops::load_pkg_lockfile(ws)?; let mut resolve = resolve_with_previous( @@ -285,7 +285,7 @@ pub fn resolve_with_previous<'cfg>( to_avoid: Option<&HashSet>, specs: &[PackageIdSpec], register_patches: bool, - max_rust_version: Option, + max_rust_version: Option<&RustVersion>, ) -> CargoResult { // We only want one Cargo at a time resolving a crate graph since this can // involve a lot of frobbing of the global caches. diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index 5dff740f3c2..f7b7c6145be 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -110,7 +110,7 @@ impl From for OptVersionReq { } #[derive( - PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone, Debug, serde::Serialize, serde::Deserialize, + PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug, serde::Serialize, serde::Deserialize, )] #[serde(transparent)] pub struct RustVersion(PartialVersion); @@ -137,7 +137,7 @@ impl Display for RustVersion { } } -#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone, Debug)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug)] pub struct PartialVersion { pub major: u64, pub minor: Option, diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9a8f1aa05e1..c43621fc036 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1961,8 +1961,9 @@ impl TomlManifest { } let rust_version = if let Some(rust_version) = &package.rust_version { - let rust_version = - rust_version.resolve("rust_version", || inherit()?.rust_version())?; + let rust_version = rust_version + .clone() + .resolve("rust_version", || inherit()?.rust_version())?; let req = rust_version.caret_req(); if let Some(first_version) = edition.first_version() { let unsupported = @@ -2244,7 +2245,7 @@ impl TomlManifest { deps, me.features.as_ref().unwrap_or(&empty_features), package.links.as_deref(), - rust_version, + rust_version.clone(), )?; let metadata = ManifestMetadata { @@ -2357,7 +2358,7 @@ impl TomlManifest { .categories .as_ref() .map(|_| MaybeWorkspace::Defined(metadata.categories.clone())); - package.rust_version = rust_version.map(|rv| MaybeWorkspace::Defined(rv)); + package.rust_version = rust_version.clone().map(|rv| MaybeWorkspace::Defined(rv)); package.exclude = package .exclude .as_ref() From 86324486ee8bfbcd644bc691db1d744b9ff45cf4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 12:31:14 -0500 Subject: [PATCH 4/9] refactor(manifest): Put conditions in field order --- src/cargo/util/semver_ext.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index f7b7c6145be..46dd836f572 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -171,12 +171,12 @@ impl std::str::FromStr for PartialVersion { let version_req = match semver::VersionReq::parse(value) { // Exclude semver operators like `^` and pre-release identifiers Ok(req) if value.chars().all(|c| c.is_ascii_digit() || c == '.') => req, - _ if value.contains('+') => { - anyhow::bail!("unexpected build field, expected a version like \"1.32\"") - } _ if value.contains('-') => { anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"") } + _ if value.contains('+') => { + anyhow::bail!("unexpected build field, expected a version like \"1.32\"") + } _ => anyhow::bail!("expected a version like \"1.32\""), }; assert_eq!( From 67fa43d09c633ccf79cc0989197e5aa4a43013d6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 12:51:35 -0500 Subject: [PATCH 5/9] refactor(manifest): Leverage Version::parse for PartialVersion --- src/cargo/util/semver_ext.rs | 77 ++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index 46dd836f572..e73b669f82f 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -158,44 +158,61 @@ impl PartialVersion { } } +impl TryFrom for PartialVersion { + type Error = anyhow::Error; + fn try_from(value: semver::Version) -> Result { + if !value.pre.is_empty() { + anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"") + } + if !value.build.is_empty() { + anyhow::bail!("unexpected build field, expected a version like \"1.32\"") + } + Ok(Self { + major: value.major, + minor: Some(value.minor), + patch: Some(value.patch), + }) + } +} + impl std::str::FromStr for PartialVersion { type Err = anyhow::Error; fn from_str(value: &str) -> Result { - // HACK: `PartialVersion` is a subset of the `VersionReq` syntax that only ever - // has one comparator with a required minor and optional patch, and uses no - // other features. if is_req(value) { anyhow::bail!("unexpected version requirement, expected a version like \"1.32\"") } - let version_req = match semver::VersionReq::parse(value) { - // Exclude semver operators like `^` and pre-release identifiers - Ok(req) if value.chars().all(|c| c.is_ascii_digit() || c == '.') => req, - _ if value.contains('-') => { - anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"") + match semver::Version::parse(value) { + Ok(ver) => ver.try_into(), + Err(_) => { + // HACK: Leverage `VersionReq` for partial version parsing + let mut version_req = match semver::VersionReq::parse(value) { + Ok(req) => req, + Err(_) if value.contains('-') => { + anyhow::bail!( + "unexpected prerelease field, expected a version like \"1.32\"" + ) + } + Err(_) if value.contains('+') => { + anyhow::bail!("unexpected build field, expected a version like \"1.32\"") + } + Err(_) => anyhow::bail!("expected a version like \"1.32\""), + }; + assert_eq!(version_req.comparators.len(), 1, "guarenteed by is_req"); + let comp = version_req.comparators.pop().unwrap(); + assert_eq!(comp.op, semver::Op::Caret, "guarenteed by is_req"); + assert_eq!( + comp.pre, + semver::Prerelease::EMPTY, + "guarenteed by `Version::parse` failing" + ); + Ok(Self { + major: comp.major, + minor: comp.minor, + patch: comp.patch, + }) } - _ if value.contains('+') => { - anyhow::bail!("unexpected build field, expected a version like \"1.32\"") - } - _ => anyhow::bail!("expected a version like \"1.32\""), - }; - assert_eq!( - version_req.comparators.len(), - 1, - "guarenteed by character check" - ); - let comp = &version_req.comparators[0]; - assert_eq!(comp.op, semver::Op::Caret, "guarenteed by character check"); - assert_eq!( - comp.pre, - semver::Prerelease::EMPTY, - "guarenteed by character check" - ); - Ok(PartialVersion { - major: comp.major, - minor: comp.minor, - patch: comp.patch, - }) + } } } From 4676e87e127654706d0e7c269b9d66431d21b237 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 12:55:55 -0500 Subject: [PATCH 6/9] refactor(manifest): Shift pre/build validation from PartialVersion to RustVersion --- src/cargo/util/semver_ext.rs | 80 +++++++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 24 deletions(-) diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index e73b669f82f..b03473fb3fa 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -109,9 +109,7 @@ impl From for OptVersionReq { } } -#[derive( - PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug, serde::Serialize, serde::Deserialize, -)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug, serde::Serialize)] #[serde(transparent)] pub struct RustVersion(PartialVersion); @@ -127,7 +125,26 @@ impl std::str::FromStr for RustVersion { type Err = anyhow::Error; fn from_str(value: &str) -> Result { - value.parse::().map(Self) + let partial = value.parse::()?; + if partial.pre.is_some() { + anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"") + } + if partial.build.is_some() { + anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"") + } + Ok(Self(partial)) + } +} + +impl<'de> serde::Deserialize<'de> for RustVersion { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + UntaggedEnumVisitor::new() + .expecting("SemVer version") + .string(|value| value.parse().map_err(serde::de::Error::custom)) + .deserialize(deserializer) } } @@ -142,6 +159,8 @@ pub struct PartialVersion { pub major: u64, pub minor: Option, pub patch: Option, + pub pre: Option, + pub build: Option, } impl PartialVersion { @@ -152,26 +171,31 @@ impl PartialVersion { major: self.major, minor: self.minor, patch: self.patch, - pre: Default::default(), + pre: self.pre.as_ref().cloned().unwrap_or_default(), }], } } } -impl TryFrom for PartialVersion { - type Error = anyhow::Error; - fn try_from(value: semver::Version) -> Result { - if !value.pre.is_empty() { - anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"") - } - if !value.build.is_empty() { - anyhow::bail!("unexpected build field, expected a version like \"1.32\"") +impl From for PartialVersion { + fn from(ver: semver::Version) -> Self { + let pre = if ver.pre.is_empty() { + None + } else { + Some(ver.pre) + }; + let build = if ver.build.is_empty() { + None + } else { + Some(ver.build) + }; + Self { + major: ver.major, + minor: Some(ver.minor), + patch: Some(ver.patch), + pre, + build, } - Ok(Self { - major: value.major, - minor: Some(value.minor), - patch: Some(value.patch), - }) } } @@ -183,7 +207,7 @@ impl std::str::FromStr for PartialVersion { anyhow::bail!("unexpected version requirement, expected a version like \"1.32\"") } match semver::Version::parse(value) { - Ok(ver) => ver.try_into(), + Ok(ver) => Ok(ver.into()), Err(_) => { // HACK: Leverage `VersionReq` for partial version parsing let mut version_req = match semver::VersionReq::parse(value) { @@ -201,15 +225,17 @@ impl std::str::FromStr for PartialVersion { assert_eq!(version_req.comparators.len(), 1, "guarenteed by is_req"); let comp = version_req.comparators.pop().unwrap(); assert_eq!(comp.op, semver::Op::Caret, "guarenteed by is_req"); - assert_eq!( - comp.pre, - semver::Prerelease::EMPTY, - "guarenteed by `Version::parse` failing" - ); + let pre = if comp.pre.is_empty() { + None + } else { + Some(comp.pre) + }; Ok(Self { major: comp.major, minor: comp.minor, patch: comp.patch, + pre, + build: None, }) } } @@ -226,6 +252,12 @@ impl Display for PartialVersion { if let Some(patch) = self.patch { write!(f, ".{patch}")?; } + if let Some(pre) = self.pre.as_ref() { + write!(f, "-{pre}")?; + } + if let Some(build) = self.build.as_ref() { + write!(f, "+{build}")?; + } Ok(()) } } From 396298200d85911885612782474101ce99f0247d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 15:07:05 -0500 Subject: [PATCH 7/9] test(profile): Verify spec version parsing --- tests/testsuite/profile_overrides.rs | 66 ++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/testsuite/profile_overrides.rs b/tests/testsuite/profile_overrides.rs index dc9bafba184..34e53bf0c4c 100644 --- a/tests/testsuite/profile_overrides.rs +++ b/tests/testsuite/profile_overrides.rs @@ -267,6 +267,72 @@ found package specs: bar, bar@0.5.0", .run(); } +#[cargo_test] +fn profile_override_spec_with_version() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + bar = { path = "bar" } + + [profile.dev.package."bar:0.5.0"] + codegen-units = 2 + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("check -v") + .with_stderr_contains("[RUNNING] `rustc [..]bar/src/lib.rs [..] -C codegen-units=2 [..]") + .run(); +} + +#[cargo_test] +fn profile_override_spec_with_partial_version() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + bar = { path = "bar" } + + [profile.dev.package."bar:0.5"] + codegen-units = 2 + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("check -v") + .with_status(101) + .with_stderr_contains( + "\ +error: failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + TOML parse error at line 9, column 34 + | + 9 | [profile.dev.package.\"bar:0.5\"] + | ^^^^^^^^^ + cannot parse '0.5' as a SemVer version +", + ) + .run(); +} + #[cargo_test] fn profile_override_spec() { let p = project() From 9008647ae28b8ba7bf6390f85aba0daa58f73827 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 15:30:13 -0500 Subject: [PATCH 8/9] test(clean): Verify spec version parsing --- tests/testsuite/clean.rs | 106 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 99 insertions(+), 7 deletions(-) diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index 65f6cf47616..bf2df9cc60e 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -569,10 +569,10 @@ fn assert_all_clean(build_dir: &Path) { } #[cargo_test] -fn clean_spec_multiple() { +fn clean_spec_version() { // clean -p foo where foo matches multiple versions - Package::new("bar", "1.0.0").publish(); - Package::new("bar", "2.0.0").publish(); + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.2.0").publish(); let p = project() .file( @@ -583,8 +583,8 @@ fn clean_spec_multiple() { version = "0.1.0" [dependencies] - bar1 = {version="1.0", package="bar"} - bar2 = {version="2.0", package="bar"} + bar1 = {version="0.1", package="bar"} + bar2 = {version="0.2", package="bar"} "#, ) .file("src/lib.rs", "") @@ -604,9 +604,9 @@ error: package ID specification `baz` did not match any packages ) .run(); - p.cargo("clean -p bar:1.0.0") + p.cargo("clean -p bar:0.1.0") .with_stderr( - "warning: version qualifier in `-p bar:1.0.0` is ignored, \ + "warning: version qualifier in `-p bar:0.1.0` is ignored, \ cleaning all versions of `bar` found", ) .run(); @@ -622,6 +622,98 @@ error: package ID specification `baz` did not match any packages } } +#[cargo_test] +fn clean_spec_partial_version() { + // clean -p foo where foo matches multiple versions + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.2.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar1 = {version="0.1", package="bar"} + bar2 = {version="0.2", package="bar"} + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build").run(); + + // Check suggestion for bad pkgid. + p.cargo("clean -p baz") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `baz` did not match any packages + +Did you mean `bar`? +", + ) + .run(); + + p.cargo("clean -p bar:0.1") + .with_status(101) + .with_stderr( + "\ +error: cannot parse '0.1' as a SemVer version +", + ) + .run(); +} + +#[cargo_test] +fn clean_spec_partial_version_ambiguous() { + // clean -p foo where foo matches multiple versions + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.2.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar1 = {version="0.1", package="bar"} + bar2 = {version="0.2", package="bar"} + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build").run(); + + // Check suggestion for bad pkgid. + p.cargo("clean -p baz") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `baz` did not match any packages + +Did you mean `bar`? +", + ) + .run(); + + p.cargo("clean -p bar:0") + .with_status(101) + .with_stderr( + "\ +error: cannot parse '0' as a SemVer version +", + ) + .run(); +} + #[cargo_test] fn clean_spec_reserved() { // Clean when a target (like a test) has a reserved name. In this case, From 82f9bd330e2b6149d9d9dc6adfd2336485b40718 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Sep 2023 13:30:58 -0500 Subject: [PATCH 9/9] feat(spec): Allow partial versions when unambigious This was proposed in #12425 to help improve usability of the existing `cargo update` when dealing with the added workflows. --- src/bin/cargo/commands/remove.rs | 2 +- src/cargo/core/package_id_spec.rs | 67 +++++++++++++++++++++------- src/cargo/ops/cargo_clean.rs | 2 +- src/cargo/util/mod.rs | 2 +- src/cargo/util/semver_ext.rs | 22 +++++++++ src/cargo/util/toml/mod.rs | 4 +- src/doc/src/reference/pkgid-spec.md | 2 + tests/testsuite/clean.rs | 32 +++++++++---- tests/testsuite/pkgid.rs | 18 +++----- tests/testsuite/profile_overrides.rs | 14 +----- 10 files changed, 110 insertions(+), 55 deletions(-) diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index aebb2f11cab..c6508a6b2d0 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -288,7 +288,7 @@ fn spec_has_match( } let version_matches = match (spec.version(), dep.version()) { - (Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(v), + (Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(&v), (Some(_), None) => false, (None, None | Some(_)) => true, }; diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index ec5918b0f85..b90d281bd65 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -10,7 +10,8 @@ use crate::core::PackageId; use crate::util::edit_distance; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{validate_package_name, IntoUrl, ToSemver}; +use crate::util::PartialVersion; +use crate::util::{validate_package_name, IntoUrl}; /// Some or all of the data required to identify a package: /// @@ -24,7 +25,7 @@ use crate::util::{validate_package_name, IntoUrl, ToSemver}; #[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] pub struct PackageIdSpec { name: InternedString, - version: Option, + version: Option, url: Option, } @@ -70,7 +71,7 @@ impl PackageIdSpec { let mut parts = spec.splitn(2, [':', '@']); let name = parts.next().unwrap(); let version = match parts.next() { - Some(version) => Some(version.to_semver()?), + Some(version) => Some(version.parse::()?), None => None, }; validate_package_name(name, "pkgid", "")?; @@ -94,12 +95,12 @@ impl PackageIdSpec { spec.query(i) } - /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `Version` and `Url` + /// 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: package_id.name(), - version: Some(package_id.version().clone()), + version: Some(package_id.version().clone().into()), url: Some(package_id.source_id().url().clone()), } } @@ -125,14 +126,14 @@ impl PackageIdSpec { match frag { Some(fragment) => match fragment.split_once([':', '@']) { Some((name, part)) => { - let version = part.to_semver()?; + let version = part.parse::()?; (InternedString::new(name), Some(version)) } None => { if fragment.chars().next().unwrap().is_alphabetic() { (InternedString::new(&fragment), None) } else { - let version = fragment.to_semver()?; + let version = fragment.parse::()?; (InternedString::new(path_name), Some(version)) } } @@ -151,7 +152,12 @@ impl PackageIdSpec { self.name } - pub fn version(&self) -> Option<&Version> { + /// Full `semver::Version`, if present + pub fn version(&self) -> Option { + self.version.as_ref().and_then(|v| v.version()) + } + + pub fn partial_version(&self) -> Option<&PartialVersion> { self.version.as_ref() } @@ -170,7 +176,8 @@ impl PackageIdSpec { } if let Some(ref v) = self.version { - if v != package_id.version() { + let req = v.exact_req(); + if !req.matches(package_id.version()) { return false; } } @@ -319,7 +326,6 @@ mod tests { use super::PackageIdSpec; use crate::core::{PackageId, SourceId}; use crate::util::interning::InternedString; - use crate::util::ToSemver; use url::Url; #[test] @@ -344,16 +350,25 @@ mod tests { "https://crates.io/foo#1.2.3", PackageIdSpec { name: InternedString::new("foo"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, "https://crates.io/foo#1.2.3", ); + ok( + "https://crates.io/foo#1.2", + PackageIdSpec { + name: InternedString::new("foo"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + }, + "https://crates.io/foo#1.2", + ); ok( "https://crates.io/foo#bar:1.2.3", PackageIdSpec { name: InternedString::new("bar"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, "https://crates.io/foo#bar@1.2.3", @@ -362,11 +377,20 @@ mod tests { "https://crates.io/foo#bar@1.2.3", PackageIdSpec { name: InternedString::new("bar"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: Some(Url::parse("https://crates.io/foo").unwrap()), }, "https://crates.io/foo#bar@1.2.3", ); + ok( + "https://crates.io/foo#bar@1.2", + PackageIdSpec { + name: InternedString::new("bar"), + version: Some("1.2".parse().unwrap()), + url: Some(Url::parse("https://crates.io/foo").unwrap()), + }, + "https://crates.io/foo#bar@1.2", + ); ok( "foo", PackageIdSpec { @@ -380,7 +404,7 @@ mod tests { "foo:1.2.3", PackageIdSpec { name: InternedString::new("foo"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: None, }, "foo@1.2.3", @@ -389,21 +413,29 @@ mod tests { "foo@1.2.3", PackageIdSpec { name: InternedString::new("foo"), - version: Some("1.2.3".to_semver().unwrap()), + version: Some("1.2.3".parse().unwrap()), url: None, }, "foo@1.2.3", ); + ok( + "foo@1.2", + PackageIdSpec { + name: InternedString::new("foo"), + version: Some("1.2".parse().unwrap()), + url: None, + }, + "foo@1.2", + ); } #[test] fn bad_parsing() { assert!(PackageIdSpec::parse("baz:").is_err()); assert!(PackageIdSpec::parse("baz:*").is_err()); - assert!(PackageIdSpec::parse("baz:1.0").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("baz@^1.0").is_err()); assert!(PackageIdSpec::parse("https://baz:1.0").is_err()); assert!(PackageIdSpec::parse("https://#baz:1.0").is_err()); } @@ -421,5 +453,6 @@ mod tests { assert!(!PackageIdSpec::parse("foo:1.2.2").unwrap().matches(foo)); 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)); } } diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index d28f759d707..e05ffecba95 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -108,7 +108,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { for spec_str in opts.spec.iter() { // Translate the spec to a Package. let spec = PackageIdSpec::parse(spec_str)?; - if spec.version().is_some() { + if spec.partial_version().is_some() { config.shell().warn(&format!( "version qualifier in `-p {}` is ignored, \ cleaning all versions of `{}` found", diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index cb9a518ed7e..5c0393f9f1d 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -22,7 +22,7 @@ pub use self::progress::{Progress, ProgressStyle}; pub use self::queue::Queue; pub use self::restricted_names::validate_package_name; pub use self::rustc::Rustc; -pub use self::semver_ext::{OptVersionReq, RustVersion, VersionExt, VersionReqExt}; +pub use self::semver_ext::{OptVersionReq, PartialVersion, RustVersion, VersionExt, VersionReqExt}; pub use self::to_semver::ToSemver; pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo}; pub use self::workspace::{ diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index b03473fb3fa..1ed12fbf519 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -164,6 +164,16 @@ pub struct PartialVersion { } impl PartialVersion { + pub fn version(&self) -> Option { + Some(Version { + major: self.major, + minor: self.minor?, + patch: self.patch?, + pre: self.pre.clone().unwrap_or_default(), + build: self.build.clone().unwrap_or_default(), + }) + } + pub fn caret_req(&self) -> VersionReq { VersionReq { comparators: vec![Comparator { @@ -175,6 +185,18 @@ impl PartialVersion { }], } } + + pub fn exact_req(&self) -> VersionReq { + VersionReq { + comparators: vec![Comparator { + op: semver::Op::Exact, + major: self.major, + minor: self.minor, + patch: self.patch, + pre: self.pre.as_ref().cloned().unwrap_or_default(), + }], + } + } } impl From for PartialVersion { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index c43621fc036..779618c331f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -2652,8 +2652,8 @@ impl TomlManifest { replacement.unused_keys(), &mut cx.warnings, ); - dep.set_version_req(OptVersionReq::exact(version)) - .lock_version(version); + dep.set_version_req(OptVersionReq::exact(&version)) + .lock_version(&version); replace.push((spec, dep)); } Ok(replace) diff --git a/src/doc/src/reference/pkgid-spec.md b/src/doc/src/reference/pkgid-spec.md index bf2e15c8c1f..7f20973b509 100644 --- a/src/doc/src/reference/pkgid-spec.md +++ b/src/doc/src/reference/pkgid-spec.md @@ -24,6 +24,7 @@ The formal grammar for a Package Id Specification is: spec := pkgname | proto "://" hostname-and-path [ "#" ( pkgname | semver ) ] pkgname := name [ ("@" | ":" ) semver ] +semver := digits [ "." digits [ "." digits [ "-" prerelease ] [ "+" build ]]] proto := "http" | "git" | ... ``` @@ -40,6 +41,7 @@ The following are references to the `regex` package on `crates.io`: | Spec | Name | Version | |:------------------------------------------------------------|:-------:|:-------:| | `regex` | `regex` | `*` | +| `regex@1.4` | `regex` | `1.4.*` | | `regex@1.4.3` | `regex` | `1.4.3` | | `https://github.com/rust-lang/crates.io-index#regex` | `regex` | `*` | | `https://github.com/rust-lang/crates.io-index#regex@1.4.3` | `regex` | `1.4.3` | diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index bf2df9cc60e..26cc11a4a2a 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -659,13 +659,21 @@ error: package ID specification `baz` did not match any packages .run(); p.cargo("clean -p bar:0.1") - .with_status(101) .with_stderr( - "\ -error: cannot parse '0.1' as a SemVer version -", + "warning: version qualifier in `-p bar:0.1` is ignored, \ + cleaning all versions of `bar` found", ) .run(); + let mut walker = walkdir::WalkDir::new(p.build_dir()) + .into_iter() + .filter_map(|e| e.ok()) + .filter(|e| { + let n = e.file_name().to_str().unwrap(); + n.starts_with("bar") || n.starts_with("libbar") + }); + if let Some(e) = walker.next() { + panic!("{:?} was not cleaned", e.path()); + } } #[cargo_test] @@ -705,13 +713,21 @@ error: package ID specification `baz` did not match any packages .run(); p.cargo("clean -p bar:0") - .with_status(101) .with_stderr( - "\ -error: cannot parse '0' as a SemVer version -", + "warning: version qualifier in `-p bar:0` is ignored, \ + cleaning all versions of `bar` found", ) .run(); + let mut walker = walkdir::WalkDir::new(p.build_dir()) + .into_iter() + .filter_map(|e| e.ok()) + .filter(|e| { + let n = e.file_name().to_str().unwrap(); + n.starts_with("bar") || n.starts_with("libbar") + }); + if let Some(e) = walker.next() { + panic!("{:?} was not cleaned", e.path()); + } } #[cargo_test] diff --git a/tests/testsuite/pkgid.rs b/tests/testsuite/pkgid.rs index 1857f43b185..944827e06d0 100644 --- a/tests/testsuite/pkgid.rs +++ b/tests/testsuite/pkgid.rs @@ -151,25 +151,19 @@ fn multiple_versions() { .with_status(101) .with_stderr( "\ -error: invalid package ID specification: `two-ver@0` - -Did you mean `two-ver`? - -Caused by: - cannot parse '0' as a SemVer version +error: There are multiple `two-ver` packages in your project, and the specification `two-ver@0` is ambiguous. +Please re-run this command with `-p ` where `` is one of the following: + two-ver@0.1.0 + two-ver@0.2.0 ", ) .run(); // Incomplete version. p.cargo("pkgid two-ver@0.2") - .with_status(101) - .with_stderr( + .with_stdout( "\ -error: invalid package ID specification: `two-ver@0.2` - -Caused by: - cannot parse '0.2' as a SemVer version +https://github.com/rust-lang/crates.io-index#two-ver@0.2.0 ", ) .run(); diff --git a/tests/testsuite/profile_overrides.rs b/tests/testsuite/profile_overrides.rs index 34e53bf0c4c..65d9f3b5192 100644 --- a/tests/testsuite/profile_overrides.rs +++ b/tests/testsuite/profile_overrides.rs @@ -317,19 +317,7 @@ fn profile_override_spec_with_partial_version() { .build(); p.cargo("check -v") - .with_status(101) - .with_stderr_contains( - "\ -error: failed to parse manifest at `[CWD]/Cargo.toml` - -Caused by: - TOML parse error at line 9, column 34 - | - 9 | [profile.dev.package.\"bar:0.5\"] - | ^^^^^^^^^ - cannot parse '0.5' as a SemVer version -", - ) + .with_stderr_contains("[RUNNING] `rustc [..]bar/src/lib.rs [..] -C codegen-units=2 [..]") .run(); }