Respect visitation order for proxy packages (#10833)

## Summary

This PR reverts https://github.com/astral-sh/uv/pull/10441 and applies a
different fix for https://github.com/astral-sh/uv/issues/10425.

In #10441, I changed prioritization to visit proxies eagerly. I think
this is actually wrong, since it means we prioritize proxy packages
above _everything_ else. And while a proxy only depends on itself, it
does mean we're selecting a _version_ for the proxy package earlier than
anything else. So, if you look at #10828, we end up choosing a version
for `async-timeout` before we choose a version for `langchain`, despite
the latter being a first-party dependency. (`async-timeout` has a marker
on it, so it has a proxy package, so we solve for it first.)

To fix #10425, we instead need to make sure we visit proxies in the
order we see them. I think the virtual tiebreaker for proxies is
reversed? We want to visit the package we see first, first.

So, in short: this reverts #10441, then corrects the ordering for
visiting proxies.

Closes https://github.com/astral-sh/uv/issues/10828.
This commit is contained in:
Charlie Marsh 2025-01-22 12:12:47 -05:00 committed by GitHub
parent e02f061ea9
commit f0dc1f4dd1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 713 additions and 178 deletions

View file

@ -4,7 +4,7 @@ use pubgrub::{Dependencies, DependencyProvider, PackageResolutionStatistics, Ran
use uv_pep440::Version;
use crate::pubgrub::{PubGrubPackage, PubGrubPriority};
use crate::pubgrub::{PubGrubPackage, PubGrubPriority, PubGrubTiebreaker};
use crate::resolver::UnavailableReason;
/// We don't use a dependency provider, we interact with state directly, but we still need this one
@ -17,8 +17,8 @@ impl DependencyProvider for UvDependencyProvider {
type V = Version;
type VS = Range<Version>;
type M = UnavailableReason;
/// Main priority and tiebreak for virtual packages
type Priority = (Option<PubGrubPriority>, u32);
/// Main priority and tiebreak for virtual packages.
type Priority = (Option<PubGrubPriority>, Option<PubGrubTiebreaker>);
type Err = Infallible;
fn prioritize(

View file

@ -1,7 +1,7 @@
pub(crate) use crate::pubgrub::dependencies::PubGrubDependency;
pub(crate) use crate::pubgrub::distribution::PubGrubDistribution;
pub(crate) use crate::pubgrub::package::{PubGrubPackage, PubGrubPackageInner, PubGrubPython};
pub(crate) use crate::pubgrub::priority::{PubGrubPriorities, PubGrubPriority};
pub(crate) use crate::pubgrub::priority::{PubGrubPriorities, PubGrubPriority, PubGrubTiebreaker};
pub(crate) use crate::pubgrub::report::PubGrubReportFormatter;
mod dependencies;

View file

@ -24,7 +24,7 @@ use crate::SentinelRange;
#[derive(Clone, Debug, Default)]
pub(crate) struct PubGrubPriorities {
package_priority: FxHashMap<PackageName, PubGrubPriority>,
virtual_package_tiebreaker: FxHashMap<PubGrubPackage, u32>,
virtual_package_tiebreaker: FxHashMap<PubGrubPackage, PubGrubTiebreaker>,
}
impl PubGrubPriorities {
@ -38,8 +38,10 @@ impl PubGrubPriorities {
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"),
PubGrubTiebreaker::from(
u32::try_from(self.virtual_package_tiebreaker.len())
.expect("Less than 2**32 packages"),
),
);
}
@ -104,26 +106,25 @@ impl PubGrubPriorities {
| PubGrubPriority::ConflictEarly(Reverse(index))
| PubGrubPriority::Singleton(Reverse(index))
| PubGrubPriority::DirectUrl(Reverse(index)) => Some(*index),
PubGrubPriority::Proxy | PubGrubPriority::Root => None,
PubGrubPriority::Root => None,
}
}
/// Return the [`PubGrubPriority`] of the given package, if it exists.
pub(crate) fn get(&self, package: &PubGrubPackage) -> (Option<PubGrubPriority>, u32) {
pub(crate) fn get(
&self,
package: &PubGrubPackage,
) -> (Option<PubGrubPriority>, Option<PubGrubTiebreaker>) {
let package_priority = match &**package {
PubGrubPackageInner::Root(_) => Some(PubGrubPriority::Root),
PubGrubPackageInner::Python(_) => Some(PubGrubPriority::Root),
PubGrubPackageInner::Marker { .. } => Some(PubGrubPriority::Proxy),
PubGrubPackageInner::Extra { .. } => Some(PubGrubPriority::Proxy),
PubGrubPackageInner::Dev { .. } => Some(PubGrubPriority::Proxy),
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)
let package_tiebreaker = self.virtual_package_tiebreaker.get(package).copied();
(package_priority, package_tiebreaker)
}
/// Mark a package as prioritized by setting it to [`PubGrubPriority::ConflictEarly`], if it
@ -225,13 +226,15 @@ pub(crate) enum PubGrubPriority {
/// [`ForkUrls`].
DirectUrl(Reverse<usize>),
/// The package is a proxy package.
///
/// We process proxy packages eagerly since each proxy package expands into two "regular"
/// [`PubGrubPackage`] packages, which gives us additional constraints while not affecting the
/// priorities (since the expanded dependencies are all linked to the same package name).
Proxy,
/// The package is the root package.
Root,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub(crate) struct PubGrubTiebreaker(Reverse<u32>);
impl From<u32> for PubGrubTiebreaker {
fn from(value: u32) -> Self {
Self(Reverse(value))
}
}

View file

@ -2301,16 +2301,16 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
}
// Verify that the package is allowed under the hash-checking policy.
if !self
.hasher
.allows_package(candidate.name(), candidate.version())
{
return Ok(None);
}
// Emit a request to fetch the metadata for this version.
if self.index.distributions().register(candidate.version_id()) {
// Verify that the package is allowed under the hash-checking policy.
if !self
.hasher
.allows_package(candidate.name(), candidate.version())
{
return Err(ResolveError::UnhashedPackage(candidate.name().clone()));
}
let dist = dist.for_resolution().to_owned();
let response = match dist {
@ -2600,9 +2600,7 @@ impl ForkState {
}
}
let for_package = &self.pubgrub.package_store[for_package];
if let Some(name) = for_package
if let Some(name) = self.pubgrub.package_store[for_package]
.name_no_root()
.filter(|name| !workspace_members.contains(name))
{
@ -2633,9 +2631,7 @@ impl ForkState {
}
// Update the package priorities.
if !for_package.is_proxy() {
self.priorities.insert(package, version, &self.fork_urls);
}
self.priorities.insert(package, version, &self.fork_urls);
}
let conflict = self.pubgrub.add_package_version_dependencies(

View file

@ -50,7 +50,7 @@ fn extra_basic() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[extra1] depends on sortedcontainers==2.3.0 and project[extra2] depends on sortedcontainers==2.4.0, we can conclude that project[extra1] and project[extra2] are incompatible.
Because project[extra2] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.3.0, we can conclude that project[extra1] and project[extra2] are incompatible.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);
@ -250,8 +250,8 @@ fn extra_basic_three_extras() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[extra2] depends on sortedcontainers==2.3.0 and project[project3] depends on sortedcontainers==2.4.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.
Because project[extra2] depends on sortedcontainers==2.3.0 and project[extra1] depends on sortedcontainers==2.2.0, we can conclude that project[extra1] and project[extra2] are incompatible.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);
// And now with the same extra configuration, we tell uv about
@ -538,8 +538,8 @@ fn extra_multiple_not_conflicting2() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[project3] depends on sortedcontainers==2.3.0 and project[project4] depends on sortedcontainers==2.4.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.
Because project[extra2] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.3.0, we can conclude that project[extra1] and project[extra2] are incompatible.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);
// If we define extra1/extra2 as conflicting and project3/project4
@ -582,7 +582,7 @@ fn extra_multiple_not_conflicting2() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[extra2] depends on sortedcontainers==2.4.0 and project[project3] depends on sortedcontainers==2.3.0, we can conclude that project[extra2] and project[project3] are incompatible.
Because project[project3] depends on sortedcontainers==2.3.0 and project[extra2] depends on sortedcontainers==2.4.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.
"###);
@ -715,8 +715,8 @@ fn extra_multiple_independent() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[project3] depends on anyio==4.1.0 and project[project4] depends on anyio==4.2.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.
Because project[extra2] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.3.0, we can conclude that project[extra1] and project[extra2] are incompatible.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);
// OK, responding to the error, we declare our anyio extras
@ -755,7 +755,7 @@ fn extra_multiple_independent() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[extra1] depends on sortedcontainers==2.3.0 and project[extra2] depends on sortedcontainers==2.4.0, we can conclude that project[extra1] and project[extra2] are incompatible.
Because project[extra2] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.3.0, we can conclude that project[extra1] and project[extra2] are incompatible.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);
@ -1047,7 +1047,7 @@ fn extra_config_change_ignore_lockfile() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project[extra1] depends on sortedcontainers==2.3.0 and project[extra2] depends on sortedcontainers==2.4.0, we can conclude that project[extra1] and project[extra2] are incompatible.
Because project[extra2] depends on sortedcontainers==2.4.0 and project[extra1] depends on sortedcontainers==2.3.0, we can conclude that project[extra1] and project[extra2] are incompatible.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);
@ -1289,9 +1289,11 @@ fn extra_nested_across_workspace() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because proxy1[extra1]==0.1.0 depends on anyio==4.1.0 and proxy1[extra2]==0.1.0 depends on anyio==4.2.0, we can conclude that proxy1[extra1]==0.1.0 and proxy1[extra2]==0.1.0 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 proxy1[extra2]==0.1.0 are incompatible.
And because only proxy1[extra2]==0.1.0 is available and dummy[extra2] depends on proxy1[extra2], we can conclude that dummy[extra2] and dummysub[extra1] 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. (1)
Because proxy1[extra1]==0.1.0 depends on anyio==4.1.0 and proxy1[extra2]==0.1.0 depends on anyio==4.2.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.
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.
"###);
@ -1415,7 +1417,7 @@ fn group_basic() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because project:group1 depends on sortedcontainers==2.3.0 and project:group2 depends on sortedcontainers==2.4.0, we can conclude that project:group1 and project:group2 are incompatible.
Because project:group2 depends on sortedcontainers==2.4.0 and project:group1 depends on sortedcontainers==2.3.0, we can conclude that project:group1 and project:group2 are incompatible.
And because your project requires project:group1 and project:group2, we can conclude that your project's requirements are unsatisfiable.
"###);
@ -1793,7 +1795,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:group1 and project[extra1] are incompatible.
And because your project requires project[extra1] and project:group1, we can conclude that your project's requirements are unsatisfiable.
"###);
@ -7126,8 +7128,8 @@ fn overlapping_resolution_markers() -> Result<()> {
"sys_platform == 'linux' and extra != 'extra-14-ads-mega-model-cpu' and extra == 'extra-14-ads-mega-model-cu118'",
"sys_platform != 'linux' and extra != 'extra-14-ads-mega-model-cpu' and extra == 'extra-14-ads-mega-model-cu118'",
"platform_machine != 'aarch64' and sys_platform == 'linux' and extra == 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",
"sys_platform != 'darwin' and sys_platform != 'linux' and extra == 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",
"platform_machine == 'aarch64' and sys_platform == 'linux' and extra == 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",
"sys_platform != 'darwin' and sys_platform != 'linux' and extra == 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",
"sys_platform == 'darwin' and extra == 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",
"sys_platform == 'linux' and extra != 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",
"sys_platform != 'linux' and extra != 'extra-14-ads-mega-model-cpu' and extra != 'extra-14-ads-mega-model-cu118'",

View file

@ -2940,7 +2940,7 @@ fn fork_non_local_fork_marker_direct() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
Because package-b==1.0.0 depends on package-c>=2.0.0 and package-a==1.0.0 depends on package-c<2.0.0, we can conclude that package-a==1.0.0 and package-b==1.0.0 are incompatible.
Because package-a==1.0.0 depends on package-c<2.0.0 and package-b==1.0.0 depends on package-c>=2.0.0, we can conclude that package-a{sys_platform == 'linux'}==1.0.0 and package-b{sys_platform == 'darwin'}==1.0.0 are incompatible.
And because your project depends on package-a{sys_platform == 'linux'}==1.0.0 and package-b{sys_platform == 'darwin'}==1.0.0, we can conclude that your project's requirements are unsatisfiable.
"###
);
@ -3015,10 +3015,8 @@ fn fork_non_local_fork_marker_transitive() -> Result<()> {
Because package-a==1.0.0 depends on package-c{sys_platform == 'linux'}<2.0.0 and only the following versions of package-c{sys_platform == 'linux'} are available:
package-c{sys_platform == 'linux'}==1.0.0
package-c{sys_platform == 'linux'}>2.0.0
we can conclude that package-a==1.0.0 depends on package-c{sys_platform == 'linux'}==1.0.0. (1)
Because only package-c{sys_platform == 'darwin'}<=2.0.0 is available and package-b==1.0.0 depends on package-c{sys_platform == 'darwin'}>=2.0.0, we can conclude that package-b==1.0.0 depends on package-c==2.0.0.
And because we know from (1) that package-a==1.0.0 depends on package-c{sys_platform == 'linux'}==1.0.0, we can conclude that package-a==1.0.0 and package-b==1.0.0 are incompatible.
we can conclude that package-a==1.0.0 depends on package-c{sys_platform == 'linux'}==1.0.0.
And because only package-c{sys_platform == 'darwin'}<=2.0.0 is available and package-b==1.0.0 depends on package-c{sys_platform == 'darwin'}>=2.0.0, we can conclude that package-a==1.0.0 and package-b==1.0.0 are incompatible.
And because your project depends on package-a==1.0.0 and package-b==1.0.0, we can conclude that your project's requirements are unsatisfiable.
"###
);

View file

@ -836,8 +836,8 @@ fn extra_incompatible_with_extra() {
----- stderr -----
× No solution found when resolving dependencies:
Because package-a[extra-b]==1.0.0 depends on package-b==1.0.0 and package-a[extra-c]==1.0.0 depends on package-b==2.0.0, we can conclude that package-a[extra-b]==1.0.0 and package-a[extra-c]==1.0.0 are incompatible.
And because only package-a[extra-c]==1.0.0 is available and only package-a[extra-b]==1.0.0 is available, we can conclude that all versions of package-a[extra-b] and all versions of package-a[extra-c] are incompatible.
Because only package-a[extra-b]==1.0.0 is available and package-a[extra-b]==1.0.0 depends on package-b==1.0.0, we can conclude that all versions of package-a[extra-b] depend on package-b==1.0.0.
And because package-a[extra-c]==1.0.0 depends on package-b==2.0.0 and only package-a[extra-c]==1.0.0 is available, we can conclude that all versions of package-a[extra-b] and all versions of package-a[extra-c] are incompatible.
And because you require package-a[extra-b] and package-a[extra-c], we can conclude that your requirements are unsatisfiable.
"###);

View file

@ -7,4 +7,4 @@ exit_code: 0
----- stdout -----
----- stderr -----
Resolved 286 packages in [TIME]
Resolved 302 packages in [TIME]