Skip to content

Commit

Permalink
Merge pull request #295 from baszalmstra/fix/consistent_sorting_of_lo…
Browse files Browse the repository at this point in the history
…ckfile

fix: ensure consistent sorting of locked packages
  • Loading branch information
tdejager authored Aug 28, 2023
2 parents 45126b9 + aa93aa1 commit fd4029f
Show file tree
Hide file tree
Showing 9 changed files with 9,056 additions and 9,027 deletions.
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

0 comments on commit fd4029f

Please sign in to comment.