Stable order for virtual packages (#10024)
Some checks are pending
CI / test windows trampoline | x86_64 (push) Blocked by required conditions
CI / typos (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / build binary | linux (push) Blocked by required conditions
CI / build binary | macos aarch64 (push) Blocked by required conditions
CI / build binary | macos x86_64 (push) Blocked by required conditions
CI / integration test | conda on ubuntu (push) Blocked by required conditions
CI / integration test | free-threaded on linux (push) Blocked by required conditions
CI / integration test | github actions (push) Blocked by required conditions
CI / integration test | determine publish changes (push) Blocked by required conditions
CI / integration test | uv publish (push) Blocked by required conditions
CI / check cache | ubuntu (push) Blocked by required conditions
CI / check cache | macos aarch64 (push) Blocked by required conditions
CI / check system | python on debian (push) Blocked by required conditions
CI / check system | python on fedora (push) Blocked by required conditions
CI / check system | python on ubuntu (push) Blocked by required conditions
CI / check system | python on opensuse (push) Blocked by required conditions
CI / check system | homebrew python on macos aarch64 (push) Blocked by required conditions
CI / check system | python on macos x86_64 (push) Blocked by required conditions
CI / check system | python3.10 on windows (push) Blocked by required conditions
CI / check system | python3.10 on windows x86 (push) Blocked by required conditions
CI / check system | python3.13 on windows (push) Blocked by required conditions
CI / check system | python3.12 via chocolatey (push) Blocked by required conditions
CI / check system | conda3.11 on windows (push) Blocked by required conditions
CI / check system | conda3.8 on windows (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / lint (push) Waiting to run
CI / cargo clippy | ubuntu (push) Blocked by required conditions
CI / check system | amazonlinux (push) Blocked by required conditions
CI / cargo clippy | windows (push) Blocked by required conditions
CI / cargo dev generate-all (push) Blocked by required conditions
CI / cargo shear (push) Waiting to run
CI / cargo test | ubuntu (push) Blocked by required conditions
CI / cargo test | macos (push) Blocked by required conditions
CI / cargo test | windows (push) Blocked by required conditions
CI / check windows trampoline | aarch64 (push) Blocked by required conditions
CI / check windows trampoline | i686 (push) Blocked by required conditions
CI / check windows trampoline | x86_64 (push) Blocked by required conditions
CI / test windows trampoline | i686 (push) Blocked by required conditions
CI / build binary | windows (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / build binary | freebsd (push) Blocked by required conditions
CI / ecosystem test | prefecthq/prefect (push) Blocked by required conditions
CI / ecosystem test | pallets/flask (push) Blocked by required conditions
CI / integration test | free-threaded on windows (push) Blocked by required conditions
CI / integration test | pypy on ubuntu (push) Blocked by required conditions
CI / integration test | pypy on windows (push) Blocked by required conditions
CI / integration test | graalpy on ubuntu (push) Blocked by required conditions
CI / integration test | graalpy on windows (push) Blocked by required conditions
CI / check system | python on rocky linux 8 (push) Blocked by required conditions
CI / check system | python on rocky linux 9 (push) Blocked by required conditions
CI / check system | pypy on ubuntu (push) Blocked by required conditions
CI / check system | pyston (push) Blocked by required conditions
CI / check system | alpine (push) Blocked by required conditions
CI / check system | python on macos aarch64 (push) Blocked by required conditions
CI / check system | python3.9 via pyenv (push) Blocked by required conditions
CI / check system | python3.13 (push) Blocked by required conditions
CI / check system | conda3.11 on linux (push) Blocked by required conditions
CI / check system | conda3.8 on linux (push) Blocked by required conditions
CI / check system | conda3.11 on macos (push) Blocked by required conditions
CI / check system | conda3.8 on macos (push) Blocked by required conditions
CI / check system | embedded python3.10 on windows (push) Blocked by required conditions
CI / benchmarks (push) Blocked by required conditions

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 https://github.com/pubgrub-rs/pubgrub/pull/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.
This commit is contained in:
konsti 2024-12-20 10:28:46 +01:00 committed by GitHub
parent ff860296c5
commit ae659c8bfe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 179 additions and 63 deletions

View file

@ -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(

View file

@ -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 {
package_priority: FxHashMap<PackageName, PubGrubPriority>,
virtual_package_tiebreaker: FxHashMap<PubGrubPackage, u32>,
}
impl PubGrubPriorities {
/// Add a [`PubGrubPackage`] to the priority map.
@ -30,14 +33,22 @@ impl PubGrubPriorities {
version: &Range<Version>,
urls: &ForkUrls,
) {
let next = self.0.len();
if !self.virtual_package_tiebreaker.contains_key(package) {
self.virtual_package_tiebreaker.insert(
package.clone(),
u32::try_from(self.virtual_package_tiebreaker.len())
.expect("Less than 2**32 packages"),
);
}
let next = self.package_priority.len();
// The root package and Python constraints have no explicit priority, the root package is
// always first and the Python version (range) is fixed.
let Some(name) = package.name_no_root() else {
return;
};
match self.0.entry(name.clone()) {
match self.package_priority.entry(name.clone()) {
std::collections::hash_map::Entry::Occupied(mut entry) => {
// Preserve the original index.
let index = Self::get_index(&entry).unwrap_or(next);
@ -92,15 +103,21 @@ 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 package_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(),
}
PubGrubPackageInner::Marker { name, .. } => self.package_priority.get(name).copied(),
PubGrubPackageInner::Extra { name, .. } => self.package_priority.get(name).copied(),
PubGrubPackageInner::Dev { name, .. } => self.package_priority.get(name).copied(),
PubGrubPackageInner::Package { name, .. } => self.package_priority.get(name).copied(),
};
let virtual_package_tiebreaker = self
.virtual_package_tiebreaker
.get(package)
.copied()
.unwrap_or_default();
(package_priority, virtual_package_tiebreaker)
}
/// Mark a package as prioritized by setting it to [`PubGrubPriority::ConflictEarly`], if it
@ -109,7 +126,7 @@ impl PubGrubPriorities {
/// Returns whether the priority was changed, i.e., it's the first time we hit this condition
/// for the package.
pub(crate) fn mark_conflict_early(&mut self, package: &PubGrubPackage) -> bool {
let next = self.0.len();
let next = self.package_priority.len();
let Some(name) = package.name_no_root() else {
// Not a correctness bug
if cfg!(debug_assertions) {
@ -118,7 +135,7 @@ impl PubGrubPriorities {
return false;
}
};
match self.0.entry(name.clone()) {
match self.package_priority.entry(name.clone()) {
std::collections::hash_map::Entry::Occupied(mut entry) => {
if matches!(
entry.get(),
@ -144,7 +161,7 @@ impl PubGrubPriorities {
/// Returns whether the priority was changed, i.e., it's the first time this package was
/// marked as conflicting above the threshold.
pub(crate) fn mark_conflict_late(&mut self, package: &PubGrubPackage) -> bool {
let next = self.0.len();
let next = self.package_priority.len();
let Some(name) = package.name_no_root() else {
// Not a correctness bug
if cfg!(debug_assertions) {
@ -153,7 +170,7 @@ impl PubGrubPriorities {
return false;
}
};
match self.0.entry(name.clone()) {
match self.package_priority.entry(name.clone()) {
std::collections::hash_map::Entry::Occupied(mut entry) => {
// The ConflictEarly` match avoids infinite loops.
if matches!(