Skip to content

Commit

Permalink
Use sets rather than vectors for lockfile requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Aug 15, 2024
1 parent 3187dc1 commit f68e27c
Show file tree
Hide file tree
Showing 12 changed files with 450 additions and 299 deletions.
13 changes: 12 additions & 1 deletion crates/pep440-rs/src/version_specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,18 @@ use crate::{
/// // VersionSpecifiers derefs into a list of specifiers
/// assert_eq!(version_specifiers.iter().position(|specifier| *specifier.operator() == Operator::LessThan), Some(1));
/// ```
#[derive(Eq, PartialEq, Debug, Clone, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)]
#[derive(
Eq,
PartialEq,
Ord,
PartialOrd,
Debug,
Clone,
Hash,
rkyv::Archive,
rkyv::Deserialize,
rkyv::Serialize,
)]
#[archive(check_bytes)]
#[archive_attr(derive(Debug))]
#[cfg_attr(feature = "pyo3", pyclass(sequence))]
Expand Down
8 changes: 6 additions & 2 deletions crates/pypi-types/src/requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ pub enum RequirementError {
///
/// The main change is using [`RequirementSource`] to represent all supported package sources over
/// [`VersionOrUrl`], which collapses all URL sources into a single stringly type.
#[derive(Hash, Debug, Clone, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
#[derive(
Hash, Debug, Clone, Eq, PartialEq, Ord, PartialOrd, serde::Serialize, serde::Deserialize,
)]
pub struct Requirement {
pub name: PackageName,
#[serde(skip_serializing_if = "Vec::is_empty", default)]
Expand Down Expand Up @@ -294,7 +296,9 @@ impl Display for Requirement {
/// We store both the parsed fields (such as the plain url and the subdirectory) and the joined
/// PEP 508 style url (e.g. `file:///<path>#subdirectory=<subdirectory>`) since we need both in
/// different locations.
#[derive(Hash, Debug, Clone, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
#[derive(
Hash, Debug, Clone, Eq, PartialEq, Ord, PartialOrd, serde::Serialize, serde::Deserialize,
)]
#[serde(try_from = "RequirementSourceWire", into = "RequirementSourceWire")]
pub enum RequirementSource {
/// The requirement has a version specifier, such as `foo >1,<2`.
Expand Down
59 changes: 34 additions & 25 deletions crates/uv-resolver/src/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,9 @@ impl Lock {

// Validate that the lockfile was generated with the same root members.
{
let expected = members;
let expected = members.iter().cloned().collect::<BTreeSet<_>>();
let actual = &self.manifest.members;
if expected != actual {
if expected != *actual {
debug!(
"Mismatched members:\n expected: {:?}\n found: {:?}",
expected, actual
Expand All @@ -693,12 +693,12 @@ impl Lock {

// Validate that the lockfile was generated with the same constraints.
{
let expected: Vec<_> = constraints
let expected: BTreeSet<_> = constraints
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
let actual: Vec<_> = self
let actual: BTreeSet<_> = self
.manifest
.constraints
.iter()
Expand All @@ -716,12 +716,12 @@ impl Lock {

// Validate that the lockfile was generated with the same overrides.
{
let expected: Vec<_> = overrides
let expected: BTreeSet<_> = overrides
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
let actual: Vec<_> = self
let actual: BTreeSet<_> = self
.manifest
.overrides
.iter()
Expand Down Expand Up @@ -772,13 +772,13 @@ impl Lock {

// Validate the `requires-dist` metadata.
{
let expected: Vec<_> = archive
let expected: BTreeSet<_> = archive
.metadata
.requires_dist
.into_iter()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
let actual: Vec<_> = package
let actual: BTreeSet<_> = package
.metadata
.requires_dist
.iter()
Expand All @@ -797,7 +797,7 @@ impl Lock {

// Validate the `dev-dependencies` metadata.
{
let expected: BTreeMap<GroupName, Vec<Requirement>> = archive
let expected: BTreeMap<GroupName, BTreeSet<Requirement>> = archive
.metadata
.dev_dependencies
.into_iter()
Expand All @@ -811,7 +811,7 @@ impl Lock {
)
})
.collect();
let actual: BTreeMap<GroupName, Vec<Requirement>> = package
let actual: BTreeMap<GroupName, BTreeSet<Requirement>> = package
.metadata
.requires_dev
.iter()
Expand Down Expand Up @@ -888,25 +888,25 @@ struct ResolverOptions {
pub struct ResolverManifest {
/// The workspace members included in the lockfile.
#[serde(default)]
members: Vec<PackageName>,
members: BTreeSet<PackageName>,
/// The constraints provided to the resolver.
#[serde(default)]
constraints: Vec<Requirement>,
constraints: BTreeSet<Requirement>,
/// The overrides provided to the resolver.
#[serde(default)]
overrides: Vec<Requirement>,
overrides: BTreeSet<Requirement>,
}

impl ResolverManifest {
pub fn new(
members: Vec<PackageName>,
constraints: Vec<Requirement>,
overrides: Vec<Requirement>,
members: impl IntoIterator<Item = PackageName>,
constraints: impl IntoIterator<Item = Requirement>,
overrides: impl IntoIterator<Item = Requirement>,
) -> Self {
Self {
members,
constraints,
overrides,
members: members.into_iter().collect(),
constraints: constraints.into_iter().collect(),
overrides: overrides.into_iter().collect(),
}
}
}
Expand Down Expand Up @@ -1010,14 +1010,24 @@ impl Package {
let sdist = SourceDist::from_annotated_dist(&id, annotated_dist)?;
let wheels = Wheel::from_annotated_dist(annotated_dist)?;
let requires_dist = if matches!(id.source, Source::Registry(..)) {
vec![]
BTreeSet::default()
} else {
annotated_dist.metadata.requires_dist.clone()
annotated_dist
.metadata
.requires_dist
.iter()
.cloned()
.collect()
};
let requires_dev = if matches!(id.source, Source::Registry(..)) {
BTreeMap::default()
} else {
annotated_dist.metadata.dev_dependencies.clone()
annotated_dist
.metadata
.dev_dependencies
.iter()
.map(|(k, v)| (k.clone(), v.iter().cloned().collect()))
.collect()
};
Ok(Package {
id,
Expand All @@ -1029,7 +1039,6 @@ impl Package {
dev_dependencies: BTreeMap::default(),
metadata: PackageMetadata {
requires_dist,

requires_dev,
},
})
Expand Down Expand Up @@ -1511,9 +1520,9 @@ struct PackageWire {
#[serde(rename_all = "kebab-case")]
struct PackageMetadata {
#[serde(default)]
requires_dist: Vec<Requirement>,
requires_dist: BTreeSet<Requirement>,
#[serde(default)]
requires_dev: BTreeMap<GroupName, Vec<Requirement>>,
requires_dev: BTreeMap<GroupName, BTreeSet<Requirement>>,
}

impl PackageWire {
Expand Down
6 changes: 3 additions & 3 deletions crates/uv/tests/branching_urls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ fn root_package_splits_transitive_too() -> Result<()> {
[package.metadata]
requires-dist = [
{ name = "anyio", marker = "python_version >= '3.12'", specifier = "==4.3.0" },
{ name = "anyio", marker = "python_version < '3.12'", specifier = "==4.2.0" },
{ name = "anyio", marker = "python_version >= '3.12'", specifier = "==4.3.0" },
{ name = "b", directory = "b" },
]
Expand Down Expand Up @@ -424,8 +424,8 @@ fn root_package_splits_other_dependencies_too() -> Result<()> {
[package.metadata]
requires-dist = [
{ name = "anyio", marker = "python_version >= '3.12'", specifier = "==4.3.0" },
{ name = "anyio", marker = "python_version < '3.12'", specifier = "==4.2.0" },
{ name = "anyio", marker = "python_version >= '3.12'", specifier = "==4.3.0" },
{ name = "b1", marker = "python_version < '3.12'", directory = "b1" },
{ name = "b2", marker = "python_version >= '3.12'", directory = "b2" },
]
Expand Down Expand Up @@ -791,8 +791,8 @@ fn dont_pre_visit_url_packages() -> Result<()> {
[package.metadata]
requires-dist = [
{ name = "c", specifier = "==0.1.0" },
{ name = "b", directory = "b" },
{ name = "c", specifier = "==0.1.0" },
]
[[package]]
Expand Down
6 changes: 3 additions & 3 deletions crates/uv/tests/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1926,12 +1926,12 @@ fn update() -> Result<()> {
[package.metadata]
requires-dist = [
{ name = "certifi", specifier = ">=2017.4.17" },
{ name = "chardet", marker = "extra == 'use-chardet-on-py3'", specifier = "<6,>=3.0.2" },
{ name = "charset-normalizer", specifier = "<4,>=2" },
{ name = "idna", specifier = "<4,>=2.5" },
{ name = "urllib3", specifier = "<3,>=1.21.1" },
{ name = "certifi", specifier = ">=2017.4.17" },
{ name = "pysocks", marker = "extra == 'socks'", specifier = "!=1.5.7,>=1.5.6" },
{ name = "chardet", marker = "extra == 'use-chardet-on-py3'", specifier = "<6,>=3.0.2" },
{ name = "urllib3", specifier = "<3,>=1.21.1" },
]
[[package]]
Expand Down
Loading

0 comments on commit f68e27c

Please sign in to comment.