From 08878b01b6a7d44d5d56f3f0cb24933ff3681f35 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 16 Jan 2025 23:28:41 -0500 Subject: [PATCH] reverts --- crates/uv-resolver/src/marker.rs | 110 +++++++++++++----- crates/uv-resolver/src/resolution/output.rs | 4 +- .../uv-resolver/src/resolver/environment.rs | 30 +---- crates/uv-resolver/src/resolver/mod.rs | 18 +-- crates/uv/tests/it/pip_compile.rs | 34 ++++++ req.in | 2 - 6 files changed, 116 insertions(+), 82 deletions(-) delete mode 100644 req.in diff --git a/crates/uv-resolver/src/marker.rs b/crates/uv-resolver/src/marker.rs index f07b8ebf31f3..376bd9c6714b 100644 --- a/crates/uv-resolver/src/marker.rs +++ b/crates/uv-resolver/src/marker.rs @@ -1,4 +1,5 @@ -use pubgrub::{Range, Ranges}; +use pubgrub::Ranges; + use uv_pep440::Version; use uv_pep508::{CanonicalMarkerValueVersion, MarkerTree, MarkerTreeKind}; @@ -10,7 +11,11 @@ pub(crate) fn requires_python(tree: MarkerTree) -> Option { /// /// Specifically, performs a DFS to collect all Python requirements on the path to every /// `MarkerTreeKind::True` node. - fn collect_python_markers(tree: MarkerTree, markers: &mut Vec>>, current_path: &mut Vec>) { + fn collect_python_markers( + tree: MarkerTree, + markers: &mut Vec>>, + current_path: &mut Vec>, + ) { match tree.kind() { MarkerTreeKind::True => { markers.push(current_path.clone()); @@ -56,15 +61,22 @@ pub(crate) fn requires_python(tree: MarkerTree) -> Option { let mut markers = Vec::new(); collect_python_markers(tree, &mut markers, &mut Vec::new()); + // If there are no Python version markers, return `None`. + if markers.iter().all(Vec::is_empty) { + return None; + } + // Take the union of the intersections of the Python version markers. let range = markers .into_iter() .map(|ranges| { - ranges.into_iter().fold(Ranges::full(), |acc: Range, range| { - acc.intersection(&range) - }) + ranges + .into_iter() + .fold(Ranges::full(), |acc: Ranges, range| { + acc.intersection(&range) + }) }) - .fold(Ranges::empty(), |acc: Range, range| { + .fold(Ranges::empty(), |acc: Ranges, range| { acc.union(&range) }); @@ -76,56 +88,90 @@ pub(crate) fn requires_python(tree: MarkerTree) -> Option { )) } - - #[cfg(test)] mod tests { use std::ops::Bound; use std::str::FromStr; + use super::*; #[test] fn test_requires_python() { - // Test exact version match + // An exact version match. let tree = MarkerTree::from_str("python_full_version == '3.8.*'").unwrap(); let range = requires_python(tree).unwrap(); - assert_eq!(*range.lower(), LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap()))); - assert_eq!(*range.upper(), UpperBound::new(Bound::Excluded(Version::from_str("3.9").unwrap()))); - - // Test version range with exclusive bounds - let tree = MarkerTree::from_str("python_full_version > '3.8' and python_full_version < '3.9'").unwrap(); + assert_eq!( + *range.lower(), + LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap())) + ); + assert_eq!( + *range.upper(), + UpperBound::new(Bound::Excluded(Version::from_str("3.9").unwrap())) + ); + + // A version range with exclusive bounds. + let tree = + MarkerTree::from_str("python_full_version > '3.8' and python_full_version < '3.9'") + .unwrap(); let range = requires_python(tree).unwrap(); - assert_eq!(*range.lower(), LowerBound::new(Bound::Excluded(Version::from_str("3.8").unwrap()))); - assert_eq!(*range.upper(), UpperBound::new(Bound::Excluded(Version::from_str("3.9").unwrap()))); - - // Test version range with inclusive bounds - let tree = MarkerTree::from_str("python_full_version >= '3.8' and python_full_version <= '3.9'").unwrap(); + assert_eq!( + *range.lower(), + LowerBound::new(Bound::Excluded(Version::from_str("3.8").unwrap())) + ); + assert_eq!( + *range.upper(), + UpperBound::new(Bound::Excluded(Version::from_str("3.9").unwrap())) + ); + + // A version range with inclusive bounds. + let tree = + MarkerTree::from_str("python_full_version >= '3.8' and python_full_version <= '3.9'") + .unwrap(); let range = requires_python(tree).unwrap(); - assert_eq!(*range.lower(), LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap()))); - assert_eq!(*range.upper(), UpperBound::new(Bound::Included(Version::from_str("3.9").unwrap()))); - - // Test version with only lower bound + assert_eq!( + *range.lower(), + LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap())) + ); + assert_eq!( + *range.upper(), + UpperBound::new(Bound::Included(Version::from_str("3.9").unwrap())) + ); + + // A version with a lower bound. let tree = MarkerTree::from_str("python_full_version >= '3.8'").unwrap(); let range = requires_python(tree).unwrap(); - assert_eq!(*range.lower(), LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap()))); + assert_eq!( + *range.lower(), + LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap())) + ); assert_eq!(*range.upper(), UpperBound::new(Bound::Unbounded)); - // Test version with only upper bound + // A version with an upper bound. let tree = MarkerTree::from_str("python_full_version < '3.9'").unwrap(); let range = requires_python(tree).unwrap(); assert_eq!(*range.lower(), LowerBound::new(Bound::Unbounded)); - assert_eq!(*range.upper(), UpperBound::new(Bound::Excluded(Version::from_str("3.9").unwrap()))); - - // Test OR with non-Python marker (should result in unbounded range) - let tree = MarkerTree::from_str("python_full_version > '3.8' or sys_platform == 'win32'").unwrap(); + assert_eq!( + *range.upper(), + UpperBound::new(Bound::Excluded(Version::from_str("3.9").unwrap())) + ); + + // A disjunction with a non-Python marker (i.e., an unbounded range). + let tree = + MarkerTree::from_str("python_full_version > '3.8' or sys_platform == 'win32'").unwrap(); let range = requires_python(tree).unwrap(); assert_eq!(*range.lower(), LowerBound::new(Bound::Unbounded)); assert_eq!(*range.upper(), UpperBound::new(Bound::Unbounded)); - // Test complex AND/OR combination + // A complex mix of conjunctions and disjunctions. let tree = MarkerTree::from_str("(python_full_version >= '3.8' and python_full_version < '3.9') or (python_full_version >= '3.10' and python_full_version < '3.11')").unwrap(); let range = requires_python(tree).unwrap(); - assert_eq!(*range.lower(), LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap()))); - assert_eq!(*range.upper(), UpperBound::new(Bound::Excluded(Version::from_str("3.11").unwrap()))); + assert_eq!( + *range.lower(), + LowerBound::new(Bound::Included(Version::from_str("3.8").unwrap())) + ); + assert_eq!( + *range.upper(), + UpperBound::new(Bound::Excluded(Version::from_str("3.11").unwrap())) + ); } } diff --git a/crates/uv-resolver/src/resolution/output.rs b/crates/uv-resolver/src/resolution/output.rs index 5014a08790b2..44dc9e887eb0 100644 --- a/crates/uv-resolver/src/resolution/output.rs +++ b/crates/uv-resolver/src/resolution/output.rs @@ -7,7 +7,7 @@ use petgraph::{ Directed, Direction, }; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; -use tracing::debug; + use uv_configuration::{Constraints, Overrides}; use uv_distribution::Metadata; use uv_distribution_types::{ @@ -165,8 +165,6 @@ impl ResolverOutput { for resolution in resolutions { let marker = resolution.env.try_universal_markers().unwrap_or_default(); - debug!("Got resolution with: {:?}", marker); - // Add every edge to the graph, propagating the marker for the current fork, if // necessary. for edge in &resolution.edges { diff --git a/crates/uv-resolver/src/resolver/environment.rs b/crates/uv-resolver/src/resolver/environment.rs index 284f721c5eef..5d2914985f10 100644 --- a/crates/uv-resolver/src/resolver/environment.rs +++ b/crates/uv-resolver/src/resolver/environment.rs @@ -1,5 +1,5 @@ use std::sync::Arc; -use tracing::{debug, trace}; +use tracing::trace; use uv_pep440::VersionSpecifiers; use uv_pep508::{MarkerEnvironment, MarkerTree}; use uv_pypi_types::{ConflictItem, ConflictItemRef, ResolverMarkerEnvironment}; @@ -544,41 +544,13 @@ pub(crate) fn fork_version_by_python_requirement( panic!("resolver must be in universal mode for forking") }; - debug!("Using env marker: {:?}", env_marker); - debug!("Using requires-python: {:?}", python_requirement.target()); - let mut envs = vec![]; if !env_marker.is_disjoint(lower.to_marker_tree()) { - debug!("Narrowing to: {:?}", lower.to_marker_tree()); - debug!("Narrowed env: {:?}", env.narrow_environment(lower.to_marker_tree())); envs.push(env.narrow_environment(lower.to_marker_tree())); } if !env_marker.is_disjoint(upper.to_marker_tree()) { - debug!("Narrowing to: {:?}", upper.to_marker_tree()); - debug!("Narrowed env: {:?}", env.narrow_environment(upper.to_marker_tree())); envs.push(env.narrow_environment(upper.to_marker_tree())); } - - // The big issue here is: if `python_requirement` is more narrow than the environment marker, - // the split will _not_ cover the entire environment. So, for example, with the current bug, - // if marker is `python_version >= '3.10' or sys_platform == 'linux'`, and the Python - // requirement is `>=3.11`, then we lose the portion of the environment that is `sys_platform - // == 'linux'`. - // - // So it's only an issue if we have a broken invariant, I think. - - // // Compute the remainder. - // let mut remainder = env_marker.clone(); - // remainder.and(lower.to_marker_tree().negate()); - // remainder.and(upper.to_marker_tree().negate()); - // // remainder.and(python_requirement.to_marker_tree()); - // if !remainder.is_false() { - // println!("Remainder: {:?}", remainder); - // println!("python_requirement: {:?}", python_requirement); - // envs.push(env.narrow_environment(remainder)); - // // return vec![]; - // } - debug_assert!(!envs.is_empty(), "at least one fork should be produced"); envs } diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 2864a93902b9..6fe985ea8f00 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -542,13 +542,7 @@ impl ResolverState version, ResolverVersion::Forked(forks) => { - debug!("Currently solving: {}", state.env); - - for y in self.version_forks_to_fork_states(state, forks) { - debug!("Forking to solve: {}", y.env); - forked_states.push(y); - } - // forked_states.extend(self.version_forks_to_fork_states(state, forks)); + forked_states.extend(self.version_forks_to_fork_states(state, forks)); continue 'FORK; } ResolverVersion::Unavailable(version, reason) => { @@ -1240,12 +1234,6 @@ impl ResolverState=3.9`. - // - // So we want to solve separately for `3.8` and `3.9`. let forks = fork_version_by_python_requirement( requires_python, python_requirement, @@ -2772,12 +2760,10 @@ impl ForkState { /// If the fork should be dropped (e.g., because its markers can never be true for its /// Python requirement), then this returns `None`. fn with_env(mut self, env: ResolverEnvironment) -> Self { - debug!("Creating env: {env}"); - debug!("Current python requirement: {:?}", self.python_requirement); self.env = env; // If the fork contains a narrowed Python requirement, apply it. if let Some(req) = self.env.narrow_python_requirement(&self.python_requirement) { - debug!("Narrowed `requires-python` bound to: {:?}", req.target()); + debug!("Narrowed `requires-python` bound to: {}", req.target()); self.python_requirement = req; } self diff --git a/crates/uv/tests/it/pip_compile.rs b/crates/uv/tests/it/pip_compile.rs index fe459358ead5..b4deda2566f7 100644 --- a/crates/uv/tests/it/pip_compile.rs +++ b/crates/uv/tests/it/pip_compile.rs @@ -14143,6 +14143,40 @@ fn compile_lowest_extra_unpinned_warning() -> Result<()> { Ok(()) } +#[test] +fn disjoint_requires_python() -> Result<()> { + let context = TestContext::new("3.8"); + + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r" + iniconfig ; platform_python_implementation == 'CPython' and python_version >= '3.10' + coverage + "})?; + + uv_snapshot!(context.filters(), context.pip_compile() + .arg("--universal") + .arg(requirements_in.path()) + .env_remove(EnvVars::UV_EXCLUDE_NEWER), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile --cache-dir [CACHE_DIR] --universal [TEMP_DIR]/requirements.in + coverage==7.6.1 ; python_full_version < '3.9' + # via -r requirements.in + coverage==7.6.10 ; python_full_version >= '3.9' + # via -r requirements.in + iniconfig==2.0.0 ; python_full_version >= '3.10' and platform_python_implementation == 'CPython' + # via -r requirements.in + + ----- stderr ----- + Resolved 3 packages in [TIME] + "### + ); + + Ok(()) +} + /// Test that we use the version in the source distribution filename for compiling, even if the /// version is declared as dynamic. /// diff --git a/req.in b/req.in deleted file mode 100644 index 4c435f657b2d..000000000000 --- a/req.in +++ /dev/null @@ -1,2 +0,0 @@ -iniconfig ; platform_python_implementation == "CPython" and python_version >= "3.10" -coverage