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

Avoid preferring constrained over unconstrained packages #3148

Merged
merged 1 commit into from
Apr 19, 2024
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
22 changes: 8 additions & 14 deletions crates/uv-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,17 @@ impl PubGrubPriorities {
std::collections::hash_map::Entry::Occupied(mut entry) => {
// Preserve the original index.
let index = match entry.get() {
PubGrubPriority::Unspecified(Reverse(index)) => *index,
PubGrubPriority::Singleton(Reverse(index)) => *index,
PubGrubPriority::Unconstrained(Reverse(index)) => *index,
PubGrubPriority::Constrained(Reverse(index)) => *index,
PubGrubPriority::DirectUrl(Reverse(index)) => *index,
PubGrubPriority::Root => next,
};

// Compute the priority.
let priority = if version.as_singleton().is_some() {
PubGrubPriority::Singleton(Reverse(index))
} else if version == &Range::full() {
PubGrubPriority::Unconstrained(Reverse(index))
} else {
PubGrubPriority::Constrained(Reverse(index))
PubGrubPriority::Unspecified(Reverse(index))
};

// Take the maximum of the new and existing priorities.
Expand All @@ -58,10 +55,8 @@ impl PubGrubPriorities {
// Insert the priority.
entry.insert(if version.as_singleton().is_some() {
PubGrubPriority::Singleton(Reverse(next))
} else if version == &Range::full() {
PubGrubPriority::Unconstrained(Reverse(next))
} else {
PubGrubPriority::Constrained(Reverse(next))
PubGrubPriority::Unspecified(Reverse(next))
});
}
}
Expand All @@ -71,9 +66,8 @@ impl PubGrubPriorities {
std::collections::hash_map::Entry::Occupied(mut entry) => {
// Preserve the original index.
let index = match entry.get() {
PubGrubPriority::Unspecified(Reverse(index)) => *index,
PubGrubPriority::Singleton(Reverse(index)) => *index,
PubGrubPriority::Unconstrained(Reverse(index)) => *index,
PubGrubPriority::Constrained(Reverse(index)) => *index,
PubGrubPriority::DirectUrl(Reverse(index)) => *index,
PubGrubPriority::Root => next,
};
Expand Down Expand Up @@ -112,10 +106,10 @@ pub(crate) enum PubGrubPriority {
///
/// As such, its priority is based on the order in which the packages were added (FIFO), such
/// that the first package we visit is prioritized over subsequent packages.
Unconstrained(Reverse<usize>),

/// The version range is constrained in some way (e.g., with a `<=` or `>` operator).
Constrained(Reverse<usize>),
///
/// TODO(charlie): Prefer constrained over unconstrained packages, if they're at the same depth
/// in the dependency graph.
Unspecified(Reverse<usize>),

/// The version range is constrained to a single version (e.g., with the `==` operator).
Singleton(Reverse<usize>),
Expand Down
42 changes: 10 additions & 32 deletions crates/uv/tests/pip_install_scenarios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,14 +460,7 @@ fn dependency_excludes_range_of_compatible_versions() {
╰─▶ Because package-a==1.0.0 depends on package-b==1.0.0 and only the following versions of package-a are available:
package-a==1.0.0
package-a>=2.0.0,<=3.0.0
we can conclude that package-a<2.0.0 depends on package-b==1.0.0.
And because package-a==3.0.0 depends on package-b==3.0.0, we can conclude that any of:
package-a<2.0.0
package-a>=3.0.0
depends on one of:
package-b<=1.0.0
package-b>=3.0.0
(1)
we can conclude that package-a<2.0.0 depends on package-b==1.0.0. (1)

Because only the following versions of package-c are available:
package-c==1.0.0
Expand All @@ -477,13 +470,8 @@ fn dependency_excludes_range_of_compatible_versions() {
package-a<2.0.0
package-a>=3.0.0

And because we know from (1) that any of:
package-a<2.0.0
package-a>=3.0.0
depends on one of:
package-b<=1.0.0
package-b>=3.0.0
we can conclude that all versions of package-c depend on one of:
And because we know from (1) that package-a<2.0.0 depends on package-b==1.0.0, we can conclude that package-a!=3.0.0, all versions of package-c, package-b!=1.0.0 are incompatible.
And because package-a==3.0.0 depends on package-b==3.0.0, we can conclude that all versions of package-c depend on one of:
package-b<=1.0.0
package-b>=3.0.0

Expand Down Expand Up @@ -597,14 +585,7 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() {
╰─▶ Because package-a==1.0.0 depends on package-b==1.0.0 and only the following versions of package-a are available:
package-a==1.0.0
package-a>=2.0.0,<=3.0.0
we can conclude that package-a<2.0.0 depends on package-b==1.0.0.
And because package-a==3.0.0 depends on package-b==3.0.0, we can conclude that any of:
package-a<2.0.0
package-a>=3.0.0
depends on one of:
package-b<=1.0.0
package-b>=3.0.0
(1)
we can conclude that package-a<2.0.0 depends on package-b==1.0.0. (1)

Because only the following versions of package-c are available:
package-c==1.0.0
Expand All @@ -614,13 +595,8 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() {
package-a<2.0.0
package-a>=3.0.0

And because we know from (1) that any of:
package-a<2.0.0
package-a>=3.0.0
depends on one of:
package-b<=1.0.0
package-b>=3.0.0
we can conclude that all versions of package-c depend on one of:
And because we know from (1) that package-a<2.0.0 depends on package-b==1.0.0, we can conclude that all versions of package-c, package-a!=3.0.0, package-b!=1.0.0 are incompatible.
And because package-a==3.0.0 depends on package-b==3.0.0, we can conclude that all versions of package-c depend on one of:
package-b<=1.0.0
package-b>=3.0.0

Expand Down Expand Up @@ -3355,8 +3331,10 @@ fn transitive_prerelease_and_stable_dependency_many_versions() {

----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because only package-c<2.0.0b1 is available and package-a==1.0.0 depends on package-c>=2.0.0b1, we can conclude that package-a==1.0.0 cannot be used.
And because only package-a==1.0.0 is available and you require package-a, we can conclude that the requirements are unsatisfiable.
╰─▶ Because only package-a==1.0.0 is available and package-a==1.0.0 depends on package-c>=2.0.0b1, we can conclude that all versions of package-a depend on package-c>=2.0.0b1.
And because only package-c<2.0.0b1 is available, we can conclude that all versions of package-a depend on package-c>3.0.0.
And because package-b==1.0.0 depends on package-c and only package-b==1.0.0 is available, we can conclude that all versions of package-b and all versions of package-a are incompatible.
And because you require package-a and you require package-b, we can conclude that the requirements are unsatisfiable.

hint: package-c was requested with a pre-release marker (e.g., package-c>=2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`)
"###);
Expand Down
Loading