Avoid narrowing requires-python marker with disjunctions

crates/uv-resolver/Cargo.toml

@@ -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/src/marker.rs

@@ -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/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.
 ///