Skip to content

Commit

Permalink
Stable order for virtual packages
Browse files Browse the repository at this point in the history
uv gives priorities to packages by package name, not by virtual package (`PubGrubPackage`). pubgrub otoh when prioritizing order the virtual packages. When the order of virtual packages changes, uv changes its resolutions and error messages. This means uv was depending on implementation details of pubgrub's prioritization caching.

This broke with pubgrub-rs/pubgrub#299, which added a tiebreaker term that made pubgrub's sorting deterministic given a deterministic ordering of allocating the packages (which happens the first time pubgrub sees a package).

The new custom tiebreaker decreases the difference to upstream pubgrub.
  • Loading branch information
konstin committed Dec 19, 2024
1 parent e65a273 commit f9aca63
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 47 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ petgraph = { version = "0.6.5" }
platform-info = { version = "2.0.3" }
proc-macro2 = { version = "1.0.86" }
procfs = { version = "0.17.0", default-features = false, features = ["flate2"] }
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "05e8d12cea8d72c6d2d017900e478d0abd28fef4" }
pubgrub = { path = "../pubgrub" }
#pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "05e8d12cea8d72c6d2d017900e478d0abd28fef4" }
quote = { version = "1.0.37" }
rayon = { version = "1.10.0" }
reflink-copy = { version = "0.1.19" }
Expand Down Expand Up @@ -175,7 +176,8 @@ unicode-width = { version = "0.1.13" }
unscanny = { version = "0.1.0" }
url = { version = "2.5.2", features = ["serde"] }
urlencoding = { version = "2.1.3" }
version-ranges = { git = "https://github.com/astral-sh/pubgrub", rev = "05e8d12cea8d72c6d2d017900e478d0abd28fef4" }
version-ranges = { path = "../pubgrub/version-ranges" }
#version-ranges = { git = "https://github.com/astral-sh/pubgrub", rev = "05e8d12cea8d72c6d2d017900e478d0abd28fef4" }
walkdir = { version = "2.5.0" }
which = { version = "7.0.0", features = ["regex"] }
windows-registry = { version = "0.3.0" }
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-resolver/src/dependency_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ impl DependencyProvider for UvDependencyProvider {
type V = Version;
type VS = Range<Version>;
type M = UnavailableReason;
type Priority = Option<PubGrubPriority>;
/// Main priority and tiebreak for virtual packages
type Priority = (Option<PubGrubPriority>, u32);
type Err = Infallible;

fn prioritize(
Expand Down
20 changes: 16 additions & 4 deletions crates/uv-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ use crate::pubgrub::PubGrubPackageInner;
///
/// See: <https://github.com/pypa/pip/blob/ef78c129b1a966dbbbdb8ebfffc43723e89110d1/src/pip/_internal/resolution/resolvelib/provider.py#L120>
#[derive(Clone, Debug, Default)]
pub(crate) struct PubGrubPriorities(FxHashMap<PackageName, PubGrubPriority>);
pub(crate) struct PubGrubPriorities(
FxHashMap<PackageName, PubGrubPriority>,
FxHashMap<PubGrubPackage, u32>,
);

impl PubGrubPriorities {
/// Add a [`PubGrubPackage`] to the priority map.
Expand All @@ -30,6 +33,13 @@ impl PubGrubPriorities {
version: &Range<Version>,
urls: &ForkUrls,
) {
if !self.1.contains_key(&package) {
self.1.insert(
package.clone(),
u32::try_from(self.1.len()).expect("Less than 2**32 packages"),
);
}

let next = self.0.len();
// The root package and Python constraints have no explicit priority, the root package is
// always first and the Python version (range) is fixed.
Expand Down Expand Up @@ -92,15 +102,17 @@ impl PubGrubPriorities {
}

/// Return the [`PubGrubPriority`] of the given package, if it exists.
pub(crate) fn get(&self, package: &PubGrubPackage) -> Option<PubGrubPriority> {
match &**package {
pub(crate) fn get(&self, package: &PubGrubPackage) -> (Option<PubGrubPriority>, u32) {
let main_priority = match &**package {
PubGrubPackageInner::Root(_) => Some(PubGrubPriority::Root),
PubGrubPackageInner::Python(_) => Some(PubGrubPriority::Root),
PubGrubPackageInner::Marker { name, .. } => self.0.get(name).copied(),
PubGrubPackageInner::Extra { name, .. } => self.0.get(name).copied(),
PubGrubPackageInner::Dev { name, .. } => self.0.get(name).copied(),
PubGrubPackageInner::Package { name, .. } => self.0.get(name).copied(),
}
};
let tiebreaker = self.1.get(&package).copied().unwrap_or_default();
(main_priority, tiebreaker)
}

/// Mark a package as prioritized by setting it to [`PubGrubPriority::ConflictEarly`], if it
Expand Down
16 changes: 7 additions & 9 deletions crates/uv/tests/it/lock_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ fn extra_basic_three_extras() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because project[project3] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.2.0, we can conclude that project[extra1] and project[project3] are incompatible.
And because your project requires project[extra1] and project[project3], we can conclude that your project's requirements are unsatisfiable.
╰─▶ Because project[project3] depends on sortedcontainers==2.4.0 and project[extra2] depends on sortedcontainers==2.3.0, we can conclude that project[extra2] and project[project3] are incompatible.
And because your project requires project[extra2] and project[project3], we can conclude that your project's requirements are unsatisfiable.
"###);

// And now with the same extra configuration, we tell uv about
Expand Down Expand Up @@ -538,8 +538,8 @@ fn extra_multiple_not_conflicting2() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because project[project4] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.3.0, we can conclude that project[extra1] and project[project4] are incompatible.
And because your project requires project[extra1] and project[project4], we can conclude that your project's requirements are unsatisfiable.
╰─▶ Because project[project4] depends on sortedcontainers==2.4.0 and project[project3] depends on sortedcontainers==2.3.0, we can conclude that project[project3] and project[project4] are incompatible.
And because your project requires project[project3] and project[project4], we can conclude that your project's requirements are unsatisfiable.
"###);

// If we define extra1/extra2 as conflicting and project3/project4
Expand Down Expand Up @@ -1289,10 +1289,8 @@ fn extra_nested_across_workspace() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because dummy[extra2] depends on proxy1[extra2] and only proxy1[extra2]==0.1.0 is available, we can conclude that dummy[extra2] depends on proxy1[extra2]==0.1.0. (1)
Because proxy1[extra2]==0.1.0 depends on anyio==4.2.0 and proxy1[extra1]==0.1.0 depends on anyio==4.1.0, we can conclude that proxy1[extra1]==0.1.0 and proxy1[extra2]==0.1.0 are incompatible.
And because we know from (1) that dummy[extra2] depends on proxy1[extra2]==0.1.0, we can conclude that dummy[extra2] and proxy1[extra1]==0.1.0 are incompatible.
╰─▶ Because dummy[extra2] depends on proxy1[extra2] and only proxy1[extra2]==0.1.0 is available, we can conclude that dummy[extra2] depends on proxy1[extra2]==0.1.0.
And because proxy1[extra2]==0.1.0 depends on anyio==4.2.0 and proxy1[extra1]==0.1.0 depends on anyio==4.1.0, we can conclude that proxy1[extra1]==0.1.0 and dummy[extra2] are incompatible.
And because only proxy1[extra1]==0.1.0 is available and dummysub[extra1] depends on proxy1[extra1], we can conclude that dummysub[extra1] and dummy[extra2] are incompatible.
And because your workspace requires dummy[extra2] and dummysub[extra1], we can conclude that your workspace's requirements are unsatisfiable.
"###);
Expand Down Expand Up @@ -1795,7 +1793,7 @@ fn mixed() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because project[extra1] depends on sortedcontainers==2.4.0 and project:group1 depends on sortedcontainers==2.3.0, we can conclude that project:group1 and project[extra1] are incompatible.
╰─▶ Because project:group1 depends on sortedcontainers==2.3.0 and project[extra1] depends on sortedcontainers==2.4.0, we can conclude that project[extra1] and project:group1 are incompatible.
And because your project requires project[extra1] and project:group1, we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down
Loading

0 comments on commit f9aca63

Please sign in to comment.