Skip to content

Commit

Permalink
Visit proxy packages eagerly (#10441)
Browse files Browse the repository at this point in the history
## Summary

The issue here is that we add `urllib3{python_full_version >= '3.8'}` as
a dependency, then `requests{python_full_version >= '3.8'}`, which adds
`urllib3`, but at that point, we haven't expanded
`urllib3{python_full_version >= '3.8'}`, so we "lose" the singleton
constraint. The solution is to ensure that we visit proxies eagerly, so
that we accumulate constraints as early as possible.

Closes
#10425 (comment).
  • Loading branch information
charliermarsh authored Jan 9, 2025
1 parent 57367ed commit 56d39d2
Show file tree
Hide file tree
Showing 10 changed files with 219 additions and 146 deletions.
24 changes: 17 additions & 7 deletions crates/uv-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use pubgrub::Range;
use rustc_hash::FxHashMap;
use std::cmp::Reverse;
use std::collections::hash_map::OccupiedEntry;

use crate::fork_urls::ForkUrls;
use pubgrub::Range;
use rustc_hash::FxHashMap;

use uv_normalize::PackageName;
use uv_pep440::Version;

use crate::fork_urls::ForkUrls;
use crate::pubgrub::package::PubGrubPackage;
use crate::pubgrub::PubGrubPackageInner;

Expand Down Expand Up @@ -57,6 +58,7 @@ impl PubGrubPriorities {
let priority = if urls.get(name).is_some() {
PubGrubPriority::DirectUrl(Reverse(index))
} else if version.as_singleton().is_some() {
// TODO(charlie): Take local version ranges into account (e.g., `[2.0, 2.0+[max])`).
PubGrubPriority::Singleton(Reverse(index))
} else {
// Keep the conflict-causing packages to avoid loops where we seesaw between
Expand All @@ -80,6 +82,7 @@ impl PubGrubPriorities {
let priority = if urls.get(name).is_some() {
PubGrubPriority::DirectUrl(Reverse(next))
} else if version.as_singleton().is_some() {
// TODO(charlie): Take local version ranges into account (e.g., `[2.0, 2.0+[max])`).
PubGrubPriority::Singleton(Reverse(next))
} else {
PubGrubPriority::Unspecified(Reverse(next))
Expand All @@ -98,7 +101,7 @@ impl PubGrubPriorities {
| PubGrubPriority::ConflictEarly(Reverse(index))
| PubGrubPriority::Singleton(Reverse(index))
| PubGrubPriority::DirectUrl(Reverse(index)) => Some(*index),
PubGrubPriority::Root => None,
PubGrubPriority::Proxy | PubGrubPriority::Root => None,
}
}

Expand All @@ -107,9 +110,9 @@ impl PubGrubPriorities {
let package_priority = match &**package {
PubGrubPackageInner::Root(_) => Some(PubGrubPriority::Root),
PubGrubPackageInner::Python(_) => Some(PubGrubPriority::Root),
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::Marker { .. } => Some(PubGrubPriority::Proxy),
PubGrubPackageInner::Extra { .. } => Some(PubGrubPriority::Proxy),
PubGrubPackageInner::Dev { .. } => Some(PubGrubPriority::Proxy),
PubGrubPackageInner::Package { name, .. } => self.package_priority.get(name).copied(),
};
let virtual_package_tiebreaker = self
Expand Down Expand Up @@ -224,6 +227,13 @@ 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,
}
8 changes: 6 additions & 2 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2591,7 +2591,9 @@ impl ForkState {
}
}

if let Some(name) = self.pubgrub.package_store[for_package]
let for_package = &self.pubgrub.package_store[for_package];

if let Some(name) = for_package
.name_no_root()
.filter(|name| !workspace_members.contains(name))
{
Expand Down Expand Up @@ -2622,7 +2624,9 @@ impl ForkState {
}

// Update the package priorities.
self.priorities.insert(package, version, &self.fork_urls);
if !for_package.is_proxy() {
self.priorities.insert(package, version, &self.fork_urls);
}
}

let conflict = self.pubgrub.add_package_version_dependencies(
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-resolver/src/version_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ impl VersionMap {
range: &Ranges<Version>,
) -> impl DoubleEndedIterator<Item = (&Version, VersionMapDistHandle)> {
// Performance optimization: If we only have a single version, return that version directly.
//
// TODO(charlie): Now that we use local version sentinels, does this ever trigger?
if let Some(version) = range.as_singleton() {
either::Either::Left(match self.inner {
VersionMapInner::Eager(ref eager) => {
Expand Down
24 changes: 12 additions & 12 deletions crates/uv/tests/it/lock_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn extra_basic() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down Expand Up @@ -250,7 +250,7 @@ fn extra_basic_three_extras() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
"###);

Expand Down Expand Up @@ -538,7 +538,7 @@ fn extra_multiple_not_conflicting2() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
"###);

Expand Down Expand Up @@ -582,7 +582,7 @@ fn extra_multiple_not_conflicting2() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
And because your project requires project[extra2] and project[project3], we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down Expand Up @@ -715,7 +715,7 @@ fn extra_multiple_independent() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because project[project4] depends on anyio==4.2.0 and project[project3] depends on anyio==4.1.0, we can conclude that project[project3] and project[project4] are incompatible.
╰─▶ 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.
"###);

Expand Down Expand Up @@ -755,7 +755,7 @@ fn extra_multiple_independent() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down Expand Up @@ -1047,7 +1047,7 @@ fn extra_config_change_ignore_lockfile() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
And because your project requires project[extra1] and project[extra2], we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down Expand Up @@ -1289,9 +1289,9 @@ 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.
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.
╰─▶ 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.
And because your workspace requires dummy[extra2] and dummysub[extra1], we can conclude that your workspace's requirements are unsatisfiable.
"###);

Expand Down Expand Up @@ -1415,7 +1415,7 @@ fn group_basic() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
And because your project requires project:group1 and project:group2, we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down Expand Up @@ -1793,7 +1793,7 @@ fn mixed() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ 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.
╰─▶ 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.
And because your project requires project[extra1] and project:group1, we can conclude that your project's requirements are unsatisfiable.
"###);

Expand Down
10 changes: 6 additions & 4 deletions crates/uv/tests/it/lock_scenarios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ fn conflict_in_fork() -> Result<()> {
× No solution found when resolving dependencies for split (sys_platform == 'darwin'):
╰─▶ Because only package-b==1.0.0 is available and package-b==1.0.0 depends on package-d==1, we can conclude that all versions of package-b depend on package-d==1.
And because package-c==1.0.0 depends on package-d==2 and only package-c==1.0.0 is available, we can conclude that all versions of package-b and all versions of package-c are incompatible.
And because package-a{sys_platform == 'darwin'}==1.0.0 depends on package-b and package-c, we can conclude that package-a{sys_platform == 'darwin'}==1.0.0 cannot be used.
And because package-a==1.0.0 depends on package-b and package-c, we can conclude that package-a==1.0.0 cannot be used.
And because only the following versions of package-a{sys_platform == 'darwin'} are available:
package-a{sys_platform == 'darwin'}==1.0.0
package-a{sys_platform == 'darwin'}>2
Expand Down Expand Up @@ -2940,7 +2940,7 @@ fn fork_non_local_fork_marker_direct() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because package-a{sys_platform == 'linux'}==1.0.0 depends on package-c<2.0.0 and package-b{sys_platform == 'darwin'}==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.
╰─▶ 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.
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.
"###
);
Expand Down Expand Up @@ -3015,8 +3015,10 @@ 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.
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.
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.
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.
"###
);
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/tests/it/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13622,9 +13622,9 @@ fn unsupported_requires_python_dynamic_metadata() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies for split (python_full_version >= '3.10'):
╰─▶ Because source-distribution{python_full_version >= '3.10'}==0.0.3 requires Python >=3.10 and you require source-distribution{python_full_version >= '3.10'}==0.0.3, we can conclude that your requirements are unsatisfiable.
╰─▶ Because source-distribution==0.0.3 requires Python >=3.10 and you require source-distribution{python_full_version >= '3.10'}==0.0.3, we can conclude that your requirements are unsatisfiable.
hint: The source distribution for `source-distribution{python_full_version >= '3.10'}` (v0.0.3) does not include static metadata. Generating metadata for this package requires Python >=3.10, but Python 3.8.[X] is installed.
hint: The source distribution for `source-distribution` (v0.0.3) does not include static metadata. Generating metadata for this package requires Python >=3.10, but Python 3.8.[X] is installed.
"###);

Ok(())
Expand Down
Loading

0 comments on commit 56d39d2

Please sign in to comment.