Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect visitation order for proxy packages #10833

Merged
merged 3 commits into from
Jan 22, 2025
+713 −178
Merged

Respect visitation order for proxy packages #10833

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd5db34
Revert "Visit proxy packages eagerly (#10441)"
charliermarsh Jan 21, 2025
d389f03
Add logic
charliermarsh Jan 21, 2025
d56b638
Update fixtures
charliermarsh Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/uv-resolver/src/dependency_provider.rs
View file
Original file line number Diff line number Diff line change
@@ -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>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #10853

type Err = Infallible;

fn prioritize(
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/pubgrub/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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;
45 changes: 24 additions & 21 deletions crates/uv-resolver/src/pubgrub/priority.rs
View file
Original file line number Diff line number Diff line change
@@ -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))
}
}
24 changes: 10 additions & 14 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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()));
}

Comment on lines +2304 to -2313
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are the hash changes coming from?

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(
34 changes: 18 additions & 16 deletions crates/uv/tests/it/lock_conflict.rs
View file
Original file line number Diff line number Diff line change
@@ -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'",
8 changes: 3 additions & 5 deletions crates/uv/tests/it/lock_scenarios.rs
View file
Original file line number Diff line number Diff line change
@@ -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.
"###
);
Loading