Skip to content

Commit

Permalink
Prioritize forks based on Python narrowing (#5642)
Browse files Browse the repository at this point in the history
## Summary

First part of: #4926. We should
solve forks that _don't_ expand the world of supported versions (e.g.,
`python_version >= '3.11'` enables us to select new packages, since we
narrow the supported version range).
  • Loading branch information
charliermarsh authored Jul 31, 2024
1 parent 0dcec9e commit f268b7c
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 24 deletions.
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use uv_normalize::{ExtraName, PackageName};
use crate::pubgrub::{PubGrubPackage, PubGrubPackageInner};
use crate::{PubGrubSpecifier, ResolveError};

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) struct PubGrubDependency {
pub(crate) package: PubGrubPackage,
pub(crate) version: Range<Version>,
Expand Down
6 changes: 6 additions & 0 deletions crates/uv-resolver/src/requires_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,12 @@ impl<'de> serde::Deserialize<'de> for RequiresPython {
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct RequiresPythonBound(Bound<Version>);

impl Default for RequiresPythonBound {
fn default() -> Self {
Self(Bound::Unbounded)
}
}

impl RequiresPythonBound {
pub fn new(bound: Bound<Version>) -> Self {
Self(match bound {
Expand Down
27 changes: 26 additions & 1 deletion crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Given a set of requirements, find a set of compatible packages.
use std::borrow::Cow;
use std::cmp::Ordering;
use std::collections::hash_map::Entry;
use std::collections::{BTreeMap, BTreeSet, VecDeque};
use std::fmt::{Display, Formatter, Write};
Expand Down Expand Up @@ -576,6 +577,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
state.markers,
start.elapsed().as_secs_f32()
);

for new_fork_state in self.forks_to_fork_states(
state,
&version,
Expand Down Expand Up @@ -2766,6 +2768,12 @@ impl Dependencies {
forks = new_forks;
diverging_packages.push(name.clone());
}

// Prioritize the forks. Prefer solving forks with lower Python bounds, since they're more
// likely to produce solutions that work for forks with higher Python bounds (whereas the
// inverse is not true).
forks.sort();

ForkedDependencies::Forked {
forks,
diverging_packages,
Expand Down Expand Up @@ -2808,7 +2816,7 @@ enum ForkedDependencies {
/// have the same name and because the marker expressions are disjoint,
/// a fork occurs. One fork will contain `a<2` but not `a>=2`, while
/// the other fork will contain `a>=2` but not `a<2`.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Eq, PartialEq)]
struct Fork {
/// The list of dependencies for this fork, guaranteed to be conflict
/// free. (i.e., There are no two packages with the same name with
Expand All @@ -2829,6 +2837,23 @@ struct Fork {
markers: MarkerTree,
}

impl Ord for Fork {
fn cmp(&self, other: &Self) -> Ordering {
// A higher `requires-python` requirement indicates a _lower-priority_ fork. We'd prefer
// to solve `<3.7` before solving `>=3.7`, since the resolution produced by the former might
// work for the latter, but the inverse is unlikely to be true.
let self_bound = requires_python_marker(&self.markers).unwrap_or_default();
let other_bound = requires_python_marker(&other.markers).unwrap_or_default();
other_bound.cmp(&self_bound)
}
}

impl PartialOrd for Fork {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl Fork {
/// Add the given dependency to this fork.
///
Expand Down
30 changes: 8 additions & 22 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7867,9 +7867,7 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] requirements.in -p 3.8 --universal
alabaster==0.7.13 ; python_version < '3.11'
# via sphinx
alabaster==0.7.16 ; python_version >= '3.11'
alabaster==0.7.13
# via sphinx
astroid==3.1.0
# via pylint
Expand Down Expand Up @@ -7915,31 +7913,19 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
# via sphinx
snowballstemmer==2.2.0
# via sphinx
sphinx==7.1.2 ; python_version < '3.11'
# via -r requirements.in
sphinx==7.2.6 ; python_version >= '3.11'
sphinx==7.1.2
# via -r requirements.in
sphinxcontrib-applehelp==1.0.4 ; python_version < '3.11'
# via sphinx
sphinxcontrib-applehelp==1.0.8 ; python_version >= '3.11'
# via sphinx
sphinxcontrib-devhelp==1.0.2 ; python_version < '3.11'
sphinxcontrib-applehelp==1.0.4
# via sphinx
sphinxcontrib-devhelp==1.0.6 ; python_version >= '3.11'
sphinxcontrib-devhelp==1.0.2
# via sphinx
sphinxcontrib-htmlhelp==2.0.1 ; python_version < '3.11'
# via sphinx
sphinxcontrib-htmlhelp==2.0.5 ; python_version >= '3.11'
sphinxcontrib-htmlhelp==2.0.1
# via sphinx
sphinxcontrib-jsmath==1.0.1
# via sphinx
sphinxcontrib-qthelp==1.0.3 ; python_version < '3.11'
# via sphinx
sphinxcontrib-qthelp==1.0.7 ; python_version >= '3.11'
# via sphinx
sphinxcontrib-serializinghtml==1.1.5 ; python_version < '3.11'
sphinxcontrib-qthelp==1.0.3
# via sphinx
sphinxcontrib-serializinghtml==1.1.10 ; python_version >= '3.11'
sphinxcontrib-serializinghtml==1.1.5
# via sphinx
tomli==2.0.1 ; python_version < '3.11'
# via pylint
Expand All @@ -7956,7 +7942,7 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
----- stderr -----
warning: The requested Python version 3.8 is not available; 3.12.[X] will be used to build dependencies instead.
Resolved 41 packages in [TIME]
Resolved 34 packages in [TIME]
"###
);

Expand Down

0 comments on commit f268b7c

Please sign in to comment.