diff --git a/crates/rattler-bin/src/commands/create.rs b/crates/rattler-bin/src/commands/create.rs index 4105aedab..5fb21c65c 100644 --- a/crates/rattler-bin/src/commands/create.rs +++ b/crates/rattler-bin/src/commands/create.rs @@ -160,13 +160,13 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> { let package_names = specs .iter() .filter_map(|spec| spec.name.as_ref()) - .map(|name| name.as_normalized().as_ref()); + .map(|name| name.as_normalized()); let repodatas = wrap_in_progress("parsing repodata", move || { SparseRepoData::load_records_recursive( &sparse_repo_datas, package_names, Some(|record| { - if record.name == "python" { + if record.name.as_normalized() == "python" { record.depends.push("pip".to_string()); } }), @@ -182,24 +182,26 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> { .iter() .map(|virt_pkg| { let elems = virt_pkg.split('=').collect::<Vec<&str>>(); - GenericVirtualPackage { - name: elems[0].to_string(), + Ok(GenericVirtualPackage { + name: elems[0].try_into()?, version: elems .get(1) .map(|s| Version::from_str(s)) .unwrap_or(Version::from_str("0")) .expect("Could not parse virtual package version"), build_string: elems.get(2).unwrap_or(&"").to_string(), - } + }) }) - .collect::<Vec<_>>()) + .collect::<anyhow::Result<Vec<_>>>()?) } else { - rattler_virtual_packages::VirtualPackage::current().map(|vpkgs| { - vpkgs - .iter() - .map(|vpkg| GenericVirtualPackage::from(vpkg.clone())) - .collect::<Vec<_>>() - }) + rattler_virtual_packages::VirtualPackage::current() + .map(|vpkgs| { + vpkgs + .iter() + .map(|vpkg| GenericVirtualPackage::from(vpkg.clone())) + .collect::<Vec<_>>() + }) + .map_err(anyhow::Error::from) } })?; @@ -250,7 +252,9 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> { let format_record = |r: &RepoDataRecord| { format!( "{} {} {}", - r.package_record.name, r.package_record.version, r.package_record.build + r.package_record.name.as_normalized(), + r.package_record.version, + r.package_record.build ) }; @@ -495,7 +499,11 @@ async fn install_package_to_environment( // Write the conda-meta information let pkg_meta_path = conda_meta_path.join(format!( "{}-{}-{}.json", - prefix_record.repodata_record.package_record.name, + prefix_record + .repodata_record + .package_record + .name + .as_normalized(), prefix_record.repodata_record.package_record.version, prefix_record.repodata_record.package_record.build )); @@ -539,7 +547,7 @@ async fn remove_package_from_environment( // Remove the conda-meta file let conda_meta_path = target_prefix.join("conda-meta").join(format!( "{}-{}-{}.json", - package.repodata_record.package_record.name, + package.repodata_record.package_record.name.as_normalized(), package.repodata_record.package_record.version, package.repodata_record.package_record.build )); @@ -623,7 +631,7 @@ async fn fetch_repo_data_records_with_progress( platform.to_string(), repo_data_json_path, Some(|record: &mut PackageRecord| { - if record.name == "python" { + if record.name.as_normalized() == "python" { record.depends.push("pip".to_string()); } }), diff --git a/crates/rattler/src/install/transaction.rs b/crates/rattler/src/install/transaction.rs index 6ebec560c..b5040cf0a 100644 --- a/crates/rattler/src/install/transaction.rs +++ b/crates/rattler/src/install/transaction.rs @@ -166,7 +166,7 @@ fn find_python_info( /// Returns true if the specified record refers to Python. fn is_python_record(record: &PackageRecord) -> bool { - record.name == "python" + record.name.as_normalized() == "python" } /// Returns true if the `from` and `to` describe the same package content diff --git a/crates/rattler/src/package_cache.rs b/crates/rattler/src/package_cache.rs index bad1f9aff..f267b610c 100644 --- a/crates/rattler/src/package_cache.rs +++ b/crates/rattler/src/package_cache.rs @@ -56,7 +56,7 @@ impl From<ArchiveIdentifier> for CacheKey { impl From<&PackageRecord> for CacheKey { fn from(record: &PackageRecord) -> Self { Self { - name: record.name.to_string(), + name: record.name.as_normalized().to_string(), version: record.version.to_string(), build_string: record.build.to_string(), } diff --git a/crates/rattler_conda_types/src/conda_lock/builder.rs b/crates/rattler_conda_types/src/conda_lock/builder.rs index c61e30781..367a3487a 100644 --- a/crates/rattler_conda_types/src/conda_lock/builder.rs +++ b/crates/rattler_conda_types/src/conda_lock/builder.rs @@ -6,7 +6,7 @@ use crate::conda_lock::{ content_hash, Channel, CondaLock, GitMeta, LockMeta, LockedDependency, Manager, PackageHashes, TimeMeta, }; -use crate::{MatchSpec, NamelessMatchSpec, NoArchType, Platform, RepoDataRecord}; +use crate::{MatchSpec, NamelessMatchSpec, NoArchType, PackageName, Platform, RepoDataRecord}; use fxhash::{FxHashMap, FxHashSet}; use std::str::FromStr; use url::Url; @@ -169,7 +169,7 @@ impl LockedPackages { /// Short-hand for creating a LockedPackage that transforms into a [`LockedDependency`] pub struct LockedPackage { /// Name of the locked package - pub name: String, + pub name: PackageName, /// Package version pub version: String, /// Package build string @@ -179,7 +179,7 @@ pub struct LockedPackage { /// Collection of package hash fields pub package_hashes: PackageHashes, /// List of dependencies for this package - pub dependency_list: FxHashMap<String, NamelessMatchSpec>, + pub dependency_list: FxHashMap<PackageName, NamelessMatchSpec>, /// Check if package is optional pub optional: Option<bool>, @@ -245,9 +245,9 @@ impl TryFrom<RepoDataRecord> for LockedPackage { .ok_or_else(|| { ConversionError::Missing(format!("dependency name for {}", match_spec_str)) })? - .to_string(); + .clone(); let version_constraint = NamelessMatchSpec::from(matchspec); - dependencies.insert(name, version_constraint); + dependencies.insert(name.clone(), version_constraint); } Ok(Self { @@ -281,20 +281,19 @@ impl LockedPackage { } /// Add a single dependency - pub fn add_dependency<S: AsRef<str>>( + pub fn add_dependency( mut self, - key: S, + key: PackageName, version_constraint: NamelessMatchSpec, ) -> Self { - self.dependency_list - .insert(key.as_ref().to_string(), version_constraint); + self.dependency_list.insert(key, version_constraint); self } /// Add multiple dependencies pub fn add_dependencies( mut self, - value: impl IntoIterator<Item = (String, NamelessMatchSpec)>, + value: impl IntoIterator<Item = (PackageName, NamelessMatchSpec)>, ) -> Self { self.dependency_list.extend(value); self @@ -391,7 +390,8 @@ mod tests { use crate::conda_lock::builder::{LockFileBuilder, LockedPackage, LockedPackages}; use crate::conda_lock::PackageHashes; use crate::{ - ChannelConfig, MatchSpec, NamelessMatchSpec, NoArchType, Platform, RepoDataRecord, + ChannelConfig, MatchSpec, NamelessMatchSpec, NoArchType, PackageName, Platform, + RepoDataRecord, }; use rattler_digest::parse_digest_from_hex; @@ -405,13 +405,13 @@ mod tests { ) .add_locked_packages(LockedPackages::new(Platform::Osx64) .add_locked_package(LockedPackage { - name: "python".to_string(), + name: PackageName::new_unchecked("python"), version: "3.11.0".to_string(), build_string: "h4150a38_1_cpython".to_string(), url: "https://conda.anaconda.org/conda-forge/osx-64/python-3.11.0-h4150a38_1_cpython.conda".parse().unwrap(), package_hashes: PackageHashes::Md5Sha256(parse_digest_from_hex::<rattler_digest::Md5>("c6f4b87020c72e2700e3e94c1fc93b70").unwrap(), parse_digest_from_hex::<rattler_digest::Sha256>("7c58de8c7d98b341bd9be117feec64782e704fec5c30f6e14713ebccaab9b5d8").unwrap()), - dependency_list: FxHashMap::from_iter([("python".to_string(), NamelessMatchSpec::from_str("3.11.0.*").unwrap())]), + dependency_list: FxHashMap::from_iter([(PackageName::new_unchecked("python"), NamelessMatchSpec::from_str("3.11.0.*").unwrap())]), optional: None, arch: Some("x86_64".to_string()), subdir: Some("noarch".to_string()), diff --git a/crates/rattler_conda_types/src/conda_lock/mod.rs b/crates/rattler_conda_types/src/conda_lock/mod.rs index a3ce8aae2..e93d407cd 100644 --- a/crates/rattler_conda_types/src/conda_lock/mod.rs +++ b/crates/rattler_conda_types/src/conda_lock/mod.rs @@ -4,11 +4,11 @@ //! However, some types were added to enforce a bit more type safety. use self::PackageHashes::{Md5, Md5Sha256, Sha256}; use crate::match_spec::parse::ParseMatchSpecError; -use crate::MatchSpec; use crate::{ utils::serde::Ordered, NamelessMatchSpec, NoArchType, PackageRecord, ParsePlatformError, ParseVersionError, Platform, RepoDataRecord, }; +use crate::{MatchSpec, PackageName}; use fxhash::FxHashMap; use rattler_digest::{serde::SerializableHash, Md5Hash, Sha256Hash}; use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer}; @@ -237,7 +237,7 @@ fn default_category() -> String { #[derive(Serialize, Deserialize, Eq, PartialEq, Clone, Debug)] pub struct LockedDependency { /// Package name of dependency - pub name: String, + pub name: PackageName, /// Locked version pub version: String, /// Pip or Conda managed @@ -248,7 +248,7 @@ pub struct LockedDependency { /// What are its own dependencies mapping name to version constraint #[serde_as(as = "FxHashMap<_, DisplayFromStr>")] - pub dependencies: FxHashMap<String, NamelessMatchSpec>, + pub dependencies: FxHashMap<PackageName, NamelessMatchSpec>, /// URL to find it at pub url: Url, /// Hashes of the package @@ -348,9 +348,7 @@ impl TryFrom<LockedDependency> for RepoDataRecord { let matchspecs = value .dependencies .into_iter() - .map(|(name, matchspec)| { - MatchSpec::from_nameless(matchspec, Some(name.into())).to_string() - }) + .map(|(name, matchspec)| MatchSpec::from_nameless(matchspec, Some(name)).to_string()) .collect::<Vec<_>>(); let version = value.version.parse()?; @@ -594,12 +592,15 @@ mod test { let result: crate::conda_lock::LockedDependency = from_str(yaml).unwrap(); - assert_eq!(result.name, "ncurses"); + assert_eq!(result.name.as_normalized(), "ncurses"); assert_eq!(result.version, "6.4"); let repodata_record = RepoDataRecord::try_from(result.clone()).unwrap(); - assert_eq!(repodata_record.package_record.name, "ncurses"); + assert_eq!( + repodata_record.package_record.name.as_normalized(), + "ncurses" + ); assert_eq!( repodata_record.package_record.version, VersionWithSource::from_str("6.4").unwrap() diff --git a/crates/rattler_conda_types/src/generic_virtual_package.rs b/crates/rattler_conda_types/src/generic_virtual_package.rs index 11ee8c313..258fedeac 100644 --- a/crates/rattler_conda_types/src/generic_virtual_package.rs +++ b/crates/rattler_conda_types/src/generic_virtual_package.rs @@ -1,4 +1,4 @@ -use crate::Version; +use crate::{PackageName, Version}; use std::fmt::{Display, Formatter}; /// A `GenericVirtualPackage` is a Conda package description that contains a `name` and a @@ -6,7 +6,7 @@ use std::fmt::{Display, Formatter}; #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub struct GenericVirtualPackage { /// The name of the package - pub name: String, + pub name: PackageName, /// The version of the package pub version: Version, @@ -17,6 +17,12 @@ pub struct GenericVirtualPackage { impl Display for GenericVirtualPackage { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}={}={}", &self.name, &self.version, &self.build_string) + write!( + f, + "{}={}={}", + &self.name.as_normalized(), + &self.version, + &self.build_string + ) } } diff --git a/crates/rattler_conda_types/src/lib.rs b/crates/rattler_conda_types/src/lib.rs index 8df3a4344..15c6cd087 100644 --- a/crates/rattler_conda_types/src/lib.rs +++ b/crates/rattler_conda_types/src/lib.rs @@ -32,7 +32,7 @@ pub use match_spec::matcher::StringMatcher; pub use match_spec::parse::ParseMatchSpecError; pub use match_spec::{MatchSpec, NamelessMatchSpec}; pub use no_arch_type::{NoArchKind, NoArchType}; -pub use package_name::PackageName; +pub use package_name::{PackageName, InvalidPackageNameError}; pub use platform::{ParsePlatformError, Platform}; pub use prefix_record::PrefixRecord; pub use repo_data::patches::{PackageRecordPatch, PatchInstructions, RepoDataPatch}; diff --git a/crates/rattler_conda_types/src/match_spec/mod.rs b/crates/rattler_conda_types/src/match_spec/mod.rs index 9806a276b..4b33d4fcc 100644 --- a/crates/rattler_conda_types/src/match_spec/mod.rs +++ b/crates/rattler_conda_types/src/match_spec/mod.rs @@ -63,38 +63,38 @@ use matcher::StringMatcher; /// # Examples: /// /// ```rust -/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher}; +/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName}; /// use std::str::FromStr; /// /// let spec = MatchSpec::from_str("foo 1.0 py27_0").unwrap(); -/// assert_eq!(spec.name, Some("foo".to_string())); +/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0").unwrap())); /// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap())); /// /// let spec = MatchSpec::from_str("foo=1.0=py27_0").unwrap(); -/// assert_eq!(spec.name, Some("foo".to_string())); +/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.version, Some(VersionSpec::from_str("==1.0").unwrap())); /// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap())); /// /// let spec = MatchSpec::from_str("conda-forge::foo[version=\"1.0.*\"]").unwrap(); -/// assert_eq!(spec.name, Some("foo".to_string())); +/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); /// assert_eq!(spec.channel, Some("conda-forge".to_string())); /// /// let spec = MatchSpec::from_str("conda-forge/linux-64::foo>=1.0").unwrap(); -/// assert_eq!(spec.name, Some("foo".to_string())); +/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap())); /// assert_eq!(spec.channel, Some("conda-forge".to_string())); /// assert_eq!(spec.subdir, Some("linux-64".to_string())); /// /// let spec = MatchSpec::from_str("*/linux-64::foo>=1.0").unwrap(); -/// assert_eq!(spec.name, Some("foo".to_string())); +/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap())); /// assert_eq!(spec.channel, Some("*".to_string())); /// assert_eq!(spec.subdir, Some("linux-64".to_string())); /// /// let spec = MatchSpec::from_str("foo[build=\"py2*\"]").unwrap(); -/// assert_eq!(spec.name, Some("foo".to_string())); +/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo"))); /// assert_eq!(spec.build, Some(StringMatcher::from_str("py2*").unwrap())); /// ``` /// @@ -149,7 +149,7 @@ impl Display for MatchSpec { } match &self.name { - Some(name) => write!(f, "{name}")?, + Some(name) => write!(f, "{}", name.as_normalized())?, None => write!(f, "*")?, } @@ -189,7 +189,7 @@ impl MatchSpec { /// Match a MatchSpec against a PackageRecord pub fn matches(&self, record: &PackageRecord) -> bool { if let Some(name) = self.name.as_ref() { - if name.as_normalized().as_ref() != record.name { + if name != &record.name { return false; } } @@ -350,7 +350,7 @@ mod tests { use rattler_digest::{parse_digest_from_hex, Md5, Sha256}; - use crate::{MatchSpec, NamelessMatchSpec, PackageRecord, Version}; + use crate::{MatchSpec, NamelessMatchSpec, PackageName, PackageRecord, Version}; #[test] fn test_matchspec_format_eq() { @@ -378,7 +378,7 @@ mod tests { ), md5: parse_digest_from_hex::<Md5>("dede6252c964db3f3e41c7d30d07f6bf"), ..PackageRecord::new( - String::from("mamba"), + PackageName::new_unchecked("mamba"), Version::from_str("1.0").unwrap(), String::from(""), ) diff --git a/crates/rattler_conda_types/src/match_spec/parse.rs b/crates/rattler_conda_types/src/match_spec/parse.rs index 202e65b90..e6ce59f12 100644 --- a/crates/rattler_conda_types/src/match_spec/parse.rs +++ b/crates/rattler_conda_types/src/match_spec/parse.rs @@ -3,7 +3,9 @@ use super::MatchSpec; use crate::package::ArchiveType; use crate::version_spec::version_tree::{recognize_constraint, recognize_version}; use crate::version_spec::{is_start_of_version_constraint, ParseVersionSpecError}; -use crate::{NamelessMatchSpec, ParseChannelError, VersionSpec}; +use crate::{ + InvalidPackageNameError, NamelessMatchSpec, PackageName, ParseChannelError, VersionSpec, +}; use nom::branch::alt; use nom::bytes::complete::{tag, take_till1, take_until, take_while, take_while1}; use nom::character::complete::{char, multispace0, one_of}; @@ -22,7 +24,7 @@ use thiserror::Error; use url::Url; /// The type of parse error that occurred when parsing match spec. -#[derive(Debug, Clone, Eq, PartialEq, Error)] +#[derive(Debug, Clone, Error)] pub enum ParseMatchSpecError { /// The path or url of the package was invalid #[error("invalid package path or url")] @@ -57,11 +59,11 @@ pub enum ParseMatchSpecError { InvalidVersionAndBuild(String), /// Invalid version spec - #[error("invalid version spec: {0}")] + #[error(transparent)] InvalidVersionSpec(#[from] ParseVersionSpecError), /// Invalid string matcher - #[error("invalid string matcher: {0}")] + #[error(transparent)] InvalidStringMatcher(#[from] StringMatcherParseError), /// Invalid build number @@ -71,6 +73,10 @@ pub enum ParseMatchSpecError { /// Unable to parse hash digest from hex #[error("Unable to parse hash digest from hex")] InvalidHashDigest, + + /// The package name was invalid + #[error(transparent)] + InvalidPackageName(#[from] InvalidPackageNameError), } impl FromStr for MatchSpec { @@ -221,11 +227,11 @@ fn parse_bracket_vec_into_components( } /// Strip the package name from the input. -fn strip_package_name(input: &str) -> Result<(&str, &str), ParseMatchSpecError> { +fn strip_package_name(input: &str) -> Result<(PackageName, &str), ParseMatchSpecError> { match take_while1(|c: char| !c.is_whitespace() && !is_start_of_version_constraint(c))(input) .finish() { - Ok((input, name)) => Ok((name.trim(), input.trim())), + Ok((input, name)) => Ok((PackageName::from_str(name.trim())?, input.trim())), Err(nom::error::Error { .. }) => Err(ParseMatchSpecError::MissingPackageName), } } @@ -397,7 +403,7 @@ fn parse(input: &str) -> Result<MatchSpec, ParseMatchSpecError> { // Step 6. Strip off the package name from the input let (name, input) = strip_package_name(input)?; - let mut match_spec = MatchSpec::from_nameless(nameless_match_spec, Some(name.into())); + let mut match_spec = MatchSpec::from_nameless(nameless_match_spec, Some(name)); // Step 7. Otherwise sort our version + build let input = input.trim(); @@ -454,6 +460,7 @@ fn parse(input: &str) -> Result<MatchSpec, ParseMatchSpecError> { #[cfg(test)] mod tests { + use assert_matches::assert_matches; use rattler_digest::{parse_digest_from_hex, Md5, Sha256}; use serde::Serialize; use std::collections::BTreeMap; @@ -493,11 +500,11 @@ mod tests { let expected: BracketVec = smallvec![("version", "1.2.3"), ("build_number", "1")]; assert_eq!(result.1, expected); - assert_eq!( + assert_matches!( strip_brackets(r#"bla [version="1.2.3", build_number=]"#), Err(ParseMatchSpecError::InvalidBracket) ); - assert_eq!( + assert_matches!( strip_brackets(r#"bla [version="1.2.3, build_number=1]"#), Err(ParseMatchSpecError::InvalidBracket) ); @@ -505,35 +512,35 @@ mod tests { #[test] fn test_split_version_and_build() { - assert_eq!( + assert_matches!( split_version_and_build("==1.0=py27_0"), Ok(("==1.0", Some("py27_0"))) ); - assert_eq!(split_version_and_build("=*=cuda"), Ok(("=*", Some("cuda")))); - assert_eq!( + assert_matches!(split_version_and_build("=*=cuda"), Ok(("=*", Some("cuda")))); + assert_matches!( split_version_and_build("=1.2.3 0"), Ok(("=1.2.3", Some("0"))) ); - assert_eq!(split_version_and_build("1.2.3=0"), Ok(("1.2.3", Some("0")))); - assert_eq!( + assert_matches!(split_version_and_build("1.2.3=0"), Ok(("1.2.3", Some("0")))); + assert_matches!( split_version_and_build(">=1.0 , < 2.0 py34_0"), Ok((">=1.0 , < 2.0", Some("py34_0"))) ); - assert_eq!( + assert_matches!( split_version_and_build(">=1.0 , < 2.0 =py34_0"), Ok((">=1.0 , < 2.0", Some("=py34_0"))) ); - assert_eq!(split_version_and_build("=1.2.3 "), Ok(("=1.2.3", None))); - assert_eq!( + assert_matches!(split_version_and_build("=1.2.3 "), Ok(("=1.2.3", None))); + assert_matches!( split_version_and_build(">1.8,<2|==1.7"), Ok((">1.8,<2|==1.7", None)) ); - assert_eq!( + assert_matches!( split_version_and_build("* openblas_0"), Ok(("*", Some("openblas_0"))) ); - assert_eq!(split_version_and_build("* *"), Ok(("*", Some("*")))); - assert_eq!( + assert_matches!(split_version_and_build("* *"), Ok(("*", Some("*")))); + assert_matches!( split_version_and_build(">=1!164.3095,<1!165"), Ok((">=1!164.3095,<1!165", None)) ); @@ -561,12 +568,12 @@ mod tests { #[test] fn test_match_spec_more() { let spec = MatchSpec::from_str("conda-forge::foo[version=\"1.0.*\"]").unwrap(); - assert_eq!(spec.name, Some("foo".into())); + assert_eq!(spec.name, Some("foo".parse().unwrap())); assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); assert_eq!(spec.channel, Some("conda-forge".to_string())); let spec = MatchSpec::from_str("conda-forge::foo[version=1.0.*]").unwrap(); - assert_eq!(spec.name, Some("foo".into())); + assert_eq!(spec.name, Some("foo".parse().unwrap())); assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap())); assert_eq!(spec.channel, Some("conda-forge".to_string())); } @@ -574,10 +581,10 @@ mod tests { #[test] fn test_hash_spec() { let spec = MatchSpec::from_str("conda-forge::foo[md5=1234567890]"); - assert_eq!(spec, Err(ParseMatchSpecError::InvalidHashDigest)); + assert_matches!(spec, Err(ParseMatchSpecError::InvalidHashDigest)); let spec = MatchSpec::from_str("conda-forge::foo[sha256=1234567890]"); - assert_eq!(spec, Err(ParseMatchSpecError::InvalidHashDigest)); + assert_matches!(spec, Err(ParseMatchSpecError::InvalidHashDigest)); let spec = MatchSpec::from_str("conda-forge::foo[sha256=315f5bdb76d078c43b8ac0064e4a0164612b1fce77c869345bfc94c75894edd3]").unwrap(); assert_eq!( diff --git a/crates/rattler_conda_types/src/package/index.rs b/crates/rattler_conda_types/src/package/index.rs index 16fbcba15..5aea26a52 100644 --- a/crates/rattler_conda_types/src/package/index.rs +++ b/crates/rattler_conda_types/src/package/index.rs @@ -1,7 +1,7 @@ use std::path::Path; use super::PackageFile; -use crate::{NoArchType, VersionWithSource}; +use crate::{NoArchType, PackageName, VersionWithSource}; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, skip_serializing_none, OneOrMany}; @@ -45,7 +45,7 @@ pub struct IndexJson { pub license_family: Option<String>, /// The lowercase name of the package - pub name: String, + pub name: PackageName, /// If this package is independent of architecture this field specifies in what way. See /// [`NoArchType`] for more information. diff --git a/crates/rattler_conda_types/src/package_name.rs b/crates/rattler_conda_types/src/package_name.rs index 1eadc9887..94f505dd7 100644 --- a/crates/rattler_conda_types/src/package_name.rs +++ b/crates/rattler_conda_types/src/package_name.rs @@ -1,85 +1,114 @@ use serde::{Serialize, Serializer}; use serde_with::DeserializeFromStr; use std::cmp::Ordering; -use std::fmt::{Display, Formatter}; use std::hash::{Hash, Hasher}; use std::str::FromStr; -use std::sync::Arc; +use thiserror::Error; /// A representation of a conda package name. This struct both stores the source string from which /// this instance was created as well as a normalized name that can be used to compare different /// names. The normalized name is guaranteed to be a valid conda package name. /// -/// Conda package names are always lowercase. +/// Conda package names are always lowercase and can only contain ascii characters. +/// +/// This struct explicitly does not implement [`Display`] because its ambiguous if that would +/// display the source or the normalized version. Simply call `as_source` or `as_normalized` to make +/// the distinction. #[derive(Debug, Clone, Eq, DeserializeFromStr)] pub struct PackageName { - source: Arc<str>, - normalized: Arc<str>, + normalized: Option<String>, + source: String, } impl PackageName { + /// Constructs a new `PackageName` from a string without checking if the string is actually a + /// valid or normalized conda package name. This should only be used if you are sure that the + /// input string is valid, otherwise use the `TryFrom` implementations. + pub fn new_unchecked<S: Into<String>>(normalized: S) -> Self { + Self { + normalized: None, + source: normalized.into(), + } + } + /// Returns the source representation of the package name. This is the string from which this /// instance was created. - pub fn as_source(&self) -> &Arc<str> { + pub fn as_source(&self) -> &str { &self.source } /// Returns the normalized version of the package name. The normalized string is guaranteed to /// be a valid conda package name. - pub fn as_normalized(&self) -> &Arc<str> { - &self.normalized + pub fn as_normalized(&self) -> &str { + self.normalized.as_ref().unwrap_or(&self.source) } } -impl From<&String> for PackageName { - fn from(value: &String) -> Self { - Arc::<str>::from(value.clone()).into() - } +/// An error that is returned when conversion from a string to a [`PackageName`] fails. +#[derive(Clone, Debug, Error)] +pub enum InvalidPackageNameError { + /// The package name contains illegal characters + #[error("'{0}' is not a valid package name. Package names can only contain 0-9, a-z, A-Z, -, _, or .")] + InvalidCharacters(String), } -impl From<String> for PackageName { - fn from(value: String) -> Self { - Arc::<str>::from(value).into() +impl TryFrom<&String> for PackageName { + type Error = InvalidPackageNameError; + + fn try_from(value: &String) -> Result<Self, Self::Error> { + value.clone().try_into() } } -impl From<Arc<str>> for PackageName { - fn from(value: Arc<str>) -> Self { - let normalized = if value.chars().any(char::is_uppercase) { - Arc::from(value.to_lowercase()) +impl TryFrom<String> for PackageName { + type Error = InvalidPackageNameError; + + fn try_from(source: String) -> Result<Self, Self::Error> { + // Ensure that the string only contains valid characters + if !source + .chars() + .all(|c| matches!(c, 'a'..='z'|'A'..='Z'|'0'..='9'|'-'|'_'|'.')) + { + return Err(InvalidPackageNameError::InvalidCharacters(source)); + } + + // Convert all characters to lowercase but only if it actually contains uppercase. This way + // we dont allocate the memory of the string if it is already lowercase. + let normalized = if source.chars().any(|c| c.is_ascii_uppercase()) { + Some(source.to_ascii_lowercase()) } else { - value.clone() + None }; - Self { - source: value, - normalized, - } + + Ok(Self { source, normalized }) } } -impl<'a> From<&'a str> for PackageName { - fn from(value: &'a str) -> Self { - Arc::<str>::from(value.to_owned()).into() +impl<'a> TryFrom<&'a str> for PackageName { + type Error = InvalidPackageNameError; + + fn try_from(value: &'a str) -> Result<Self, Self::Error> { + value.to_owned().try_into() } } impl FromStr for PackageName { - type Err = String; + type Err = InvalidPackageNameError; fn from_str(s: &str) -> Result<Self, Self::Err> { - Ok(Self::from(s)) + s.to_owned().try_into() } } impl Hash for PackageName { fn hash<H: Hasher>(&self, state: &mut H) { - self.normalized.hash(state) + self.as_normalized().hash(state) } } impl PartialEq for PackageName { fn eq(&self, other: &Self) -> bool { - self.normalized.eq(&other.normalized) + self.as_normalized().eq(other.as_normalized()) } } @@ -91,7 +120,7 @@ impl PartialOrd for PackageName { impl Ord for PackageName { fn cmp(&self, other: &Self) -> Ordering { - self.normalized.cmp(&other.normalized) + self.as_normalized().cmp(&other.as_normalized()) } } @@ -100,13 +129,7 @@ impl Serialize for PackageName { where S: Serializer, { - self.source.as_ref().serialize(serializer) - } -} - -impl Display for PackageName { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.source.as_ref()) + self.as_source().serialize(serializer) } } @@ -115,15 +138,17 @@ mod test { use super::*; #[test] - fn test_packagename_basics() { - let name1 = PackageName::from("cuDNN"); - assert_eq!(name1.as_source().as_ref(), "cuDNN"); - assert_eq!(name1.as_normalized().as_ref(), "cudnn"); + fn test_package_name_basics() { + let name1 = PackageName::try_from("cuDNN").unwrap(); + assert_eq!(name1.as_source(), "cuDNN"); + assert_eq!(name1.as_normalized(), "cudnn"); - let name2 = PackageName::from("cudnn"); - assert_eq!(name2.as_source().as_ref(), "cudnn"); - assert_eq!(name2.as_normalized().as_ref(), "cudnn"); + let name2 = PackageName::try_from("cudnn").unwrap(); + assert_eq!(name2.as_source(), "cudnn"); + assert_eq!(name2.as_normalized(), "cudnn"); assert_eq!(name1, name2); + + assert!(PackageName::try_from("invalid$").is_err()); } } diff --git a/crates/rattler_conda_types/src/repo_data/mod.rs b/crates/rattler_conda_types/src/repo_data/mod.rs index f3909e736..5407bf160 100644 --- a/crates/rattler_conda_types/src/repo_data/mod.rs +++ b/crates/rattler_conda_types/src/repo_data/mod.rs @@ -18,7 +18,7 @@ use thiserror::Error; use rattler_macros::sorted; use crate::package::IndexJson; -use crate::{Channel, NoArchType, Platform, RepoDataRecord, VersionWithSource}; +use crate::{Channel, NoArchType, PackageName, Platform, RepoDataRecord, VersionWithSource}; /// [`RepoData`] is an index of package binaries available on in a subdirectory of a Conda channel. // Note: we cannot use the sorted macro here, because the `packages` and `conda_packages` fields are @@ -106,7 +106,7 @@ pub struct PackageRecord { pub md5: Option<Md5Hash>, /// The name of the package - pub name: String, + pub name: PackageName, /// If this package is independent of architecture this field specifies in what way. See /// [`NoArchType`] for more information. @@ -149,7 +149,13 @@ pub struct PackageRecord { impl Display for PackageRecord { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}={}={}", self.name, self.version, self.build) + write!( + f, + "{}={}={}", + self.name.as_normalized(), + self.version, + self.build + ) } } @@ -182,7 +188,7 @@ impl RepoData { impl PackageRecord { /// A simple helper method that constructs a `PackageRecord` with the bare minimum values. - pub fn new(name: String, version: impl Into<VersionWithSource>, build: String) -> Self { + pub fn new(name: PackageName, version: impl Into<VersionWithSource>, build: String) -> Self { Self { arch: None, build, diff --git a/crates/rattler_conda_types/src/repo_data/topological_sort.rs b/crates/rattler_conda_types/src/repo_data/topological_sort.rs index eb6560373..a7a92917b 100644 --- a/crates/rattler_conda_types/src/repo_data/topological_sort.rs +++ b/crates/rattler_conda_types/src/repo_data/topological_sort.rs @@ -18,7 +18,7 @@ pub fn sort_topologically<T: AsRef<PackageRecord> + Clone>(packages: Vec<T>) -> let mut all_packages = packages .iter() .cloned() - .map(|p| (p.as_ref().name.clone(), p)) + .map(|p| (p.as_ref().name.as_normalized().to_owned(), p)) .collect(); // Detect cycles @@ -56,7 +56,10 @@ fn get_graph_roots<T: AsRef<PackageRecord>>( records: &[T], cycle_breaks: Option<&FxHashSet<(String, String)>>, ) -> Vec<String> { - let all_packages: FxHashSet<_> = records.iter().map(|r| r.as_ref().name.as_str()).collect(); + let all_packages: FxHashSet<_> = records + .iter() + .map(|r| r.as_ref().name.as_normalized()) + .collect(); let dependencies: FxHashSet<_> = records .iter() @@ -68,7 +71,8 @@ fn get_graph_roots<T: AsRef<PackageRecord>>( .filter(|d| { // filter out circular dependencies if let Some(cycle_breaks) = cycle_breaks { - !cycle_breaks.contains(&(r.as_ref().name.clone(), d.to_string())) + !cycle_breaks + .contains(&(r.as_ref().name.as_normalized().to_owned(), d.to_string())) } else { true } @@ -236,11 +240,11 @@ mod tests { ) { let all_sorted_packages: FxHashSet<_> = sorted_packages .iter() - .map(|p| p.package_record.name.as_str()) + .map(|p| p.package_record.name.as_normalized()) .collect(); let all_original_packages: FxHashSet<_> = original_packages .iter() - .map(|p| p.package_record.name.as_str()) + .map(|p| p.package_record.name.as_normalized()) .collect(); let missing_in_sorted: Vec<_> = all_original_packages .difference(&all_sorted_packages) @@ -274,7 +278,7 @@ mod tests { .iter() .map(|p| { ( - p.package_record.name.as_str(), + p.package_record.name.as_normalized(), p.package_record.depends.as_slice(), ) }) @@ -282,7 +286,7 @@ mod tests { let mut installed = FxHashSet::default(); for (i, p) in sorted_packages.iter().enumerate() { - let name = p.package_record.name.as_str(); + let name = p.package_record.name.as_normalized(); let &deps = packages_by_name.get(name).unwrap(); // All the package's dependencies must have already been installed @@ -347,7 +351,10 @@ mod tests { // Sanity check: the last package should be python (or pip when it is present) let last_package = &sorted_packages[sorted_packages.len() - 1]; - assert_eq!(last_package.package_record.name, expected_last_package) + assert_eq!( + last_package.package_record.name.as_normalized(), + expected_last_package + ) } fn get_resolved_packages_for_two_roots() -> Vec<RepoDataRecord> { diff --git a/crates/rattler_libsolv_rs/src/conda_util.rs b/crates/rattler_libsolv_rs/src/conda_util.rs index cef078db1..49fa28cc3 100644 --- a/crates/rattler_libsolv_rs/src/conda_util.rs +++ b/crates/rattler_libsolv_rs/src/conda_util.rs @@ -3,7 +3,7 @@ use crate::id::{NameId, SolvableId}; use crate::mapping::Mapping; use crate::solvable::Solvable; use crate::MatchSpecId; -use rattler_conda_types::{MatchSpec, Version}; +use rattler_conda_types::{MatchSpec, PackageName, Version}; use std::cell::OnceCell; use std::cmp::Ordering; use std::collections::HashMap; @@ -14,7 +14,7 @@ pub(crate) fn compare_candidates( a: SolvableId, b: SolvableId, solvables: &Arena<SolvableId, Solvable>, - interned_strings: &HashMap<String, NameId>, + interned_strings: &HashMap<PackageName, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -140,7 +140,7 @@ pub(crate) fn compare_candidates( pub(crate) fn find_highest_version( match_spec_id: MatchSpecId, solvables: &Arena<SolvableId, Solvable>, - interned_strings: &HashMap<String, NameId>, + interned_strings: &HashMap<PackageName, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, match_specs: &Arena<MatchSpecId, MatchSpec>, match_spec_to_candidates: &Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -184,7 +184,7 @@ pub(crate) fn find_highest_version( pub(crate) fn find_candidates<'b>( match_spec_id: MatchSpecId, match_specs: &Arena<MatchSpecId, MatchSpec>, - names_to_ids: &HashMap<String, NameId>, + names_to_ids: &HashMap<PackageName, NameId>, packages_by_name: &Mapping<NameId, Vec<SolvableId>>, solvables: &Arena<SolvableId, Solvable>, match_spec_to_candidates: &'b Mapping<MatchSpecId, OnceCell<Vec<SolvableId>>>, @@ -192,7 +192,7 @@ pub(crate) fn find_candidates<'b>( match_spec_to_candidates[match_spec_id].get_or_init(|| { let match_spec = &match_specs[match_spec_id]; let Some(match_spec_name) = match_spec.name.as_ref() else { return Vec::new() }; - let Some(name_id) = names_to_ids.get(match_spec_name.as_normalized().as_ref()) else { return Vec::new() }; + let Some(name_id) = names_to_ids.get(match_spec_name) else { return Vec::new() }; packages_by_name[*name_id] .iter() diff --git a/crates/rattler_libsolv_rs/src/pool.rs b/crates/rattler_libsolv_rs/src/pool.rs index f164cd986..76f26627e 100644 --- a/crates/rattler_libsolv_rs/src/pool.rs +++ b/crates/rattler_libsolv_rs/src/pool.rs @@ -22,10 +22,10 @@ pub struct Pool<'a> { total_repos: u32, /// Interned package names - package_names: Arena<NameId, String>, + package_names: Arena<NameId, PackageName>, /// Map from package names to the id of their interned counterpart - pub(crate) names_to_ids: HashMap<String, NameId>, + pub(crate) names_to_ids: HashMap<PackageName, NameId>, /// Map from interned package names to the solvables that have that name pub(crate) packages_by_name: Mapping<NameId, Vec<SolvableId>>, @@ -136,10 +136,7 @@ impl<'a> Pool<'a> { ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec.name.as_ref().expect("match spec without name!"); - let name_id = match self - .names_to_ids - .get(match_spec_name.as_normalized().as_ref()) - { + let name_id = match self.names_to_ids.get(&match_spec_name) { None => return, Some(&name_id) => name_id, }; @@ -188,10 +185,7 @@ impl<'a> Pool<'a> { ) { let match_spec = &self.match_specs[match_spec_id]; let match_spec_name = match_spec.name.as_ref().expect("match spec without name!"); - let name_id = match self - .names_to_ids - .get(match_spec_name.as_normalized().as_ref()) - { + let name_id = match self.names_to_ids.get(&match_spec_name) { None => return, Some(&name_id) => name_id, }; @@ -228,12 +222,8 @@ impl<'a> Pool<'a> { } /// Interns a package name into the `Pool`, returning its `NameId` - fn intern_package_name<T: Into<PackageName>>(&mut self, name: T) -> NameId { - let package_name = name.into(); - match self - .names_to_ids - .entry(package_name.as_normalized().to_string()) - { + fn intern_package_name(&mut self, name: &PackageName) -> NameId { + match self.names_to_ids.entry(name.clone()) { Entry::Occupied(e) => *e.get(), Entry::Vacant(e) => { let next_id = self.package_names.alloc(e.key().clone()); @@ -250,7 +240,7 @@ impl<'a> Pool<'a> { /// Returns the package name associated to the provided id /// /// Panics if the package name is not found in the pool - pub fn resolve_package_name(&self, name_id: NameId) -> &str { + pub fn resolve_package_name(&self, name_id: NameId) -> &PackageName { &self.package_names[name_id] } diff --git a/crates/rattler_libsolv_rs/src/problem.rs b/crates/rattler_libsolv_rs/src/problem.rs index 8ba524554..3b810be88 100644 --- a/crates/rattler_libsolv_rs/src/problem.rs +++ b/crates/rattler_libsolv_rs/src/problem.rs @@ -100,7 +100,7 @@ impl Problem { .cloned() .find(|&ms| { let ms = solver.pool().resolve_match_spec(ms); - ms.name.as_ref().unwrap().as_normalized().as_ref() == dep.record.name + ms.name.as_ref().unwrap() == &dep.record.name }) .unwrap(); @@ -636,9 +636,13 @@ impl<'a> DisplayUnsat<'a> { let is_leaf = graph.edges(candidate).next().is_none(); if is_leaf { - writeln!(f, "{indent}{} {version}", solvable.record.name)?; + writeln!( + f, + "{indent}{} {version}", + solvable.record.name.as_normalized() + )?; } else if already_installed { - writeln!(f, "{indent}{} {version}, which conflicts with the versions reported above.", solvable.record.name)?; + writeln!(f, "{indent}{} {version}, which conflicts with the versions reported above.", solvable.record.name.as_normalized())?; } else if constrains_conflict { let match_specs = graph .edges(candidate) @@ -653,7 +657,7 @@ impl<'a> DisplayUnsat<'a> { writeln!( f, "{indent}{} {version} would constrain", - solvable.record.name + solvable.record.name.as_normalized() )?; let indent = Self::get_indent(depth + 1, top_level_indent); @@ -669,7 +673,7 @@ impl<'a> DisplayUnsat<'a> { writeln!( f, "{indent}{} {version} would require", - solvable.record.name + solvable.record.name.as_normalized() )?; let requirements = graph .edges(candidate) @@ -735,7 +739,8 @@ impl fmt::Display for DisplayUnsat<'_> { writeln!( f, "{indent}{} {} is locked, but another version is required as reported above", - locked.record.name, locked.record.version + locked.record.name.as_normalized(), + locked.record.version )?; } } diff --git a/crates/rattler_libsolv_rs/src/solvable.rs b/crates/rattler_libsolv_rs/src/solvable.rs index 37d0a7352..9bb68b703 100644 --- a/crates/rattler_libsolv_rs/src/solvable.rs +++ b/crates/rattler_libsolv_rs/src/solvable.rs @@ -73,7 +73,7 @@ impl<'a> Solvable<'a> { build: None, }, SolvableInner::Package(p) => SolvableDisplay { - name: &p.record.name, + name: p.record.name.as_normalized(), version: Some(&p.record.version), build: Some(&p.record.build), }, diff --git a/crates/rattler_libsolv_rs/src/solver/clause.rs b/crates/rattler_libsolv_rs/src/solver/clause.rs index 7b862656e..ad4e46e4b 100644 --- a/crates/rattler_libsolv_rs/src/solver/clause.rs +++ b/crates/rattler_libsolv_rs/src/solver/clause.rs @@ -434,7 +434,7 @@ impl Debug for ClauseDebug<'_> { .package() .record .name - .as_str(); + .as_normalized(); write!(f, "only one {name} allowed") } } diff --git a/crates/rattler_libsolv_rs/src/solver/mod.rs b/crates/rattler_libsolv_rs/src/solver/mod.rs index 246852d38..b43614497 100644 --- a/crates/rattler_libsolv_rs/src/solver/mod.rs +++ b/crates/rattler_libsolv_rs/src/solver/mod.rs @@ -923,7 +923,7 @@ mod test { license: None, license_family: None, md5: None, - name: name.to_string(), + name: name.parse().unwrap(), noarch: Default::default(), platform: None, sha256: None, @@ -1003,7 +1003,7 @@ mod test { .pool .resolve_solvable_inner(solved.steps[0]) .package(); - assert_eq!(solvable.record.name, "asdf"); + assert_eq!(solvable.record.name.as_normalized(), "asdf"); assert_eq!(solvable.record.version.to_string(), "1.2.3"); } @@ -1023,14 +1023,14 @@ mod test { .pool .resolve_solvable_inner(solved.steps[0]) .package(); - assert_eq!(solvable.record.name, "asdf"); + assert_eq!(solvable.record.name.as_normalized(), "asdf"); assert_eq!(solvable.record.version.to_string(), "1.2.3"); let solvable = solver .pool .resolve_solvable_inner(solved.steps[1]) .package(); - assert_eq!(solvable.record.name, "efgh"); + assert_eq!(solvable.record.name.as_normalized(), "efgh"); assert_eq!(solvable.record.version.to_string(), "4.5.6"); } @@ -1051,14 +1051,14 @@ mod test { .pool .resolve_solvable_inner(solved.steps[0]) .package(); - assert_eq!(solvable.record.name, "asdf"); + assert_eq!(solvable.record.name.as_normalized(), "asdf"); assert_eq!(solvable.record.version.to_string(), "1.2.4"); let solvable = solver .pool .resolve_solvable_inner(solved.steps[1]) .package(); - assert_eq!(solvable.record.name, "efgh"); + assert_eq!(solvable.record.name.as_normalized(), "efgh"); assert_eq!(solvable.record.version.to_string(), "4.5.7"); } @@ -1101,7 +1101,7 @@ mod test { .pool .resolve_solvable_inner(solved.steps[0]) .package(); - assert_eq!(solvable.record.name, "asdf"); + assert_eq!(solvable.record.name.as_normalized(), "asdf"); assert_eq!(solvable.record.version.to_string(), "1.2.3"); } @@ -1169,7 +1169,7 @@ mod test { .pool .resolve_solvable_inner(solved.steps[0]) .package(); - assert_eq!(solvable.record.name, "asdf"); + assert_eq!(solvable.record.name.as_normalized(), "asdf"); assert_eq!(solvable.record.version, Version::from_str("1.2.4").unwrap()); } diff --git a/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_missing_top_level_dep_2.snap b/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_missing_top_level_dep_2.snap index 12ca4df4e..977e8300f 100644 --- a/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_missing_top_level_dep_2.snap +++ b/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_missing_top_level_dep_2.snap @@ -1,6 +1,7 @@ --- source: crates/rattler_libsolv_rs/src/solver/mod.rs +assertion_line: 1306 expression: error --- -No candidates where found for b 14.*. +No candidates were found for b 14.*. diff --git a/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_1.snap b/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_1.snap index e2616024b..4b2e9a15b 100644 --- a/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_1.snap +++ b/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_1.snap @@ -1,8 +1,9 @@ --- source: crates/rattler_libsolv_rs/src/solver/mod.rs +assertion_line: 1281 expression: error --- asdf cannot be installed because there are no viable options: |-- asdf 1.2.3 would require - |-- C >1, for which no candidates were found. + |-- c >1, for which no candidates were found. diff --git a/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_2.snap b/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_2.snap index a75e5d80c..5beda1977 100644 --- a/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_2.snap +++ b/crates/rattler_libsolv_rs/src/solver/snapshots/rattler_libsolv_rs__solver__test__unsat_no_candidates_for_child_2.snap @@ -1,8 +1,9 @@ --- source: crates/rattler_libsolv_rs/src/solver/mod.rs +assertion_line: 1290 expression: error --- -A <1000 cannot be installed because there are no viable options: +a <1000 cannot be installed because there are no viable options: |-- a 41 would require - |-- B <20, for which no candidates where found. + |-- b <20, for which no candidates were found. diff --git a/crates/rattler_solve/src/libsolv_c/input.rs b/crates/rattler_solve/src/libsolv_c/input.rs index 867028401..25e28288e 100644 --- a/crates/rattler_solve/src/libsolv_c/input.rs +++ b/crates/rattler_solve/src/libsolv_c/input.rs @@ -84,7 +84,7 @@ pub fn add_repodata_records<'a>( let record = &repo_data.package_record; // Name and version - solvable.name = pool.intern_str(record.name.as_str()).into(); + solvable.name = pool.intern_str(record.name.as_normalized()).into(); solvable.evr = pool.intern_str(record.version.to_string()).into(); let rel_eq = pool.rel_eq(solvable.name, solvable.evr); repo.add_provides(solvable, rel_eq); @@ -248,7 +248,7 @@ pub fn add_virtual_packages(pool: &Pool, repo: &Repo, packages: &[GenericVirtual let solvable = unsafe { solvable_id.resolve_raw(pool).as_mut() }; // Name and version - solvable.name = pool.intern_str(package.name.as_str()).into(); + solvable.name = pool.intern_str(package.name.as_normalized()).into(); solvable.evr = pool.intern_str(package.version.to_string()).into(); let rel_eq = pool.rel_eq(solvable.name, solvable.evr); repo.add_provides(solvable, rel_eq); diff --git a/crates/rattler_solve/tests/backends.rs b/crates/rattler_solve/tests/backends.rs index b97230c6e..fd1dc2cf7 100644 --- a/crates/rattler_solve/tests/backends.rs +++ b/crates/rattler_solve/tests/backends.rs @@ -75,7 +75,7 @@ fn installed_package( channel: channel.to_string(), file_name: "dummy-filename".to_string(), package_record: PackageRecord { - name: name.to_string(), + name: name.parse().unwrap(), version: version.parse().unwrap(), build: build.to_string(), build_number, @@ -129,7 +129,9 @@ fn solve_real_world<T: SolverImpl + Default>(specs: Vec<&str>) -> Vec<String> { .map(|pkg| { format!( "{} {} {}", - pkg.package_record.name, pkg.package_record.version, pkg.package_record.build + pkg.package_record.name.as_normalized(), + pkg.package_record.version, + pkg.package_record.build ) }) .collect::<Vec<_>>(); @@ -215,7 +217,7 @@ macro_rules! solver_backend_tests { dummy_channel_json_path(), Vec::new(), vec![GenericVirtualPackage { - name: "__unix".to_string(), + name: rattler_conda_types::PackageName::new_unchecked("__unix"), version: Version::from_str("0").unwrap(), build_string: "0".to_string(), }], @@ -226,7 +228,7 @@ macro_rules! solver_backend_tests { assert_eq!(pkgs.len(), 1); let info = &pkgs[0]; - assert_eq!("bar", &info.package_record.name); + assert_eq!("bar", info.package_record.name.as_normalized()); assert_eq!("1.2.3", &info.package_record.version.to_string()); } @@ -249,7 +251,7 @@ macro_rules! solver_backend_tests { info.url.to_string() ); assert_eq!("https://conda.anaconda.org/conda-forge/", info.channel); - assert_eq!("foo", info.package_record.name); + assert_eq!("foo", info.package_record.name.as_normalized()); assert_eq!("linux-64", info.package_record.subdir); assert_eq!("3.0.2", info.package_record.version.to_string()); assert_eq!("py36h1af98f8_1", info.package_record.build); @@ -313,7 +315,7 @@ macro_rules! solver_backend_tests { // Install let info = &pkgs[0]; - assert_eq!("foo", &info.package_record.name); + assert_eq!("foo", info.package_record.name.as_normalized()); assert_eq!("3.0.2", &info.package_record.version.to_string()); } @@ -338,7 +340,7 @@ macro_rules! solver_backend_tests { // Install let info = &pkgs[0]; - assert_eq!("foo", &info.package_record.name); + assert_eq!("foo", info.package_record.name.as_normalized()); assert_eq!("4.0.2", &info.package_record.version.to_string()); } @@ -365,7 +367,7 @@ macro_rules! solver_backend_tests { // Uninstall let info = &pkgs[0]; - assert_eq!("foo", &info.package_record.name); + assert_eq!("foo", info.package_record.name.as_normalized()); assert_eq!("3.0.2", &info.package_record.version.to_string()); } @@ -443,7 +445,7 @@ mod libsolv_c { info.url.to_string() ); assert_eq!("https://conda.anaconda.org/conda-forge/", info.channel); - assert_eq!("foo", info.package_record.name); + assert_eq!("foo", info.package_record.name.as_normalized()); assert_eq!("linux-64", info.package_record.subdir); assert_eq!("3.0.2", info.package_record.version.to_string()); assert_eq!("py36h1af98f8_1", info.package_record.build); @@ -515,7 +517,7 @@ fn compare_solve(specs: Vec<&str>) { let names = specs .iter() .filter_map(|s| s.name.as_ref()) - .map(|name| name.as_normalized().as_ref()); + .map(|name| name.as_normalized()); let available_packages = SparseRepoData::load_records_recursive(sparse_repo_datas, names, None).unwrap(); @@ -525,7 +527,9 @@ fn compare_solve(specs: Vec<&str>) { .map(|pkg| { format!( "{} {} {}", - pkg.package_record.name, pkg.package_record.version, pkg.package_record.build + pkg.package_record.name.as_normalized(), + pkg.package_record.version, + pkg.package_record.build ) }) .collect::<Vec<_>>(); diff --git a/crates/rattler_virtual_packages/src/lib.rs b/crates/rattler_virtual_packages/src/lib.rs index d30542911..abda6d32e 100644 --- a/crates/rattler_virtual_packages/src/lib.rs +++ b/crates/rattler_virtual_packages/src/lib.rs @@ -34,7 +34,7 @@ pub mod linux; pub mod osx; use once_cell::sync::OnceCell; -use rattler_conda_types::{GenericVirtualPackage, Platform, Version}; +use rattler_conda_types::{GenericVirtualPackage, PackageName, Platform, Version}; use std::str::FromStr; use crate::osx::ParseOsxVersionError; @@ -71,12 +71,12 @@ impl From<VirtualPackage> for GenericVirtualPackage { fn from(package: VirtualPackage) -> Self { match package { VirtualPackage::Win => GenericVirtualPackage { - name: "__win".into(), + name: PackageName::new_unchecked("__win"), version: Version::from_str("0").unwrap(), build_string: "0".into(), }, VirtualPackage::Unix => GenericVirtualPackage { - name: "__unix".into(), + name: PackageName::new_unchecked("__unix"), version: Version::from_str("0").unwrap(), build_string: "0".into(), }, @@ -174,7 +174,7 @@ impl Linux { impl From<Linux> for GenericVirtualPackage { fn from(linux: Linux) -> Self { GenericVirtualPackage { - name: "__linux".into(), + name: PackageName::new_unchecked("__linux"), version: linux.version, build_string: "0".into(), } @@ -211,7 +211,9 @@ impl LibC { impl From<LibC> for GenericVirtualPackage { fn from(libc: LibC) -> Self { GenericVirtualPackage { - name: format!("__{}", libc.family.to_lowercase()), + name: format!("__{}", libc.family.to_lowercase()) + .try_into() + .unwrap(), version: libc.version, build_string: "0".into(), } @@ -242,7 +244,7 @@ impl Cuda { impl From<Cuda> for GenericVirtualPackage { fn from(cuda: Cuda) -> Self { GenericVirtualPackage { - name: "__cuda".into(), + name: PackageName::new_unchecked("__cuda"), version: cuda.version, build_string: "0".into(), } @@ -296,7 +298,7 @@ impl Archspec { impl From<Archspec> for GenericVirtualPackage { fn from(archspec: Archspec) -> Self { GenericVirtualPackage { - name: "__archspec".into(), + name: PackageName::new_unchecked("__archspec"), version: Version::from_str("1").unwrap(), build_string: archspec.spec, } @@ -330,7 +332,7 @@ impl Osx { impl From<Osx> for GenericVirtualPackage { fn from(osx: Osx) -> Self { GenericVirtualPackage { - name: "__osx".into(), + name: PackageName::new_unchecked("__osx"), version: osx.version, build_string: "0".into(), }