From ebbd6a2d20d6d02266c4e57ddfa46ca8c39930be Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 28 Feb 2024 16:36:02 +0000 Subject: [PATCH] Split CrateID CrateNameAndVersionReq (#2525) Currently we use the same CrateId type for two contexts: A known crate name + version, and in an annotation to specify a version requirement that an annotation applies to. These are different use-cases, and neither should use String to represent a the version. Also, update CrateId to use a `semver::Version` (which is required, not optional). --- crate_universe/src/config.rs | 298 ++++++++++++++---- crate_universe/src/context.rs | 15 +- crate_universe/src/context/crate_context.rs | 28 +- crate_universe/src/context/platforms.rs | 70 +++- crate_universe/src/lockfile.rs | 4 +- crate_universe/src/metadata.rs | 30 +- .../src/metadata/metadata_annotation.rs | 20 +- crate_universe/src/rendering.rs | 221 +++++++++++-- crate_universe/src/splicing.rs | 2 +- crate_universe/src/splicing/splicer.rs | 3 +- 10 files changed, 537 insertions(+), 154 deletions(-) diff --git a/crate_universe/src/config.rs b/crate_universe/src/config.rs index 46f075c6f4..ee926b05db 100644 --- a/crate_universe/src/config.rs +++ b/crate_universe/src/config.rs @@ -1,18 +1,21 @@ //! A module for configuration information +use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; use std::convert::AsRef; +use std::fmt::Formatter; use std::iter::Sum; use std::ops::Add; use std::path::Path; +use std::str::FromStr; use std::{fmt, fs}; -use anyhow::Result; +use anyhow::{Context, Result}; use cargo_lock::package::GitReference; use cargo_metadata::Package; use semver::VersionReq; use serde::de::value::SeqAccessDeserializer; -use serde::de::{Deserializer, SeqAccess, Visitor}; +use serde::de::{Deserializer, SeqAccess, Unexpected, Visitor}; use serde::{Deserialize, Serialize, Serializer}; use crate::select::{Select, Selectable}; @@ -516,55 +519,21 @@ pub struct CrateId { pub name: String, /// The crate's semantic version - pub version: String, + pub version: semver::Version, } impl CrateId { /// Construct a new [CrateId] - pub fn new(name: String, version: String) -> Self { + pub fn new(name: String, version: semver::Version) -> Self { Self { name, version } } - - /// Compares a [CrateId] against a [cargo_metadata::Package]. - pub fn matches(&self, package: &Package) -> bool { - // If the package name does not match, it's obviously - // not the right package - if self.name != "*" && self.name != package.name { - return false; - } - - // First see if the package version matches exactly - if package.version.to_string() == self.version { - return true; - } - - // If the version provided is the wildcard "*", it matches. Do not - // delegate to the semver crate in this case because semver does not - // consider "*" to match prerelease packages. That's expected behavior - // in the context of declaring package dependencies, but not in the - // context of declaring which versions of preselected packages an - // annotation applies to. - if self.version == "*" { - return true; - } - - // Next, check to see if the version provided is a semver req and - // check if the package matches the condition - if let Ok(semver) = VersionReq::parse(&self.version) { - if semver.matches(&package.version) { - return true; - } - } - - false - } } impl From<&Package> for CrateId { fn from(package: &Package) -> Self { Self { name: package.name.clone(), - version: package.version.to_string(), + version: package.version.clone(), } } } @@ -590,16 +559,20 @@ impl<'de> Visitor<'de> for CrateIdVisitor { where E: serde::de::Error, { - v.rsplit_once(' ') - .map(|(name, version)| CrateId { - name: name.to_string(), - version: version.to_string(), - }) - .ok_or_else(|| { - E::custom(format!( - "Expected string value of `{{name}} {{version}}`. Got '{v}'" - )) - }) + let (name, version_str) = v.rsplit_once(' ').ok_or_else(|| { + E::custom(format!( + "Expected string value of `{{name}} {{version}}`. Got '{v}'" + )) + })?; + let version = semver::Version::parse(version_str).map_err(|err| { + E::custom(format!( + "Couldn't parse {version_str} as a semver::Version: {err}" + )) + })?; + Ok(CrateId { + name: name.to_string(), + version, + }) } } @@ -688,7 +661,7 @@ pub struct Config { /// Additional settings to apply to generated crates #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub annotations: BTreeMap, + pub annotations: BTreeMap, /// Settings used to determine various render info pub rendering: RenderConfig, @@ -708,6 +681,178 @@ impl Config { } } +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub struct CrateNameAndVersionReq { + /// The name of the crate + pub name: String, + + version_req_string: VersionReqString, +} + +impl Serialize for CrateNameAndVersionReq { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&format!( + "{} {}", + self.name, self.version_req_string.original + )) + } +} + +struct CrateNameAndVersionReqVisitor; +impl<'de> Visitor<'de> for CrateNameAndVersionReqVisitor { + type Value = CrateNameAndVersionReq; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("Expected string value of `{name} {version}`.") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + let (name, version) = v.rsplit_once(' ').ok_or_else(|| { + E::custom(format!( + "Expected string value of `{{name}} {{version}}`. Got '{v}'" + )) + })?; + version + .parse() + .map(|version| CrateNameAndVersionReq { + name: name.to_string(), + version_req_string: version, + }) + .map_err(|err| E::custom(err.to_string())) + } +} + +impl<'de> Deserialize<'de> for CrateNameAndVersionReq { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_str(CrateNameAndVersionReqVisitor) + } +} + +/// A version requirement (i.e. a semver::VersionReq) which preserves the original string it was parsed from. +/// This means that you can report back to the user whether they wrote `1` or `1.0.0` or `^1.0.0` or `>=1,<2`, +/// and support exact round-trip serialization and deserialization. +#[derive(Clone, Debug)] +pub struct VersionReqString { + original: String, + + parsed: VersionReq, +} + +impl FromStr for VersionReqString { + type Err = anyhow::Error; + + fn from_str(original: &str) -> Result { + let parsed = VersionReq::parse(original) + .context("VersionReqString must be a valid semver requirement")?; + Ok(VersionReqString { + original: original.to_owned(), + parsed, + }) + } +} + +impl PartialEq for VersionReqString { + fn eq(&self, other: &Self) -> bool { + self.original == other.original + } +} + +impl Eq for VersionReqString {} + +impl PartialOrd for VersionReqString { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for VersionReqString { + fn cmp(&self, other: &Self) -> Ordering { + Ord::cmp(&self.original, &other.original) + } +} + +impl Serialize for VersionReqString { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: Serializer, + { + serializer.serialize_str(&self.original) + } +} + +impl<'de> Deserialize<'de> for VersionReqString { + fn deserialize(deserializer: D) -> std::result::Result + where + D: Deserializer<'de>, + { + struct StringVisitor; + + impl<'de> Visitor<'de> for StringVisitor { + type Value = String; + + fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { + formatter.write_str("string of a semver requirement") + } + } + + let original = deserializer.deserialize_str(StringVisitor)?; + let parsed = VersionReq::parse(&original).map_err(|_| { + serde::de::Error::invalid_value( + Unexpected::Str(&original), + &"a valid semver requirement", + ) + })?; + Ok(VersionReqString { original, parsed }) + } +} + +impl CrateNameAndVersionReq { + #[cfg(test)] + pub fn new(name: String, version_req_string: VersionReqString) -> CrateNameAndVersionReq { + CrateNameAndVersionReq { + name, + version_req_string, + } + } + + /// Compares a [CrateNameAndVersionReq] against a [cargo_metadata::Package]. + pub fn matches(&self, package: &Package) -> bool { + // If the package name does not match, it's obviously + // not the right package + if self.name != "*" && self.name != package.name { + return false; + } + + // First see if the package version matches exactly + if package.version.to_string() == self.version_req_string.original { + return true; + } + + // If the version provided is the wildcard "*", it matches. Do not + // delegate to the semver crate in this case because semver does not + // consider "*" to match prerelease packages. That's expected behavior + // in the context of declaring package dependencies, but not in the + // context of declaring which versions of preselected packages an + // annotation applies to. + if self.version_req_string.original == "*" { + return true; + } + + // Next, check to see if the version provided is a semver req and + // check if the package matches the condition + self.version_req_string.parsed.matches(&package.version) + } +} + #[cfg(test)] mod test { use super::*; @@ -717,21 +862,17 @@ mod test { #[test] fn test_crate_id_serde() { let id: CrateId = serde_json::from_str("\"crate 0.1.0\"").unwrap(); - assert_eq!(id, CrateId::new("crate".to_owned(), "0.1.0".to_owned())); + assert_eq!( + id, + CrateId::new("crate".to_owned(), semver::Version::new(0, 1, 0)) + ); assert_eq!(serde_json::to_string(&id).unwrap(), "\"crate 0.1.0\""); } - #[test] - fn test_crate_id_serde_semver() { - let semver_id: CrateId = serde_json::from_str("\"crate *\"").unwrap(); - assert_eq!(semver_id, CrateId::new("crate".to_owned(), "*".to_owned())); - assert_eq!(serde_json::to_string(&semver_id).unwrap(), "\"crate *\""); - } - #[test] fn test_crate_id_matches() { let mut package = mock_cargo_metadata_package(); - let id = CrateId::new("mock-pkg".to_owned(), "0.1.0".to_owned()); + let id = CrateNameAndVersionReq::new("mock-pkg".to_owned(), "0.1.0".parse().unwrap()); package.version = cargo_metadata::semver::Version::new(0, 1, 0); assert!(id.matches(&package)); @@ -741,19 +882,43 @@ mod test { } #[test] - fn test_crate_id_semver_matches() { + fn test_crate_name_and_version_req_serde() { + let id: CrateNameAndVersionReq = serde_json::from_str("\"crate 0.1.0\"").unwrap(); + assert_eq!( + id, + CrateNameAndVersionReq::new( + "crate".to_owned(), + VersionReqString::from_str("0.1.0").unwrap() + ) + ); + assert_eq!(serde_json::to_string(&id).unwrap(), "\"crate 0.1.0\""); + } + + #[test] + fn test_crate_name_and_version_req_serde_semver() { + let id: CrateNameAndVersionReq = serde_json::from_str("\"crate *\"").unwrap(); + assert_eq!( + id, + CrateNameAndVersionReq::new( + "crate".to_owned(), + VersionReqString::from_str("*").unwrap() + ) + ); + assert_eq!(serde_json::to_string(&id).unwrap(), "\"crate *\""); + } + + #[test] + fn test_crate_name_and_version_req_semver_matches() { let mut package = mock_cargo_metadata_package(); package.version = cargo_metadata::semver::Version::new(1, 0, 0); - let mut id = CrateId::new("mock-pkg".to_owned(), "0.1.0".to_owned()); - - id.version = "*".to_owned(); + let id = CrateNameAndVersionReq::new("mock-pkg".to_owned(), "*".parse().unwrap()); assert!(id.matches(&package)); let mut prerelease = mock_cargo_metadata_package(); prerelease.version = cargo_metadata::semver::Version::parse("1.0.0-pre.0").unwrap(); assert!(id.matches(&prerelease)); - id.version = "<1".to_owned(); + let id = CrateNameAndVersionReq::new("mock-pkg".to_owned(), "<1".parse().unwrap()); assert!(!id.matches(&package)); } @@ -770,7 +935,10 @@ mod test { // Annotations let annotation = config .annotations - .get(&CrateId::new("rand".to_owned(), "0.8.5".to_owned())) + .get(&CrateNameAndVersionReq::new( + "rand".to_owned(), + "0.8.5".parse().unwrap(), + )) .unwrap(); assert_eq!( annotation.crate_features, diff --git a/crate_universe/src/context.rs b/crate_universe/src/context.rs index dc9ea80fa2..386b5b2816 100644 --- a/crate_universe/src/context.rs +++ b/crate_universe/src/context.rs @@ -228,6 +228,7 @@ impl Context { #[cfg(test)] mod test { use super::*; + use semver::Version; use crate::config::Config; @@ -264,8 +265,8 @@ mod test { .map(|dep| (&dep.id, context.has_duplicate_workspace_member_dep(dep))) .collect::>(), [ - (&CrateId::new("bitflags".to_owned(), "1.3.2".to_owned()), false), - (&CrateId::new("cfg-if".to_owned(), "1.0.0".to_owned()), false), + (&CrateId::new("bitflags".to_owned(), Version::new(1, 3, 2)), false), + (&CrateId::new("cfg-if".to_owned(), Version::new(1, 0, 0)), false), ], } } @@ -281,11 +282,11 @@ mod test { .map(|dep| (&dep.id, context.has_duplicate_workspace_member_dep(dep))) .collect::>(), [ - (&CrateId::new("log".to_owned(), "0.3.9".to_owned()), false), - (&CrateId::new("log".to_owned(), "0.4.14".to_owned()), false), - (&CrateId::new("names".to_owned(), "0.12.1-dev".to_owned()), false), - (&CrateId::new("names".to_owned(), "0.13.0".to_owned()), false), - (&CrateId::new("value-bag".to_owned(), "1.0.0-alpha.7".to_owned()), false), + (&CrateId::new("log".to_owned(), Version::new(0, 3, 9)), false), + (&CrateId::new("log".to_owned(), Version::new(0, 4, 14)), false), + (&CrateId::new("names".to_owned(), Version::parse("0.12.1-dev").unwrap()), false), + (&CrateId::new("names".to_owned(), Version::new(0, 13, 0)), false), + (&CrateId::new("value-bag".to_owned(), Version::parse("1.0.0-alpha.7").unwrap()), false), ], } } diff --git a/crate_universe/src/context/crate_context.rs b/crate_universe/src/context/crate_context.rs index 7eaf8cb97f..c36685d721 100644 --- a/crate_universe/src/context/crate_context.rs +++ b/crate_universe/src/context/crate_context.rs @@ -242,61 +242,73 @@ impl Default for BuildScriptAttributes { } } -#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(default)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CrateContext { /// The package name of the current crate pub name: String, /// The full version of the current crate - pub version: String, + pub version: semver::Version, /// The package URL of the current crate + #[serde(default)] pub package_url: Option, /// Optional source annotations if they were discoverable in the /// lockfile. Workspace Members will not have source annotations and /// potentially others. + #[serde(default)] pub repository: Option, /// A list of all targets (lib, proc-macro, bin) associated with this package + #[serde(default)] pub targets: BTreeSet, /// The name of the crate's root library target. This is the target that a dependent /// would get if they were to depend on `{crate_name}`. + #[serde(default)] pub library_target_name: Option, /// A set of attributes common to most [Rule] types or target types. + #[serde(default)] pub common_attrs: CommonAttributes, /// Optional attributes for build scripts. This field is only populated if /// a build script (`custom-build`) target is defined for the crate. #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub build_script_attrs: Option, /// The license used by the crate + #[serde(default)] pub license: Option, /// The SPDX licence IDs + /// #[serde(default)] pub license_ids: BTreeSet, - // The license file + /// The license file + #[serde(default)] pub license_file: Option, /// Additional text to add to the generated BUILD file. #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub additive_build_file_content: Option, /// If true, disables pipelining for library targets generated for this crate #[serde(skip_serializing_if = "std::ops::Not::not")] + #[serde(default)] pub disable_pipelining: bool, /// Extra targets that should be aliased. #[serde(skip_serializing_if = "BTreeMap::is_empty")] + #[serde(default)] pub extra_aliased_targets: BTreeMap, /// Transition rule to use instead of `alias`. #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] pub alias_rule: Option, } @@ -311,7 +323,7 @@ impl CrateContext { include_build_scripts: bool, ) -> Self { let package: &Package = &packages[&annotation.node.id]; - let current_crate_id = CrateId::new(package.name.clone(), package.version.to_string()); + let current_crate_id = CrateId::new(package.name.clone(), package.version.clone()); let new_crate_dep = |dep: Dependency| -> CrateDependency { let pkg = &packages[&dep.package_id]; @@ -322,7 +334,7 @@ impl CrateContext { let target = sanitize_module_name(&dep.target_name); CrateDependency { - id: CrateId::new(pkg.name.clone(), pkg.version.to_string()), + id: CrateId::new(pkg.name.clone(), pkg.version.clone()), target, alias: dep.alias, } @@ -460,7 +472,7 @@ impl CrateContext { // Create the crate's context and apply extra settings CrateContext { name: package.name.clone(), - version: package.version.to_string(), + version: package.version.clone(), license: package.license.clone(), license_ids, license_file, @@ -828,7 +840,7 @@ mod test { let mut pairred_extras = BTreeMap::new(); pairred_extras.insert( - CrateId::new("common".to_owned(), "0.1.0".to_owned()), + CrateId::new("common".to_owned(), semver::Version::new(0, 1, 0)), PairedExtras { package_id, crate_extra: CrateAnnotations { diff --git a/crate_universe/src/context/platforms.rs b/crate_universe/src/context/platforms.rs index f3bc160edd..52ed589d34 100644 --- a/crate_universe/src/context/platforms.rs +++ b/crate_universe/src/context/platforms.rs @@ -117,6 +117,8 @@ mod test { use super::*; + const VERSION_ZERO_ONE_ZERO: semver::Version = semver::Version::new(0, 1, 0); + fn supported_platform_triples() -> BTreeSet { BTreeSet::from([ TargetTriple::from_bazel("aarch64-apple-darwin".to_owned()), @@ -130,7 +132,7 @@ mod test { let mut deps: Select> = Select::default(); deps.insert( CrateDependency { - id: CrateId::new("mock_crate_b".to_owned(), "0.1.0".to_owned()), + id: CrateId::new("mock_crate_b".to_owned(), VERSION_ZERO_ONE_ZERO), target: "mock_crate_b".to_owned(), alias: None, }, @@ -139,12 +141,23 @@ mod test { let context = CrateContext { name: "mock_crate_a".to_owned(), - version: "0.1.0".to_owned(), + version: VERSION_ZERO_ONE_ZERO, + package_url: None, + repository: None, + targets: BTreeSet::default(), + library_target_name: None, common_attrs: CommonAttributes { deps, ..CommonAttributes::default() }, - ..CrateContext::default() + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }; let configurations = @@ -176,7 +189,7 @@ mod test { let mut deps: Select> = Select::default(); deps.insert( CrateDependency { - id: CrateId::new("mock_crate_b".to_owned(), "0.1.0".to_owned()), + id: CrateId::new("mock_crate_b".to_owned(), VERSION_ZERO_ONE_ZERO), target: "mock_crate_b".to_owned(), alias: None, }, @@ -185,12 +198,23 @@ mod test { CrateContext { name: "mock_crate_a".to_owned(), - version: "0.1.0".to_owned(), + version: VERSION_ZERO_ONE_ZERO, + package_url: None, + repository: None, + targets: BTreeSet::default(), + library_target_name: None, common_attrs: CommonAttributes { deps, ..CommonAttributes::default() }, - ..CrateContext::default() + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, } } @@ -250,7 +274,7 @@ mod test { let mut deps: Select> = Select::default(); deps.insert( CrateDependency { - id: CrateId::new("mock_crate_b".to_owned(), "0.1.0".to_owned()), + id: CrateId::new("mock_crate_b".to_owned(), VERSION_ZERO_ONE_ZERO), target: "mock_crate_b".to_owned(), alias: None, }, @@ -259,12 +283,23 @@ mod test { let context = CrateContext { name: "mock_crate_a".to_owned(), - version: "0.1.0".to_owned(), + version: VERSION_ZERO_ONE_ZERO, + package_url: None, + repository: None, + targets: BTreeSet::default(), + library_target_name: None, common_attrs: CommonAttributes { deps, ..CommonAttributes::default() }, - ..CrateContext::default() + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }; let configurations = @@ -304,7 +339,7 @@ mod test { let mut deps: Select> = Select::default(); deps.insert( CrateDependency { - id: CrateId::new("mock_crate_b".to_owned(), "0.1.0".to_owned()), + id: CrateId::new("mock_crate_b".to_owned(), VERSION_ZERO_ONE_ZERO), target: "mock_crate_b".to_owned(), alias: None, }, @@ -313,12 +348,23 @@ mod test { let context = CrateContext { name: "mock_crate_a".to_owned(), - version: "0.1.0".to_owned(), + version: VERSION_ZERO_ONE_ZERO, + package_url: None, + repository: None, + targets: BTreeSet::default(), + library_target_name: None, common_attrs: CommonAttributes { deps, ..CommonAttributes::default() }, - ..CrateContext::default() + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }; let configurations = diff --git a/crate_universe/src/lockfile.rs b/crate_universe/src/lockfile.rs index 3eaae23c8e..7109e4f010 100644 --- a/crate_universe/src/lockfile.rs +++ b/crate_universe/src/lockfile.rs @@ -187,7 +187,7 @@ impl PartialEq for Digest { #[cfg(test)] mod test { - use crate::config::{CrateAnnotations, CrateId}; + use crate::config::{CrateAnnotations, CrateNameAndVersionReq}; use crate::splicing::cargo_config::{AdditionalRegistry, CargoConfig, Registry}; use crate::utils::target_triple::TargetTriple; @@ -223,7 +223,7 @@ mod test { generate_binaries: false, generate_build_scripts: false, annotations: BTreeMap::from([( - CrateId::new("rustonomicon".to_owned(), "1.0.0".to_owned()), + CrateNameAndVersionReq::new("rustonomicon".to_owned(), "1.0.0".parse().unwrap()), CrateAnnotations { compile_data_glob: Some(BTreeSet::from(["arts/**".to_owned()])), ..CrateAnnotations::default() diff --git a/crate_universe/src/metadata.rs b/crate_universe/src/metadata.rs index a7b5a32c1e..798a7f4e42 100644 --- a/crate_universe/src/metadata.rs +++ b/crate_universe/src/metadata.rs @@ -604,18 +604,14 @@ where parts[1] ); } - let crate_id = CrateId::new( - crate_id_parts[0].to_owned(), - crate_id_parts[1] - .strip_prefix('v') - .ok_or_else(|| { - anyhow!( - "Unexpected crate version '{}' when parsing 'cargo tree' output.", - crate_id_parts[1] - ) - })? - .to_owned(), - ); + let version_str = crate_id_parts[1].strip_prefix('v').ok_or_else(|| { + anyhow!( + "Unexpected crate version '{}' when parsing 'cargo tree' output.", + crate_id_parts[1] + ) + })?; + let version = Version::parse(version_str).context("Failed to parse version")?; + let crate_id = CrateId::new(crate_id_parts[0].to_owned(), version); let mut features = if parts[2].is_empty() { BTreeSet::new() } else { @@ -746,35 +742,35 @@ mod test { ( CrateId { name: "multi_cfg_dep".to_owned(), - version: "0.1.0".to_owned() + version: Version::new(0, 1, 0), }, BTreeSet::from([]) ), ( CrateId { name: "cpufeatures".to_owned(), - version: "0.2.1".to_owned() + version: Version::new(0, 2, 1), }, BTreeSet::from([]) ), ( CrateId { name: "libc".to_owned(), - version: "0.2.117".to_owned() + version: Version::new(0, 2, 117), }, BTreeSet::from(["default".to_owned(), "std".to_owned()]) ), ( CrateId { name: "serde_derive".to_owned(), - version: "1.0.152".to_owned() + version: Version::new(1, 0, 152), }, BTreeSet::from([]) ), ( CrateId { name: "chrono".to_owned(), - version: "0.4.24".to_owned() + version: Version::new(0, 4, 24), }, BTreeSet::from(["default".to_owned(), "std".to_owned(), "serde".to_owned()]) ), diff --git a/crate_universe/src/metadata/metadata_annotation.rs b/crate_universe/src/metadata/metadata_annotation.rs index f1f957cb70..9d58e77f2d 100644 --- a/crate_universe/src/metadata/metadata_annotation.rs +++ b/crate_universe/src/metadata/metadata_annotation.rs @@ -306,7 +306,7 @@ impl LockfileAnnotation { package: &cargo_lock::Package, metadata: &WorkspaceMetadata, ) -> Option { - let crate_id = CrateId::new(package.name.to_string(), package.version.to_string()); + let crate_id = CrateId::new(package.name.to_string(), package.version.clone()); metadata.sources.get(&crate_id).cloned() } @@ -406,7 +406,7 @@ impl Annotations { None } else { Some(( - CrateId::new(pkg.name.clone(), pkg.version.to_string()), + CrateId::new(pkg.name.clone(), pkg.version.clone()), PairedExtras { package_id: pkg_id.clone(), crate_extra, @@ -448,7 +448,7 @@ fn is_workspace_member(id: &PackageId, cargo_metadata: &CargoMetadata) -> bool { if cargo_metadata.workspace_members.contains(id) { if let Some(data) = find_workspace_metadata(cargo_metadata) { let pkg = &cargo_metadata[id]; - let crate_id = CrateId::new(pkg.name.clone(), pkg.version.to_string()); + let crate_id = CrateId::new(pkg.name.clone(), pkg.version.clone()); !data.sources.contains_key(&crate_id) } else { @@ -472,6 +472,7 @@ fn cargo_meta_pkg_to_locked_pkg<'a>( #[cfg(test)] mod test { use super::*; + use crate::config::CrateNameAndVersionReq; use crate::test::*; @@ -575,7 +576,7 @@ mod test { // Create a config with some random annotation let mut config = Config::default(); config.annotations.insert( - CrateId::new("mock-crate".to_owned(), "0.1.0".to_owned()), + CrateNameAndVersionReq::new("mock-crate".to_owned(), "0.1.0".parse().unwrap()), CrateAnnotations::default(), ); @@ -589,7 +590,14 @@ mod test { #[test] fn defaults_from_package_metadata() { - let crate_id = CrateId::new("has_package_metadata".to_owned(), "0.0.0".to_owned()); + let crate_id = CrateId::new( + "has_package_metadata".to_owned(), + semver::Version::new(0, 0, 0), + ); + let crate_name_and_version_req = CrateNameAndVersionReq::new( + "has_package_metadata".to_owned(), + "0.0.0".parse().unwrap(), + ); let annotations = CrateAnnotations { rustc_env: Some(Select::from_value(BTreeMap::from([( "BAR".to_owned(), @@ -601,7 +609,7 @@ mod test { let mut config = Config::default(); config .annotations - .insert(crate_id.clone(), annotations.clone()); + .insert(crate_name_and_version_req, annotations.clone()); // Combine the above annotations with default values provided by the // crate author in package metadata. diff --git a/crate_universe/src/rendering.rs b/crate_universe/src/rendering.rs index f88667ea64..b1b6950aa8 100644 --- a/crate_universe/src/rendering.rs +++ b/crate_universe/src/rendering.rs @@ -192,7 +192,11 @@ impl Renderer { } else { rename.clone() }, - actual: self.crate_label(&krate.name, &krate.version, library_target_name), + actual: self.crate_label( + &krate.name, + &krate.version.to_string(), + library_target_name, + ), tags: BTreeSet::from(["manual".to_owned()]), }); } @@ -201,7 +205,7 @@ impl Renderer { dependencies.push(Alias { rule: alias_rule.rule(), name: alias.clone(), - actual: self.crate_label(&krate.name, &krate.version, target), + actual: self.crate_label(&krate.name, &krate.version.to_string(), target), tags: BTreeSet::from(["manual".to_owned()]), }); } @@ -242,7 +246,7 @@ impl Renderer { }, actual: self.crate_label( &krate.name, - &krate.version, + &krate.version.to_string(), &format!("{}__bin", bin.crate_name), ), tags: BTreeSet::from(["manual".to_owned()]), @@ -277,7 +281,7 @@ impl Renderer { let label = match render_build_file_template( &self.config.build_file_template, &id.name, - &id.version, + &id.version.to_string(), ) { Ok(label) => label, Err(e) => bail!(e), @@ -342,7 +346,7 @@ impl Renderer { name: "package_info".to_owned(), package_name: krate.name.clone(), package_url: krate.package_url.clone().unwrap_or_default(), - package_version: krate.version.clone(), + package_version: krate.version.to_string(), })); if has_license_ids { @@ -715,7 +719,7 @@ impl Renderer { if let Some(alias) = &dependency.alias { let label = self.crate_label( &dependency.id.name, - &dependency.id.version, + &dependency.id.version.to_string(), &dependency.target, ); aliases.insert((label, alias.clone()), configuration.clone()); @@ -731,7 +735,9 @@ impl Renderer { extra_deps: Select>, ) -> Select> { Select::merge( - deps.map(|dep| self.crate_label(&dep.id.name, &dep.id.version, &dep.target)), + deps.map(|dep| { + self.crate_label(&dep.id.name, &dep.id.version.to_string(), &dep.target) + }), extra_deps, ) } @@ -901,6 +907,8 @@ mod test { use crate::metadata::Annotations; use crate::test; + const VERSION_ZERO_ONE_ZERO: semver::Version = semver::Version::new(0, 1, 0); + fn mock_target_attributes() -> TargetAttributes { TargetAttributes { crate_name: "mock_crate".to_owned(), @@ -948,14 +956,25 @@ mod test { #[test] fn render_rust_library() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -974,15 +993,25 @@ mod test { #[test] fn test_disable_pipelining() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, disable_pipelining: true, - ..CrateContext::default() + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -999,20 +1028,30 @@ mod test { #[test] fn render_cargo_build_script() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::BuildScript(TargetAttributes { crate_name: "build_script_build".to_owned(), crate_root: Some("build.rs".to_owned()), ..TargetAttributes::default() })]), // Build script attributes are required. + library_target_name: None, + common_attrs: CommonAttributes::default(), build_script_attrs: Some(BuildScriptAttributes::default()), - ..CrateContext::default() + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1034,14 +1073,25 @@ mod test { #[test] fn render_proc_macro() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::ProcMacro(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1060,14 +1110,25 @@ mod test { #[test] fn render_binary() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Binary(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1086,17 +1147,27 @@ mod test { #[test] fn render_additive_build_contents() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Binary(mock_target_attributes())]), + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, additive_build_file_content: Some( "# Hello World from additive section!".to_owned(), ), - ..CrateContext::default() + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1132,14 +1203,25 @@ mod test { #[test] fn render_crate_repositories() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1154,14 +1236,25 @@ mod test { #[test] fn remote_remote_vendor_mode() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1182,14 +1275,25 @@ mod test { #[test] fn remote_local_vendor_mode() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1211,7 +1315,7 @@ mod test { #[test] fn duplicate_rustc_flags() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); let rustc_flags = vec![ "-l".to_owned(), @@ -1225,12 +1329,22 @@ mod test { CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), + library_target_name: None, common_attrs: CommonAttributes { rustc_flags: Select::from_value(rustc_flags.clone()), ..CommonAttributes::default() }, - ..CrateContext::default() + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1316,7 +1430,7 @@ mod test { .collect(), ..Context::default() }; - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); let mut crate_features: Select> = Select::default(); crate_features.insert("foo".to_owned(), Some("aarch64-apple-darwin".to_owned())); crate_features.insert("bar".to_owned(), None); @@ -1325,12 +1439,22 @@ mod test { CrateContext { name: crate_id.name, version: crate_id.version, + package_url: None, + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), + library_target_name: None, common_attrs: CommonAttributes { crate_features, ..CommonAttributes::default() }, - ..CrateContext::default() + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1358,15 +1482,25 @@ mod test { #[test] fn crate_package_metadata_without_license_ids() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { name: crate_id.name, version: crate_id.version, package_url: Some("http://www.mock_crate.com/".to_owned()), + repository: None, targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + license: None, + license_ids: BTreeSet::default(), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), + alias_rule: None, }, ); @@ -1400,7 +1534,7 @@ mod test { #[test] fn crate_package_metadata_with_license_ids() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { @@ -1408,8 +1542,17 @@ mod test { version: crate_id.version, package_url: Some("http://www.mock_crate.com/".to_owned()), license_ids: BTreeSet::from(["Apache-2.0".to_owned(), "MIT".to_owned()]), + license_file: None, + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + repository: None, + license: None, + alias_rule: None, }, ); @@ -1454,7 +1597,7 @@ mod test { #[test] fn crate_package_metadata_with_license_ids_and_file() { let mut context = Context::default(); - let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned()); + let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO); context.crates.insert( crate_id.clone(), CrateContext { @@ -1463,8 +1606,16 @@ mod test { package_url: Some("http://www.mock_crate.com/".to_owned()), license_ids: BTreeSet::from(["Apache-2.0".to_owned(), "MIT".to_owned()]), license_file: Some("LICENSE.txt".to_owned()), + additive_build_file_content: None, + disable_pipelining: false, + extra_aliased_targets: BTreeMap::default(), targets: BTreeSet::from([Rule::Library(mock_target_attributes())]), - ..CrateContext::default() + library_target_name: None, + common_attrs: CommonAttributes::default(), + build_script_attrs: None, + repository: None, + license: None, + alias_rule: None, }, ); diff --git a/crate_universe/src/splicing.rs b/crate_universe/src/splicing.rs index 69b83d2d9e..2107a1be4b 100644 --- a/crate_universe/src/splicing.rs +++ b/crate_universe/src/splicing.rs @@ -375,7 +375,7 @@ impl WorkspaceMetadata { })?; lookup.get_source_info(pkg).map(|source_info| { ( - CrateId::new(pkg.name.as_str().to_owned(), pkg.version.to_string()), + CrateId::new(pkg.name.as_str().to_owned(), pkg.version.clone()), source_info, ) }) diff --git a/crate_universe/src/splicing/splicer.rs b/crate_universe/src/splicing/splicer.rs index 552d4e165c..afed559a0e 100644 --- a/crate_universe/src/splicing/splicer.rs +++ b/crate_universe/src/splicing/splicer.rs @@ -576,7 +576,8 @@ pub fn default_cargo_package_manifest() -> cargo_toml::Manifest { pub fn default_splicing_package_crate_id() -> CrateId { CrateId::new( DEFAULT_SPLICING_PACKAGE_NAME.to_string(), - DEFAULT_SPLICING_PACKAGE_VERSION.to_string(), + semver::Version::parse(DEFAULT_SPLICING_PACKAGE_VERSION) + .expect("Known good version didn't parse"), ) }