From e7093c6563e358f5041cd1e087a76222a8b7dc5d Mon Sep 17 00:00:00 2001
From: Andrew Gallant <andrew@astral.sh>
Date: Fri, 22 Nov 2024 20:38:41 -0500
Subject: [PATCH] uv-resolver: evaluate extras when reading from lock

This ensures we only pull out dependencies that satisfy their conflict
markers.
---
 crates/uv-resolver/src/lock/mod.rs         |  2 +-
 crates/uv-resolver/src/lock/target.rs      |  7 ++----
 crates/uv-resolver/src/universal_marker.rs | 25 ++++++++++++++++++++--
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs
index 6966782cd33ed..cb8a192f9cb7f 100644
--- a/crates/uv-resolver/src/lock/mod.rs
+++ b/crates/uv-resolver/src/lock/mod.rs
@@ -3618,7 +3618,7 @@ struct DependencyWire {
     extra: BTreeSet<ExtraName>,
     #[serde(default)]
     marker: SimplifiedMarkerTree,
-    #[serde(default)]
+    #[serde(rename = "conflict-marker", default)]
     conflict_marker: MarkerTree,
 }
 
diff --git a/crates/uv-resolver/src/lock/target.rs b/crates/uv-resolver/src/lock/target.rs
index 1871893d5110c..6d68fed540e7b 100644
--- a/crates/uv-resolver/src/lock/target.rs
+++ b/crates/uv-resolver/src/lock/target.rs
@@ -221,7 +221,7 @@ impl<'env> InstallTarget<'env> {
                 })
                 .flatten()
             {
-                if !dep.complexified_marker.evaluate(marker_env, &[]) {
+                if !dep.complexified_marker.satisfies(marker_env, extras, dev) {
                     continue;
                 }
 
@@ -252,9 +252,6 @@ impl<'env> InstallTarget<'env> {
                     // a specific marker environment and set of extras/groups.
                     // So at this point, we know the extras/groups have been
                     // satisfied, so we can safely drop the conflict marker.
-                    //
-                    // FIXME: Make the above true. We aren't actually checking
-                    // the conflict marker yet.
                     Edge::Dev(group.clone(), dep.complexified_marker.pep508().clone()),
                 );
 
@@ -350,7 +347,7 @@ impl<'env> InstallTarget<'env> {
                 Either::Right(package.dependencies.iter())
             };
             for dep in deps {
-                if !dep.complexified_marker.evaluate(marker_env, &[]) {
+                if !dep.complexified_marker.satisfies(marker_env, extras, dev) {
                     continue;
                 }
 
diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs
index 5e1590b34efe6..1d09ba21457b8 100644
--- a/crates/uv-resolver/src/universal_marker.rs
+++ b/crates/uv-resolver/src/universal_marker.rs
@@ -1,5 +1,6 @@
 use itertools::Itertools;
 
+use uv_configuration::{DevGroupsManifest, ExtrasSpecification};
 use uv_normalize::ExtraName;
 use uv_pep508::{MarkerEnvironment, MarkerTree};
 use uv_pypi_types::Conflicts;
@@ -135,12 +136,32 @@ impl UniversalMarker {
 
     /// Returns true if this universal marker is satisfied by the given
     /// marker environment and list of activated extras.
-    ///
-    /// FIXME: This also needs to accept a list of groups.
     pub(crate) fn evaluate(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool {
         self.pep508_marker.evaluate(env, extras) && self.conflict_marker.evaluate(env, extras)
     }
 
+    /// Returns true if this universal marker is satisfied by the given
+    /// marker environment and list of activated extras and groups.
+    ///
+    /// TODO: Articulate the difference between this and `evaluate`.
+    pub(crate) fn satisfies(
+        &self,
+        env: &MarkerEnvironment,
+        extras: &ExtrasSpecification,
+        _dev: &DevGroupsManifest,
+    ) -> bool {
+        if !self.pep508_marker.evaluate(env, &[]) {
+            return false;
+        }
+        let extra_list = match *extras {
+            // TODO(ag): This should still evaluate `dev`.
+            ExtrasSpecification::All => return true,
+            ExtrasSpecification::None => &[][..],
+            ExtrasSpecification::Some(ref list) => list,
+        };
+        self.conflict_marker.evaluate(env, extra_list)
+    }
+
     /// Returns the PEP 508 marker for this universal marker.
     ///
     /// One should be cautious using this. Generally speaking, it should only