mirror of
https://github.com/astral-sh/uv.git
synced 2025-07-07 13:25:00 +00:00
uv-resolver: add support for incomplete markers
In some cases, it's possible for the marker expressions on conflicting dependency specification to be disjoint but *incomplete*. That is, if one unions the disjoint markers, the result is not the complete set of marker environments possible. There may be some "gap" of marker environments not covered by the markers. This is a problem in practice because, before this commit, we only created forks in the resolver for specific marker expressions. So if a dependency happened to fall in a "gap," our resolver would never see it. This commit fixes this by adding a new split covering the negation of the union of all marker expressions in a set of forks for a specific package. Originally, I had planned on only creating this split when it was known that the gap actually existed. That is, when the negation of the marker expressions did *not* correspond to the empty set. After a lot of thought, unfortunately, this (I believe) effectively boils down to 3SAT, which is NP-complete. Instead, what we do here is *always* create an extra split unless we can definitively tell that it is empty. We look for a few cases, but otherwise throw our hands up and potentially do wasted work. This also updates the lock scenario tests to reflect the actual bug fix here.
This commit is contained in:
parent
f36f2f41ac
commit
563507edba
4 changed files with 122 additions and 1 deletions
|
@ -2515,6 +2515,12 @@ mod test {
|
||||||
neg("os_name == 'bar' or os_name == 'foo'"),
|
neg("os_name == 'bar' or os_name == 'foo'"),
|
||||||
"os_name != 'bar' and os_name != 'foo'"
|
"os_name != 'bar' and os_name != 'foo'"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Always true negates to always false!
|
||||||
|
assert_eq!(
|
||||||
|
neg("python_version >= '3.6' or python_version < '3.6'"),
|
||||||
|
"python_version < '3.6' and python_version >= '3.6'"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
|
@ -15,6 +15,47 @@ use pep508_rs::{
|
||||||
use crate::pubgrub::PubGrubSpecifier;
|
use crate::pubgrub::PubGrubSpecifier;
|
||||||
use crate::RequiresPythonBound;
|
use crate::RequiresPythonBound;
|
||||||
|
|
||||||
|
/// Returns true when it can be proven that the given marker expression
|
||||||
|
/// evaluates to true for precisely zero marker environments.
|
||||||
|
///
|
||||||
|
/// When this returns false, it *may* be the case that is evaluates to
|
||||||
|
/// true for precisely zero marker environments. That is, this routine
|
||||||
|
/// never has false positives but may have false negatives.
|
||||||
|
pub(crate) fn is_definitively_empty_set(tree: &MarkerTree) -> bool {
|
||||||
|
match *tree {
|
||||||
|
// A conjunction is definitively empty when it is known that
|
||||||
|
// *any* two of its conjuncts are disjoint. Since this would
|
||||||
|
// imply that the entire conjunction could never be true.
|
||||||
|
MarkerTree::And(ref trees) => {
|
||||||
|
// Since this is quadratic in the case where the
|
||||||
|
// expression is *not* empty, we limit ourselves
|
||||||
|
// to a small number of conjuncts. In practice,
|
||||||
|
// this should hopefully cover most cases.
|
||||||
|
if trees.len() > 10 {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
for (i, tree1) in trees.iter().enumerate() {
|
||||||
|
for tree2 in &trees[i..] {
|
||||||
|
if is_disjoint(tree1, tree2) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
// A disjunction is definitively empty when all of its
|
||||||
|
// disjuncts are definitively empty.
|
||||||
|
MarkerTree::Or(ref trees) => trees.iter().all(is_definitively_empty_set),
|
||||||
|
// An "arbitrary" expression is always false, so we
|
||||||
|
// at least know it is definitively empty.
|
||||||
|
MarkerTree::Expression(MarkerExpression::Arbitrary { .. }) => true,
|
||||||
|
// Can't really do much with a single expression. There are maybe
|
||||||
|
// trivial cases we could check (like `python_version < '0'`), but I'm
|
||||||
|
// not sure it's worth doing?
|
||||||
|
MarkerTree::Expression(_) => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns `true` if there is no environment in which both marker trees can both apply, i.e.
|
/// Returns `true` if there is no environment in which both marker trees can both apply, i.e.
|
||||||
/// the expression `first and second` is always false.
|
/// the expression `first and second` is always false.
|
||||||
pub(crate) fn is_disjoint(first: &MarkerTree, second: &MarkerTree) -> bool {
|
pub(crate) fn is_disjoint(first: &MarkerTree, second: &MarkerTree) -> bool {
|
||||||
|
@ -1079,6 +1120,27 @@ mod tests {
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn is_definitively_empty_set() {
|
||||||
|
assert!(is_empty("'wat' == 'wat'"));
|
||||||
|
assert!(is_empty(
|
||||||
|
"python_version < '3.10' and python_version >= '3.10'"
|
||||||
|
));
|
||||||
|
assert!(is_empty(
|
||||||
|
"(python_version < '3.10' and python_version >= '3.10') \
|
||||||
|
or (python_version < '3.9' and python_version >= '3.9')",
|
||||||
|
));
|
||||||
|
|
||||||
|
assert!(!is_empty("python_version < '3.10'"));
|
||||||
|
assert!(!is_empty("python_version < '0'"));
|
||||||
|
assert!(!is_empty(
|
||||||
|
"python_version < '3.10' and python_version >= '3.9'"
|
||||||
|
));
|
||||||
|
assert!(!is_empty(
|
||||||
|
"python_version < '3.10' or python_version >= '3.11'"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
fn test_version_bounds_disjointness(version: &str) {
|
fn test_version_bounds_disjointness(version: &str) {
|
||||||
assert!(!is_disjoint(
|
assert!(!is_disjoint(
|
||||||
format!("{version} > '2.7.0'"),
|
format!("{version} > '2.7.0'"),
|
||||||
|
@ -1125,6 +1187,11 @@ mod tests {
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_empty(tree: &str) -> bool {
|
||||||
|
let tree = MarkerTree::parse_reporter(tree, &mut TracingReporter).unwrap();
|
||||||
|
super::is_definitively_empty_set(&tree)
|
||||||
|
}
|
||||||
|
|
||||||
fn is_disjoint(one: impl AsRef<str>, two: impl AsRef<str>) -> bool {
|
fn is_disjoint(one: impl AsRef<str>, two: impl AsRef<str>) -> bool {
|
||||||
let one = MarkerTree::parse_reporter(one.as_ref(), &mut TracingReporter).unwrap();
|
let one = MarkerTree::parse_reporter(one.as_ref(), &mut TracingReporter).unwrap();
|
||||||
let two = MarkerTree::parse_reporter(two.as_ref(), &mut TracingReporter).unwrap();
|
let two = MarkerTree::parse_reporter(two.as_ref(), &mut TracingReporter).unwrap();
|
||||||
|
|
|
@ -2476,6 +2476,13 @@ impl Dependencies {
|
||||||
};
|
};
|
||||||
assert!(fork_groups.forks.len() >= 2, "expected definitive fork");
|
assert!(fork_groups.forks.len() >= 2, "expected definitive fork");
|
||||||
let mut new_forks: Vec<Fork> = vec![];
|
let mut new_forks: Vec<Fork> = vec![];
|
||||||
|
if let Some(markers) = fork_groups.remaining_universe() {
|
||||||
|
debug!("Adding split to cover possibly incomplete markers: {markers}");
|
||||||
|
new_forks.push(Fork {
|
||||||
|
dependencies: vec![],
|
||||||
|
markers,
|
||||||
|
});
|
||||||
|
}
|
||||||
for group in fork_groups.forks {
|
for group in fork_groups.forks {
|
||||||
let mut new_forks_for_group = forks.clone();
|
let mut new_forks_for_group = forks.clone();
|
||||||
for (index, _) in group.packages {
|
for (index, _) in group.packages {
|
||||||
|
@ -2730,6 +2737,22 @@ impl<'a> PossibleForkGroups<'a> {
|
||||||
.iter_mut()
|
.iter_mut()
|
||||||
.find(|fork| fork.is_overlapping(marker))
|
.find(|fork| fork.is_overlapping(marker))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns a marker tree corresponding to the set of marker expressions
|
||||||
|
/// outside of this fork group.
|
||||||
|
///
|
||||||
|
/// In many cases, it can be easily known that the set of marker
|
||||||
|
/// expressions referred to by this marker tree is empty. In this case,
|
||||||
|
/// `None` is returned. But note that if a marker tree is returned, it is
|
||||||
|
/// still possible for it to describe exactly zero marker environments.
|
||||||
|
fn remaining_universe(&self) -> Option<MarkerTree> {
|
||||||
|
let have = MarkerTree::Or(self.forks.iter().map(PossibleFork::union).collect());
|
||||||
|
let missing = have.negate();
|
||||||
|
if crate::marker::is_definitively_empty_set(&missing) {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
Some(missing)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Intermediate state representing a single possible fork.
|
/// Intermediate state representing a single possible fork.
|
||||||
|
@ -2765,6 +2788,19 @@ impl<'a> PossibleFork<'a> {
|
||||||
}
|
}
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns the union of all the marker expressions in this possible fork.
|
||||||
|
///
|
||||||
|
/// Each marker expression in the union returned is guaranteed to be overlapping
|
||||||
|
/// with at least one other expression in the same union.
|
||||||
|
fn union(&self) -> MarkerTree {
|
||||||
|
MarkerTree::Or(
|
||||||
|
self.packages
|
||||||
|
.iter()
|
||||||
|
.map(|&(_, tree)| (*tree).clone())
|
||||||
|
.collect(),
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns true if and only if the given requirement's marker expression has a
|
/// Returns true if and only if the given requirement's marker expression has a
|
||||||
|
|
|
@ -664,7 +664,7 @@ fn fork_incomplete_markers() -> Result<()> {
|
||||||
|
|
||||||
----- stderr -----
|
----- stderr -----
|
||||||
warning: `uv lock` is experimental and may change without warning.
|
warning: `uv lock` is experimental and may change without warning.
|
||||||
Resolved 4 packages in [TIME]
|
Resolved 5 packages in [TIME]
|
||||||
"###
|
"###
|
||||||
);
|
);
|
||||||
|
|
||||||
|
@ -699,11 +699,23 @@ fn fork_incomplete_markers() -> Result<()> {
|
||||||
name = "package-b"
|
name = "package-b"
|
||||||
version = "1.0.0"
|
version = "1.0.0"
|
||||||
source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }
|
source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }
|
||||||
|
dependencies = [
|
||||||
|
{ name = "package-c", marker = "python_version == '3.10'" },
|
||||||
|
]
|
||||||
sdist = { url = "https://astral-sh.github.io/packse/0.3.30/files/fork_incomplete_markers_b-1.0.0.tar.gz#sha256=c4deba44768923473d077bdc0e177033fcb6e6fd406d56830d7ee6f4ffad68c1", hash = "sha256:c4deba44768923473d077bdc0e177033fcb6e6fd406d56830d7ee6f4ffad68c1" }
|
sdist = { url = "https://astral-sh.github.io/packse/0.3.30/files/fork_incomplete_markers_b-1.0.0.tar.gz#sha256=c4deba44768923473d077bdc0e177033fcb6e6fd406d56830d7ee6f4ffad68c1", hash = "sha256:c4deba44768923473d077bdc0e177033fcb6e6fd406d56830d7ee6f4ffad68c1" }
|
||||||
wheels = [
|
wheels = [
|
||||||
{ url = "https://astral-sh.github.io/packse/0.3.30/files/fork_incomplete_markers_b-1.0.0-py3-none-any.whl#sha256=5c2a5f446580787ed7b3673431b112474237ddeaf1c81325bb30b86e7ee76adb", hash = "sha256:5c2a5f446580787ed7b3673431b112474237ddeaf1c81325bb30b86e7ee76adb" },
|
{ url = "https://astral-sh.github.io/packse/0.3.30/files/fork_incomplete_markers_b-1.0.0-py3-none-any.whl#sha256=5c2a5f446580787ed7b3673431b112474237ddeaf1c81325bb30b86e7ee76adb", hash = "sha256:5c2a5f446580787ed7b3673431b112474237ddeaf1c81325bb30b86e7ee76adb" },
|
||||||
]
|
]
|
||||||
|
|
||||||
|
[[distribution]]
|
||||||
|
name = "package-c"
|
||||||
|
version = "1.0.0"
|
||||||
|
source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }
|
||||||
|
sdist = { url = "https://astral-sh.github.io/packse/0.3.30/files/fork_incomplete_markers_c-1.0.0.tar.gz#sha256=ecc02ea1cc8d3b561c8dcb9d2ba1abcdae2dd32de608bf8e8ed2878118426022", hash = "sha256:ecc02ea1cc8d3b561c8dcb9d2ba1abcdae2dd32de608bf8e8ed2878118426022" }
|
||||||
|
wheels = [
|
||||||
|
{ url = "https://astral-sh.github.io/packse/0.3.30/files/fork_incomplete_markers_c-1.0.0-py3-none-any.whl#sha256=03fa287aa4cb78457211cb3df7459b99ba1ee2259aae24bc745eaab45e7eaaee", hash = "sha256:03fa287aa4cb78457211cb3df7459b99ba1ee2259aae24bc745eaab45e7eaaee" },
|
||||||
|
]
|
||||||
|
|
||||||
[[distribution]]
|
[[distribution]]
|
||||||
name = "project"
|
name = "project"
|
||||||
version = "0.1.0"
|
version = "0.1.0"
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue