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

fix: ensure consistent sorting of locked packages #295

Merged
merged 2 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/rattler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ regex = "1.9.1"
reqwest = { version = "0.11.18", default-features = false, features = ["stream", "json", "gzip"] }
serde = { version = "1.0.171", features = ["derive"] }
serde_json = { version = "1.0.102", features = ["raw_value"] }
serde_with = "3.0.0"
serde_with = "3.3.0"
smallvec = { version = "1.11.0", features = ["serde", "const_new", "const_generics", "union"] }
tempfile = "3.6.0"
thiserror = "1.0.43"
Expand Down
3 changes: 2 additions & 1 deletion crates/rattler_conda_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ readme.workspace = true
chrono = "0.4.26"
fxhash = "0.2.1"
hex = "0.4.3"
indexmap = { version = "2.0.0", features = ["serde"] }
itertools = "0.11.0"
lazy-regex = "3.0.0"
nom = "7.1.3"
Expand All @@ -22,7 +23,7 @@ serde = { version = "1.0.171", features = ["derive"] }
serde_json = "1.0.102"
serde-json-python-formatter = "0.1.0"
serde_yaml = "0.9.22"
serde_with = "3.0.0"
serde_with = { version = "3.3.0", features = ["indexmap_2"] }
serde_repr = "0.1"
smallvec = { version = "1.11.0", features = ["serde", "const_new", "const_generics", "union"] }
strum = { version = "0.25.0", features = ["derive"] }
Expand Down
13 changes: 6 additions & 7 deletions crates/rattler_conda_types/src/conda_lock/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use crate::conda_lock::{
TimeMeta,
};
use crate::{MatchSpec, NamelessMatchSpec, NoArchType, Platform, RepoDataRecord};
use fxhash::{FxHashMap, FxHashSet};
use fxhash::{FxBuildHasher, FxHashMap, FxHashSet};
use indexmap::IndexMap;
use std::str::FromStr;
use url::Url;

Expand Down Expand Up @@ -76,7 +77,7 @@ impl LockFileBuilder {
content_hash::calculate_content_hash(plat, &self.input_specs, &self.channels)?,
))
})
.collect::<Result<FxHashMap<_, _>, CalculateContentHashError>>()?;
.collect::<Result<_, CalculateContentHashError>>()?;

let lock = CondaLock {
metadata: LockMeta {
Expand All @@ -94,7 +95,6 @@ impl LockFileBuilder {
.into_values()
.flat_map(|package| package.build())
.collect(),
version: super::default_version(),
};
Ok(lock)
}
Expand Down Expand Up @@ -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: IndexMap<String, NamelessMatchSpec, FxBuildHasher>,
/// Check if package is optional
pub optional: Option<bool>,

Expand Down Expand Up @@ -236,7 +236,7 @@ impl TryFrom<RepoDataRecord> for LockedPackage {
let hashes = hashes.ok_or_else(|| ConversionError::Missing("md5 or sha265".to_string()))?;

// Convert dependencies
let mut dependencies = FxHashMap::default();
let mut dependencies = IndexMap::default();
for match_spec_str in record.package_record.depends.iter() {
let matchspec = MatchSpec::from_str(match_spec_str)?;
let name = matchspec
Expand Down Expand Up @@ -385,7 +385,6 @@ impl LockedPackage {
#[cfg(test)]
mod tests {
use chrono::Utc;
use fxhash::FxHashMap;
use std::str::FromStr;

use crate::conda_lock::builder::{LockFileBuilder, LockedPackage, LockedPackages};
Expand All @@ -411,7 +410,7 @@ mod tests {
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: FromIterator::from_iter([("python".to_string(), NamelessMatchSpec::from_str("3.11.0.*").unwrap())]),
optional: None,
arch: Some("x86_64".to_string()),
subdir: Some("noarch".to_string()),
Expand Down
62 changes: 44 additions & 18 deletions crates/rattler_conda_types/src/conda_lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,27 @@ use crate::{
utils::serde::Ordered, NamelessMatchSpec, NoArchType, PackageRecord, ParsePlatformError,
ParseVersionError, Platform, RepoDataRecord,
};
use fxhash::FxHashMap;
use fxhash::FxBuildHasher;
use indexmap::IndexMap;
use rattler_digest::{serde::SerializableHash, Md5Hash, Sha256Hash};
use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer};
use serde_with::{serde_as, skip_serializing_none, DisplayFromStr};
use std::{fs::File, io::Read, path::Path, str::FromStr};
use std::{collections::BTreeMap, fs::File, io::Read, path::Path, str::FromStr};
use url::Url;

pub mod builder;
mod content_hash;

/// Default version for the conda-lock file format
const fn default_version() -> u32 {
1
}

/// Represents the conda-lock file
/// Contains the metadata regarding the lock files
/// also the locked packages
#[derive(Serialize, Deserialize, Clone, Debug)]
#[derive(Deserialize, Clone, Debug)]
pub struct CondaLock {
/// Metadata for the lock file
pub metadata: LockMeta,

/// Locked packages
pub package: Vec<LockedDependency>,

/// Version of the conda-lock file format
#[serde(default = "default_version")]
pub version: u32,
}

#[allow(missing_docs)]
Expand Down Expand Up @@ -98,7 +90,7 @@ impl CondaLock {
/// Metadata for the [`CondaLock`] file
pub struct LockMeta {
/// Hash of dependencies for each target platform
pub content_hash: FxHashMap<Platform, String>,
pub content_hash: BTreeMap<Platform, String>,
/// Channels used to resolve dependencies
pub channels: Vec<Channel>,
/// The platforms this lock file supports
Expand All @@ -111,9 +103,9 @@ pub struct LockMeta {
/// Metadata dealing with the git repo the lockfile was created in and the user that created it
pub git_metadata: Option<GitMeta>,
/// Metadata dealing with the input files used to create the lockfile
pub inputs_metadata: Option<FxHashMap<String, PackageHashes>>,
pub inputs_metadata: Option<IndexMap<String, PackageHashes>>,
/// Custom metadata provided by the user to be added to the lockfile
pub custom_metadata: Option<FxHashMap<String, String>>,
pub custom_metadata: Option<IndexMap<String, String>>,
}

/// Stores information about when the lockfile was generated
Expand Down Expand Up @@ -246,9 +238,8 @@ pub struct LockedDependency {
/// this actually represents the _full_ subdir (incl. arch))
pub platform: Platform,
/// What are its own dependencies mapping name to version constraint

#[serde_as(as = "FxHashMap<_, DisplayFromStr>")]
pub dependencies: FxHashMap<String, NamelessMatchSpec>,
#[serde_as(as = "IndexMap<_, DisplayFromStr, FxBuildHasher>")]
pub dependencies: IndexMap<String, NamelessMatchSpec, FxBuildHasher>,
/// URL to find it at
pub url: Url,
/// Hashes of the package
Expand Down Expand Up @@ -444,6 +435,41 @@ impl From<&str> for Channel {
}
}

impl Serialize for CondaLock {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
#[derive(Serialize)]
struct Raw<'a> {
metadata: &'a LockMeta,
package: Vec<&'a LockedDependency>,
version: u32,
}

// Sort all packages in alphabetical order. We choose to use alphabetic order instead of
// topological because the alphabetic order will create smaller diffs when packages change
// or are added.
// See: https://github.com/conda/conda-lock/issues/491
let mut sorted_deps = self.package.iter().collect::<Vec<_>>();
sorted_deps.sort_by(|&a, &b| {
a.name
.cmp(&b.name)
.then_with(|| a.platform.cmp(&b.platform))
.then_with(|| a.version.cmp(&b.version))
.then_with(|| a.build.cmp(&b.build))
});

let raw = Raw {
metadata: &self.metadata,
package: sorted_deps,
version: 1,
};

raw.serialize(serializer)
}
}

#[cfg(test)]
mod test {
use super::{channel_from_url, file_name_from_url, CondaLock, PackageHashes};
Expand Down
Loading