uv-resolver: filter out dependencies whose markers are always false

Resolution itself won't produce dependencies that can never be
installed. But after we get a resolution, we "simplify" the markers for
each dependency when in universal mode relative to the requires-python
specification. And this step can produce a marker that always evaluates
to false. It doesn't make sense to write these out, so this commit
filters them out.

We do the filtering in a somewhat principled way. Namely,
the `SimplifiedMarkerTree` constructor is now fallible, which forces
callers to deal with the fact that dependencies might be excluded after
simplification.

Author: BurntSushi

crates/uv-resolver/src/lock/mod.rs

@@ -719,7 +719,9 @@ impl Lock {
             let fork_markers = each_element_on_its_line_array(
                 self.fork_markers
                     .iter()
-                    .map(|marker| SimplifiedMarkerTree::new(&self.requires_python, marker.clone()))
+                    .filter_map(|marker| {
+                        SimplifiedMarkerTree::new(&self.requires_python, marker.clone())
+                    })
                     .filter_map(|marker| marker.try_to_string()),
             );
             doc.insert("resolution-markers", value(fork_markers));
@@ -729,7 +731,9 @@ impl Lock {
             let supported_environments = each_element_on_its_line_array(
                 self.supported_environments
                     .iter()
-                    .map(|marker| SimplifiedMarkerTree::new(&self.requires_python, marker.clone()))
+                    .filter_map(|marker| {
+                        SimplifiedMarkerTree::new(&self.requires_python, marker.clone())
+                    })
                     .filter_map(|marker| marker.try_to_string()),
             );
             doc.insert("supported-markers", value(supported_environments));
@@ -1614,15 +1618,22 @@ impl Package {
     }
 
     /// Add the [`AnnotatedDist`] as a dependency of the [`Package`].
+    ///
+    /// This may skip adding the dependency if the combination of its markers
+    /// and the requires-python constraint result in a scenario where the
+    /// dependency can never be installed.
     fn add_dependency(
         &mut self,
         requires_python: &RequiresPython,
         annotated_dist: &AnnotatedDist,
         marker: MarkerTree,
         root: &Path,
     ) -> Result<(), LockError> {
-        let new_dep =
-            Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
+        let Some(new_dep) =
+            Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?
+        else {
+            return Ok(());
+        };
         for existing_dep in &mut self.dependencies {
             if existing_dep.package_id == new_dep.package_id
                 // It's important that we do a comparison on
@@ -1659,6 +1670,10 @@ impl Package {
     }
 
     /// Add the [`AnnotatedDist`] as an optional dependency of the [`Package`].
+    ///
+    /// This may skip adding the dependency if the combination of its markers
+    /// and the requires-python constraint result in a scenario where the
+    /// dependency can never be installed.
     fn add_optional_dependency(
         &mut self,
         requires_python: &RequiresPython,
@@ -1667,7 +1682,11 @@ impl Package {
         marker: MarkerTree,
         root: &Path,
     ) -> Result<(), LockError> {
-        let dep = Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
+        let Some(dep) =
+            Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?
+        else {
+            return Ok(());
+        };
         let optional_deps = self.optional_dependencies.entry(extra).or_default();
         for existing_dep in &mut *optional_deps {
             if existing_dep.package_id == dep.package_id
@@ -1685,6 +1704,10 @@ impl Package {
     }
 
     /// Add the [`AnnotatedDist`] to a dependency group of the [`Package`].
+    ///
+    /// This may skip adding the dependency if the combination of its markers
+    /// and the requires-python constraint result in a scenario where the
+    /// dependency can never be installed.
     fn add_group_dependency(
         &mut self,
         requires_python: &RequiresPython,
@@ -1693,7 +1716,11 @@ impl Package {
         marker: MarkerTree,
         root: &Path,
     ) -> Result<(), LockError> {
-        let dep = Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
+        let Some(dep) =
+            Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?
+        else {
+            return Ok(());
+        };
         let deps = self.dependency_groups.entry(group).or_default();
         for existing_dep in &mut *deps {
             if existing_dep.package_id == dep.package_id
@@ -2026,7 +2053,7 @@ impl Package {
             let wheels = each_element_on_its_line_array(
                 self.fork_markers
                     .iter()
-                    .map(|marker| SimplifiedMarkerTree::new(requires_python, marker.clone()))
+                    .filter_map(|marker| SimplifiedMarkerTree::new(requires_python, marker.clone()))
                     .filter_map(|marker| marker.try_to_string()),
             );
             table.insert("resolution-markers", value(wheels));
@@ -3559,23 +3586,27 @@ impl Dependency {
         package_id: PackageId,
         extra: BTreeSet<ExtraName>,
         complexified_marker: MarkerTree,
-    ) -> Dependency {
+    ) -> Option<Dependency> {
         let simplified_marker =
-            SimplifiedMarkerTree::new(requires_python, complexified_marker.clone());
-        Dependency {
+            SimplifiedMarkerTree::new(requires_python, complexified_marker.clone())?;
+        Some(Dependency {
             package_id,
             extra,
             simplified_marker,
             complexified_marker,
-        }
+        })
     }
 
+    /// Creates a new `Dependency` from the given annotated distribution.
+    ///
+    /// This can return `None` if the marker given is simplified by
+    /// `requires-python` to a marker that is always false.
     fn from_annotated_dist(
         requires_python: &RequiresPython,
         annotated_dist: &AnnotatedDist,
         complexified_marker: MarkerTree,
         root: &Path,
-    ) -> Result<Dependency, LockError> {
+    ) -> Result<Option<Dependency>, LockError> {
         let package_id = PackageId::from_annotated_dist(annotated_dist, root)?;
         let extra = annotated_dist.extra.iter().cloned().collect();
         Ok(Dependency::new(

crates/uv-resolver/src/requires_python.rs

@@ -328,6 +328,12 @@ impl RequiresPython {
     /// a lock file are deserialized and turned into a `ResolutionGraph`, the
     /// markers are "complexified" to put the `requires-python` assumption back
     /// into the marker explicitly.
+    ///
+    /// Note that this may convert a marker tree that is sometime true to
+    /// a marker tree that is always false. For example, given the marker
+    /// `python_version < '3.5'` and `requires-python = ">=3.8"`, the marker
+    /// can never be true. When printed, the marker looks like `python_version
+    /// < '0'`.
     pub(crate) fn simplify_markers(&self, marker: MarkerTree) -> MarkerTree {
         let (lower, upper) = (self.range().lower(), self.range().upper());
         marker.simplify_python_versions(lower.0.as_ref(), upper.0.as_ref())
@@ -548,11 +554,20 @@ pub(crate) struct SimplifiedMarkerTree(MarkerTree);
 impl SimplifiedMarkerTree {
     /// Simplifies the given markers by assuming the given `requires-python`
     /// bound is true.
+    ///
+    /// If simplifying the marker expression given would result in a marker
+    /// that is always false (i.e., the corresponding dependency should never
+    /// be installed under any circumstances), then this returns `None`.
     pub(crate) fn new(
         requires_python: &RequiresPython,
         marker: MarkerTree,
-    ) -> SimplifiedMarkerTree {
-        SimplifiedMarkerTree(requires_python.simplify_markers(marker))
+    ) -> Option<SimplifiedMarkerTree> {
+        let simplified = requires_python.simplify_markers(marker);
+        if simplified.is_false() {
+            None
+        } else {
+            Some(SimplifiedMarkerTree(simplified))
+        }
     }
 
     /// Complexifies the given markers by adding the given `requires-python` as

crates/uv-resolver/src/resolution/display.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::collections::BTreeSet;
 
 use owo_colors::OwoColorize;
@@ -180,13 +181,16 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
         // Print out the dependency graph.
         for (index, node) in nodes {
             // Display the node itself.
-            let mut line = node
+            let Some(mut line) = node
                 .to_requirements_txt(
                     &self.resolution.requires_python,
                     self.include_extras,
                     self.include_markers,
                 )
-                .to_string();
+                .map(Cow::into_owned)
+            else {
+                continue;
+            };
 
             // Display the distribution hashes, if any.
             let mut has_hashes = false;

crates/uv-resolver/src/resolution/requirements_txt.rs

@@ -37,12 +37,12 @@ impl<'dist> RequirementsTxtDist<'dist> {
         requires_python: &RequiresPython,
         include_extras: bool,
         include_markers: bool,
-    ) -> Cow<str> {
+    ) -> Option<Cow<str>> {
         // If the URL is editable, write it as an editable requirement.
         if self.dist.is_editable() {
             if let VersionOrUrlRef::Url(url) = self.dist.version_or_url() {
                 let given = url.verbatim();
-                return Cow::Owned(format!("-e {given}"));
+                return Some(Cow::Owned(format!("-e {given}")));
             }
         }
 
@@ -93,21 +93,23 @@ impl<'dist> RequirementsTxtDist<'dist> {
                     }
                 };
                 if let Some(given) = given {
-                    return if let Some(markers) =
-                        SimplifiedMarkerTree::new(requires_python, self.markers.clone())
-                            .try_to_string()
-                            .filter(|_| include_markers)
-                    {
-                        Cow::Owned(format!("{given} ; {markers}"))
-                    } else {
-                        given
-                    };
+                    return Some(
+                        if let Some(markers) =
+                            SimplifiedMarkerTree::new(requires_python, self.markers.clone())?
+                                .try_to_string()
+                                .filter(|_| include_markers)
+                        {
+                            Cow::Owned(format!("{given} ; {markers}"))
+                        } else {
+                            given
+                        },
+                    );
                 }
             }
         }
 
-        if self.extras.is_empty() || !include_extras {
-            if let Some(markers) = SimplifiedMarkerTree::new(requires_python, self.markers.clone())
+        Some(if self.extras.is_empty() || !include_extras {
+            if let Some(markers) = SimplifiedMarkerTree::new(requires_python, self.markers.clone())?
                 .try_to_string()
                 .filter(|_| include_markers)
             {
@@ -119,7 +121,7 @@ impl<'dist> RequirementsTxtDist<'dist> {
             let mut extras = self.extras.clone();
             extras.sort_unstable();
             extras.dedup();
-            if let Some(markers) = SimplifiedMarkerTree::new(requires_python, self.markers.clone())
+            if let Some(markers) = SimplifiedMarkerTree::new(requires_python, self.markers.clone())?
                 .try_to_string()
                 .filter(|_| include_markers)
             {
@@ -138,7 +140,7 @@ impl<'dist> RequirementsTxtDist<'dist> {
                     self.version_or_url().verbatim()
                 ))
             }
-        }
+        })
     }
 
     pub(crate) fn to_comparator(&self) -> RequirementsTxtComparator {

crates/uv/tests/it/pip_compile.rs

@@ -484,14 +484,10 @@ typing_extensions==4.3.0
     apache-airflow==2.3.4     # via apache-airflow-providers-microsoft-azure, -r requirements.in
     apache-airflow-providers-common-sql==1.4.0  # via apache-airflow-providers-sqlite
     apache-airflow-providers-ftp==3.3.1  # via apache-airflow
-    apache-airflow-providers-ftp==3.7.0 ; python_version < '0'  # via apache-airflow
     apache-airflow-providers-http==4.3.0  # via apache-airflow
-    apache-airflow-providers-http==4.10.0 ; python_version < '0'  # via apache-airflow
     apache-airflow-providers-imap==3.1.1  # via apache-airflow
-    apache-airflow-providers-imap==3.5.0 ; python_version < '0'  # via apache-airflow
     apache-airflow-providers-microsoft-azure==4.2.0  # via apache-airflow, -c constraints.txt
     apache-airflow-providers-sqlite==3.3.2  # via apache-airflow
-    apache-airflow-providers-sqlite==3.7.1 ; python_version < '0'  # via apache-airflow
     apispec==3.3.2            # via flask-appbuilder
     argcomplete==3.2.3        # via apache-airflow
     asgiref==3.8.1            # via apache-airflow-providers-http, connexion, flask
@@ -501,13 +497,10 @@ typing_extensions==4.3.0
     azure-batch==14.1.0       # via apache-airflow-providers-microsoft-azure
     azure-common==1.1.28      # via azure-batch, azure-keyvault-secrets, azure-mgmt-containerinstance, azure-mgmt-datafactory, azure-mgmt-datalake-store, azure-mgmt-resource, azure-storage-common, azure-storage-file
     azure-core==1.29.1        # via azure-cosmos, azure-identity, azure-keyvault-secrets, azure-mgmt-core, azure-servicebus, azure-storage-blob, msrest
-    azure-core==1.30.1 ; python_version < '0'  # via azure-identity
     azure-cosmos==4.5.1       # via apache-airflow-providers-microsoft-azure
-    azure-cosmos==4.6.0 ; python_version < '0'  # via apache-airflow-providers-microsoft-azure
     azure-datalake-store==0.0.53  # via apache-airflow-providers-microsoft-azure
     azure-identity==1.10.0    # via apache-airflow-providers-microsoft-azure, -c constraints.txt
     azure-keyvault-secrets==4.7.0  # via apache-airflow-providers-microsoft-azure
-    azure-keyvault-secrets==4.8.0 ; python_version < '0'  # via apache-airflow-providers-microsoft-azure
     azure-kusto-data==0.0.45  # via apache-airflow-providers-microsoft-azure
     azure-mgmt-containerinstance==1.5.0  # via apache-airflow-providers-microsoft-azure
     azure-mgmt-core==1.4.0    # via azure-mgmt-datafactory, azure-mgmt-resource
@@ -523,11 +516,9 @@ typing_extensions==4.3.0
     azure-storage-file==2.1.0  # via apache-airflow-providers-microsoft-azure
     babel==2.14.0             # via flask-babel
     black==22.12.0            # via -r requirements.in
-    black==24.3.0 ; python_version < '0'  # via -r requirements.in
     blinker==1.7.0            # via apache-airflow
     cachelib==0.9.0           # via flask-caching
     cattrs==23.1.2            # via apache-airflow
-    cattrs==23.2.3 ; python_version < '0'  # via apache-airflow
     certifi==2024.2.2         # via httpcore, httpx, msrest, requests
     cffi==1.16.0              # via azure-datalake-store, cryptography
     charset-normalizer==3.3.2  # via requests
@@ -571,7 +562,6 @@ typing_extensions==4.3.0
     jinja2==3.1.3             # via apache-airflow, connexion, flask, flask-babel, python-nvd3, swagger-ui-bundle
     joblib==1.3.2             # via scikit-learn
     jsonschema==4.17.3        # via apache-airflow, connexion, flask-appbuilder
-    jsonschema==4.21.1 ; python_version < '0'  # via apache-airflow, flask-appbuilder
     lazy-object-proxy==1.10.0  # via apache-airflow
     linkify-it-py==2.0.3      # via apache-airflow
     lockfile==0.12.2          # via apache-airflow, python-daemon
@@ -626,7 +616,6 @@ typing_extensions==4.3.0
     requests-oauthlib==2.0.0  # via msrest
     requests-toolbelt==1.0.0  # via apache-airflow-providers-http
     rich==13.3.1              # via apache-airflow
-    rich==13.7.1 ; python_version < '0'  # via apache-airflow
     scikit-learn==1.2.2       # via -r requirements.in
     scipy==1.12.0             # via scikit-learn
     setproctitle==1.3.3       # via apache-airflow