Skip to content

Commit

Permalink
Stable sorting of requirements.txt in universal mode (#5334)
Browse files Browse the repository at this point in the history
The `RequirementsTxtComparator` was written assuming there is one
distribution per package name. This changed with the universal
resolution, which allows multiple versions or urls for the same package
name. The sorting we emitted for these new entries was incidental.

With this change, we properly sort these entries by name, version and
then url in universal mode.

This is an output format change for `--universal` users.
  • Loading branch information
konstin committed Jul 23, 2024
1 parent 4308424 commit bea8bc6
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 10 deletions.
1 change: 1 addition & 0 deletions crates/uv-resolver/src/resolution/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ impl ResolutionGraph {
// Add the distribution to the graph.
let index = petgraph.add_node(ResolutionGraphNode::Dist(AnnotatedDist {
dist,
version: version.clone(),
extra: extra.clone(),
dev: dev.clone(),
hashes,
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-resolver/src/resolution/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt::Display;

use distribution_types::{DistributionMetadata, Name, ResolvedDist, VersionOrUrlRef};
use pep440_rs::Version;
use pypi_types::HashDigest;
use uv_distribution::Metadata;
use uv_normalize::{ExtraName, GroupName, PackageName};
Expand All @@ -20,6 +21,7 @@ mod requirements_txt;
#[derive(Debug, Clone)]
pub(crate) struct AnnotatedDist {
pub(crate) dist: ResolvedDist,
pub(crate) version: Version,
pub(crate) extra: Option<ExtraName>,
pub(crate) dev: Option<GroupName>,
pub(crate) hashes: Vec<HashDigest>,
Expand Down
25 changes: 23 additions & 2 deletions crates/uv-resolver/src/resolution/requirements_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::Path;
use itertools::Itertools;

use distribution_types::{DistributionMetadata, Name, ResolvedDist, Verbatim, VersionOrUrlRef};
use pep440_rs::Version;
use pep508_rs::{split_scheme, MarkerTree, Scheme};
use pypi_types::HashDigest;
use uv_normalize::{ExtraName, PackageName};
Expand All @@ -15,6 +16,7 @@ use crate::resolution::AnnotatedDist;
/// A pinned package with its resolved distribution and all the extras that were pinned for it.
pub(crate) struct RequirementsTxtDist {
pub(crate) dist: ResolvedDist,
pub(crate) version: Version,
pub(crate) extras: Vec<ExtraName>,
pub(crate) hashes: Vec<HashDigest>,
pub(crate) markers: Option<MarkerTree>,
Expand Down Expand Up @@ -133,14 +135,27 @@ impl RequirementsTxtDist {
}
}

RequirementsTxtComparator::Name(self.name())
if let VersionOrUrlRef::Url(url) = self.version_or_url() {
RequirementsTxtComparator::Name {
name: self.name(),
version: &self.version,
url: Some(url.verbatim()),
}
} else {
RequirementsTxtComparator::Name {
name: self.name(),
version: &self.version,
url: None,
}
}
}
}

impl From<&AnnotatedDist> for RequirementsTxtDist {
fn from(annotated: &AnnotatedDist) -> Self {
Self {
dist: annotated.dist.clone(),
version: annotated.version.clone(),
extras: if let Some(extra) = annotated.extra.clone() {
vec![extra]
} else {
Expand All @@ -155,7 +170,13 @@ impl From<&AnnotatedDist> for RequirementsTxtDist {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub(crate) enum RequirementsTxtComparator<'a> {
Url(Cow<'a, str>),
Name(&'a PackageName),
/// In universal mode, we can have multiple versions for a package, so we track the version and
/// the URL (for non-index packages) to have a stable sort for those, too.
Name {
name: &'a PackageName,
version: &'a Version,
url: Option<Cow<'a, str>>,
},
}

impl Name for RequirementsTxtDist {
Expand Down
16 changes: 8 additions & 8 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7250,13 +7250,13 @@ fn universal_nested_overlapping_local_requirement() -> Result<()> {
# via torch
tbb==2021.11.0 ; platform_machine != 'x86_64' and platform_system == 'Windows'
# via mkl
torch==2.3.0 ; platform_machine != 'x86_64'
# via -r requirements.in
torch==2.0.0+cu118 ; platform_machine == 'x86_64'
# via
# -r requirements.in
# example
# triton
torch==2.3.0 ; platform_machine != 'x86_64'
# via -r requirements.in
triton==2.0.0 ; platform_machine == 'x86_64' and platform_system == 'Linux'
# via torch
typing-extensions==4.10.0
Expand Down Expand Up @@ -7324,13 +7324,13 @@ fn universal_nested_overlapping_local_requirement() -> Result<()> {
# via torch
tbb==2021.11.0 ; platform_machine != 'x86_64' and platform_system == 'Windows'
# via mkl
torch==2.3.0 ; platform_machine != 'x86_64'
# via -r requirements.in
torch==2.0.0+cu118 ; platform_machine == 'x86_64'
# via
# -r requirements.in
# example
# triton
torch==2.3.0 ; platform_machine != 'x86_64'
# via -r requirements.in
triton==2.0.0 ; platform_machine == 'x86_64' and platform_system == 'Linux'
# via torch
typing-extensions==4.10.0
Expand Down Expand Up @@ -7440,17 +7440,17 @@ fn universal_nested_disjoint_local_requirement() -> Result<()> {
# via torch
tbb==2021.11.0 ; os_name != 'Linux' and platform_system == 'Windows'
# via mkl
torch==2.0.0+cpu ; os_name == 'Linux'
# via
# -r requirements.in
# example
torch==2.0.0+cu118 ; os_name == 'Linux'
# via
# -r requirements.in
# example
# triton
torch==2.3.0 ; os_name != 'Linux'
# via -r requirements.in
torch==2.0.0+cpu ; os_name == 'Linux'
# via
# -r requirements.in
# example
triton==2.0.0 ; os_name == 'Linux' and platform_machine == 'x86_64' and platform_system == 'Linux'
# via torch
typing-extensions==4.10.0
Expand Down

0 comments on commit bea8bc6

Please sign in to comment.