Avoid narrowing requires-python marker with disjunctions (#10704)

## Summary

A bug in `requires_python` (which infers the Python requirement from a
marker) was leading us to break an invariant around the relationship
between the marker environment and the Python requirement. This, in
turn, was leading us to drop parts of the environment space when
solving.

Specifically, in the linked example, we generated a fork for
`python_full_version < '3.10' or platform_python_implementation !=
'CPython'`, which was later split into `python_full_version == '3.8.*'`
and `python_full_version == '3.9.*'`, losing the
`platform_python_implementation != 'CPython'` portion.

Closes #10669.

Author: charliermarsh

Cargo.lock

@@ -55,6 +55,7 @@ rustc-hash = { workspace = true }
 same-file = { workspace = true }
 schemars = { workspace = true, optional = true }
 serde = { workspace = true }
+smallvec = { workspace = true }
 textwrap = { workspace = true }
 thiserror = { workspace = true }
 tokio = { workspace = true }

crates/uv-resolver/Cargo.toml

@@ -1,63 +1,85 @@
-use pubgrub::Range;
+use pubgrub::Ranges;
+use smallvec::SmallVec;
+use std::ops::Bound;
+
 use uv_pep440::Version;
 use uv_pep508::{CanonicalMarkerValueVersion, MarkerTree, MarkerTreeKind};
 
 use crate::requires_python::{LowerBound, RequiresPythonRange, UpperBound};
 
 /// Returns the bounding Python versions that can satisfy the [`MarkerTree`], if it's constrained.
 pub(crate) fn requires_python(tree: MarkerTree) -> Option<RequiresPythonRange> {
-    fn collect_python_markers(tree: MarkerTree, markers: &mut Vec<Range<Version>>) {
+    /// A small vector of Python version markers.
+    type Markers = SmallVec<[Ranges<Version>; 3]>;
+
+    /// Collect the Python version markers from the tree.
+    ///
+    /// 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 Markers, range: &Ranges<Version>) {
         match tree.kind() {
-            MarkerTreeKind::True | MarkerTreeKind::False => {}
+            MarkerTreeKind::True => {
+                markers.push(range.clone());
+            }
+            MarkerTreeKind::False => {}
             MarkerTreeKind::Version(marker) => match marker.key() {
                 CanonicalMarkerValueVersion::PythonFullVersion => {
                     for (range, tree) in marker.edges() {
-                        if !tree.is_false() {
-                            markers.push(range.clone());
-                        }
+                        collect_python_markers(tree, markers, range);
                     }
                 }
                 CanonicalMarkerValueVersion::ImplementationVersion => {
                     for (_, tree) in marker.edges() {
-                        collect_python_markers(tree, markers);
+                        collect_python_markers(tree, markers, range);
                     }
                 }
             },
             MarkerTreeKind::String(marker) => {
                 for (_, tree) in marker.children() {
-                    collect_python_markers(tree, markers);
+                    collect_python_markers(tree, markers, range);
                 }
             }
             MarkerTreeKind::In(marker) => {
                 for (_, tree) in marker.children() {
-                    collect_python_markers(tree, markers);
+                    collect_python_markers(tree, markers, range);
                 }
             }
             MarkerTreeKind::Contains(marker) => {
                 for (_, tree) in marker.children() {
-                    collect_python_markers(tree, markers);
+                    collect_python_markers(tree, markers, range);
                 }
             }
             MarkerTreeKind::Extra(marker) => {
                 for (_, tree) in marker.children() {
-                    collect_python_markers(tree, markers);
+                    collect_python_markers(tree, markers, range);
                 }
             }
         }
     }
 
-    let mut markers = Vec::new();
-    collect_python_markers(tree, &mut markers);
+    if tree.is_true() || tree.is_false() {
+        return None;
+    }
+
+    let mut markers = Markers::new();
+    collect_python_markers(tree, &mut markers, &Ranges::full());
 
-    // Take the union of all Python version markers.
+    // If there are no Python version markers, return `None`.
+    if markers.iter().all(|range| {
+        let Some((lower, upper)) = range.bounding_range() else {
+            return true;
+        };
+        matches!((lower, upper), (Bound::Unbounded, Bound::Unbounded))
+    }) {
+        return None;
+    }
+
+    // Take the union of the intersections of the Python version markers.
     let range = markers
         .into_iter()
-        .fold(None, |acc: Option<Range<Version>>, range| {
-            Some(match acc {
-                Some(acc) => acc.union(&range),
-                None => range.clone(),
-            })
-        })?;
+        .fold(Ranges::empty(), |acc: Ranges<Version>, range| {
+            acc.union(&range)
+        });
 
     let (lower, upper) = range.bounding_range()?;
 
@@ -66,3 +88,97 @@ pub(crate) fn requires_python(tree: MarkerTree) -> Option<RequiresPythonRange> {
         UpperBound::new(upper.cloned()),
     ))
 }
+
+#[cfg(test)]
+mod tests {
+    use std::ops::Bound;
+    use std::str::FromStr;
+
+    use super::*;
+
+    #[test]
+    fn test_requires_python() {
+        // 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()))
+        );
+
+        // 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()))
+        );
+
+        // 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()))
+        );
+
+        // 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.upper(), UpperBound::new(Bound::Unbounded));
+
+        // 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()))
+        );
+
+        // 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));
+
+        // 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()))
+        );
+
+        // An unbounded range across two specifiers.
+        let tree =
+            MarkerTree::from_str("python_full_version > '3.8' or python_full_version <= '3.8'")
+                .unwrap();
+        assert_eq!(requires_python(tree), None);
+    }
+}

crates/uv-resolver/src/marker.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.
 ///