Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: normalize package names where applicable #285

Merged
merged 12 commits into from
Aug 28, 2023
40 changes: 24 additions & 16 deletions crates/rattler-bin/src/commands/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,13 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> {
.collect::<Result<Vec<_>, _>>()?;

// Get the package names from the matchspecs so we can only load the package records that we need.
let package_names = specs.iter().filter_map(|spec| spec.name.as_ref());
let package_names = specs.iter().filter_map(|spec| spec.name.as_ref().cloned());
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());
}
}),
Expand All @@ -179,24 +179,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)
}
})?;

Expand Down Expand Up @@ -247,7 +249,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
)
};

Expand Down Expand Up @@ -492,7 +496,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
));
Expand Down Expand Up @@ -536,7 +544,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
));
Expand Down Expand Up @@ -620,7 +628,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());
}
}),
Expand Down
2 changes: 1 addition & 1 deletion crates/rattler/src/install/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/rattler/src/package_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down
26 changes: 13 additions & 13 deletions crates/rattler_conda_types/src/conda_lock/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{FxBuildHasher, FxHashMap, FxHashSet};
use indexmap::IndexMap;
use std::str::FromStr;
Expand Down Expand Up @@ -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
Expand All @@ -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: IndexMap<String, NamelessMatchSpec, FxBuildHasher>,
pub dependency_list: IndexMap<PackageName, NamelessMatchSpec, FxBuildHasher>,
/// Check if package is optional
pub optional: Option<bool>,

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -390,7 +389,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;

Expand All @@ -404,13 +404,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: FromIterator::from_iter([("python".to_string(), NamelessMatchSpec::from_str("3.11.0.*").unwrap())]),
dependency_list: FromIterator::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()),
Expand Down
11 changes: 8 additions & 3 deletions crates/rattler_conda_types/src/conda_lock/content_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,14 @@ pub fn calculate_content_data(
.iter()
.map(|spec| {
Ok(CondaLockVersionedDependency {
name: spec.name.clone().ok_or_else(|| {
CalculateContentHashError::RequiredAttributeMissing("name".to_string())
})?,
name: spec
.name
.clone()
.ok_or_else(|| {
CalculateContentHashError::RequiredAttributeMissing("name".to_string())
})?
.as_source()
.to_string(),
manager: "conda".to_string(),
optional: false,
category: "main".to_string(),
Expand Down
13 changes: 8 additions & 5 deletions crates/rattler_conda_types/src/conda_lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::FxBuildHasher;
use indexmap::IndexMap;
use rattler_digest::{serde::SerializableHash, Md5Hash, Sha256Hash};
Expand Down Expand Up @@ -229,7 +229,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
Expand All @@ -239,7 +239,7 @@ pub struct LockedDependency {
pub platform: Platform,
/// What are its own dependencies mapping name to version constraint
#[serde_as(as = "IndexMap<_, DisplayFromStr, FxBuildHasher>")]
pub dependencies: IndexMap<String, NamelessMatchSpec, FxBuildHasher>,
pub dependencies: IndexMap<PackageName, NamelessMatchSpec, FxBuildHasher>,
/// URL to find it at
pub url: Url,
/// Hashes of the package
Expand Down Expand Up @@ -618,12 +618,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()
Expand Down
12 changes: 9 additions & 3 deletions crates/rattler_conda_types/src/generic_virtual_package.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
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
/// `version` and a `build_string`.
#[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,
Expand All @@ -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
)
}
}
2 changes: 2 additions & 0 deletions crates/rattler_conda_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub mod version_spec;
pub mod conda_lock;
mod generic_virtual_package;
pub mod package;
mod package_name;
pub mod prefix_record;

pub use channel::{Channel, ChannelConfig, ParseChannelError};
Expand All @@ -31,6 +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::{InvalidPackageNameError, PackageName};
pub use platform::{ParsePlatformError, Platform};
pub use prefix_record::PrefixRecord;
pub use repo_data::patches::{PackageRecordPatch, PatchInstructions, RepoDataPatch};
Expand Down
Loading