diff --git a/crates/pep508-rs/src/marker.rs b/crates/pep508-rs/src/marker.rs index 809fb758b..9115b3058 100644 --- a/crates/pep508-rs/src/marker.rs +++ b/crates/pep508-rs/src/marker.rs @@ -2515,6 +2515,12 @@ mod test { neg("os_name == 'bar' or 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] diff --git a/crates/uv-resolver/src/marker.rs b/crates/uv-resolver/src/marker.rs index c36547cfd..d2e738e88 100644 --- a/crates/uv-resolver/src/marker.rs +++ b/crates/uv-resolver/src/marker.rs @@ -15,6 +15,47 @@ use pep508_rs::{ use crate::pubgrub::PubGrubSpecifier; 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. /// the expression `first and second` is always false. 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) { assert!(!is_disjoint( 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, two: impl AsRef) -> bool { let one = MarkerTree::parse_reporter(one.as_ref(), &mut TracingReporter).unwrap(); let two = MarkerTree::parse_reporter(two.as_ref(), &mut TracingReporter).unwrap(); diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 43fe644d3..89640845b 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -2476,6 +2476,13 @@ impl Dependencies { }; assert!(fork_groups.forks.len() >= 2, "expected definitive fork"); let mut new_forks: Vec = 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 { let mut new_forks_for_group = forks.clone(); for (index, _) in group.packages { @@ -2730,6 +2737,22 @@ impl<'a> PossibleForkGroups<'a> { .iter_mut() .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 { + 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. @@ -2765,6 +2788,19 @@ impl<'a> PossibleFork<'a> { } 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 diff --git a/crates/uv/tests/lock_scenarios.rs b/crates/uv/tests/lock_scenarios.rs index 070df62eb..5840b1a0d 100644 --- a/crates/uv/tests/lock_scenarios.rs +++ b/crates/uv/tests/lock_scenarios.rs @@ -664,7 +664,7 @@ fn fork_incomplete_markers() -> Result<()> { ----- stderr ----- 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" version = "1.0.0" 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" } 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" }, ] + [[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]] name = "project" version = "0.1.0"