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.

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/pubgrub/mod.rs
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;
Expand Down
45 changes: 24 additions & 21 deletions crates/uv-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"),
),
);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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(
Expand Down
34 changes: 18 additions & 16 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[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.
"###);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
"###);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
"###);

Expand Down Expand Up @@ -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.
"###);

Expand Down Expand Up @@ -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.
"###);

Expand Down Expand Up @@ -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.
"###);

Expand Down Expand Up @@ -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.
"###);

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