uv-resolver: require PackageName with ExtraName for ConflictMarker

This utilizes the new `ConflictMarker` type to include package names
when evaluating conflict markers.

crates/uv-resolver/src/graph_ops.rs

@@ -4,7 +4,7 @@ use petgraph::{Direction, Graph};
 use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
 use std::collections::hash_map::Entry;
 
-use uv_normalize::ExtraName;
+use uv_normalize::{ExtraName, PackageName};
 
 use crate::resolution::ResolutionGraphNode;
 use crate::universal_marker::UniversalMarker;
@@ -91,7 +91,7 @@ pub(crate) fn marker_reachability<T>(
 pub(crate) fn simplify_conflict_markers(graph: &mut Graph<ResolutionGraphNode, UniversalMarker>) {
     // The set of activated extras (and TODO, in the future, groups)
     // for each node. The ROOT nodes don't have any extras activated.
-    let mut activated: FxHashMap<NodeIndex, FxHashSet<ExtraName>> =
+    let mut activated: FxHashMap<NodeIndex, FxHashSet<(PackageName, ExtraName)>> =
         FxHashMap::with_capacity_and_hasher(graph.node_count(), FxBuildHasher);
 
     // Collect the root nodes.
@@ -108,20 +108,21 @@ pub(crate) fn simplify_conflict_markers(graph: &mut Graph<ResolutionGraphNode, U
         })
         .collect();
 
-    let mut assume_by_edge: FxHashMap<EdgeIndex, FxHashSet<ExtraName>> = FxHashMap::default();
+    let mut assume_by_edge: FxHashMap<EdgeIndex, FxHashSet<(PackageName, ExtraName)>> =
+        FxHashMap::default();
     let mut seen: FxHashSet<NodeIndex> = FxHashSet::default();
     while let Some(parent_index) = queue.pop() {
         for child_edge in graph.edges_directed(parent_index, Direction::Outgoing) {
             // TODO: The below seems excessively clone-y.
             // Consider tightening this up a bit.
             let target = child_edge.target();
-            let mut extras: FxHashSet<ExtraName> =
+            let mut extras: FxHashSet<(PackageName, ExtraName)> =
                 activated.get(&parent_index).cloned().unwrap_or_default();
-            if let Some(extra) = graph[parent_index].extra() {
-                extras.insert(extra.clone());
+            if let Some((package, extra)) = graph[parent_index].package_extra_names() {
+                extras.insert((package.clone(), extra.clone()));
             }
-            if let Some(extra) = graph[target].extra() {
-                extras.insert(extra.clone());
+            if let Some((package, extra)) = graph[target].package_extra_names() {
+                extras.insert((package.clone(), extra.clone()));
             }
             activated.entry(target).or_default().extend(extras.clone());
             assume_by_edge
@@ -134,8 +135,8 @@ pub(crate) fn simplify_conflict_markers(graph: &mut Graph<ResolutionGraphNode, U
         }
     }
     for (edge_id, extras) in assume_by_edge {
-        for extra in &extras {
-            graph[edge_id].assume_extra(extra);
+        for &(ref package, ref extra) in &extras {
+            graph[edge_id].assume_extra(package, extra);
         }
     }
 }

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

@@ -164,6 +164,7 @@ impl<'env> InstallTarget<'env> {
 
         let mut queue: VecDeque<(&Package, Option<&ExtraName>)> = VecDeque::new();
         let mut seen = FxHashSet::default();
+        let mut activated_extras: Vec<(PackageName, ExtraName)> = vec![];
 
         let root = petgraph.add_node(Node::Root);
 
@@ -198,11 +199,13 @@ impl<'env> InstallTarget<'env> {
                     ExtrasSpecification::All => {
                         for extra in dist.optional_dependencies.keys() {
                             queue.push_back((dist, Some(extra)));
+                            activated_extras.push((dist.id.name.clone(), extra.clone()));
                         }
                     }
                     ExtrasSpecification::Some(extras) => {
                         for extra in extras {
                             queue.push_back((dist, Some(extra)));
+                            activated_extras.push((dist.id.name.clone(), extra.clone()));
                         }
                     }
                 }
@@ -221,7 +224,10 @@ impl<'env> InstallTarget<'env> {
                 })
                 .flatten()
             {
-                if !dep.complexified_marker.satisfies(marker_env, extras, dev) {
+                if !dep
+                    .complexified_marker
+                    .satisfies(marker_env, &activated_extras, dev)
+                {
                     continue;
                 }
 
@@ -347,7 +353,10 @@ impl<'env> InstallTarget<'env> {
                 Either::Right(package.dependencies.iter())
             };
             for dep in deps {
-                if !dep.complexified_marker.satisfies(marker_env, extras, dev) {
+                if !dep
+                    .complexified_marker
+                    .satisfies(marker_env, &activated_extras, dev)
+                {
                     continue;
                 }
 

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

@@ -73,10 +73,13 @@ impl ResolutionGraphNode {
         }
     }
 
-    pub(crate) fn extra(&self) -> Option<&ExtraName> {
+    pub(crate) fn package_extra_names(&self) -> Option<(&PackageName, &ExtraName)> {
         match *self {
             ResolutionGraphNode::Root => None,
-            ResolutionGraphNode::Dist(ref dist) => dist.extra.as_ref(),
+            ResolutionGraphNode::Dist(ref dist) => {
+                let extra = dist.extra.as_ref()?;
+                Some((&dist.name, extra))
+            }
         }
     }
 }

crates/uv-resolver/src/resolver/environment.rs

@@ -383,13 +383,14 @@ impl ResolverEnvironment {
                 let mut conflict_marker = ConflictMarker::TRUE;
                 for item in exclude.iter() {
                     if let Some(extra) = item.extra() {
-                        conflict_marker =
-                            conflict_marker.and(ConflictMarker::extra(extra.clone()).negate());
+                        conflict_marker = conflict_marker
+                            .and(ConflictMarker::extra(item.package(), extra).negate());
                     }
                 }
                 for item in include.iter() {
                     if let Some(extra) = item.extra() {
-                        conflict_marker = conflict_marker.and(ConflictMarker::extra(extra.clone()));
+                        conflict_marker =
+                            conflict_marker.and(ConflictMarker::extra(item.package(), extra));
                     }
                 }
                 Some(UniversalMarker::new(markers.clone(), conflict_marker))

crates/uv-resolver/src/universal_marker.rs

@@ -3,7 +3,7 @@
 use itertools::Itertools;
 
 use uv_configuration::{DevGroupsManifest, ExtrasSpecification};
-use uv_normalize::ExtraName;
+use uv_normalize::{ExtraName, PackageName};
 use uv_pep508::{MarkerEnvironment, MarkerEnvironmentBuilder, MarkerTree};
 use uv_pypi_types::Conflicts;
 
@@ -95,8 +95,8 @@ impl UniversalMarker {
                 let (Some(extra1), Some(extra2)) = (item1.extra(), item2.extra()) else {
                     continue;
                 };
-                let pair = ConflictMarker::extra(extra1.clone())
-                    .and(ConflictMarker::extra(extra2.clone()));
+                let pair = ConflictMarker::extra(item1.package(), extra1)
+                    .and(ConflictMarker::extra(item2.package(), extra2));
                 marker = marker.or(pair);
             }
         }
@@ -105,12 +105,12 @@ impl UniversalMarker {
             .implies(std::mem::take(&mut self.conflict_marker));
     }
 
-    /// Assumes that a given extra is activated.
+    /// Assumes that a given extra for the given package is activated.
     ///
     /// This may simplify the conflicting marker component of this universal
     /// marker.
-    pub(crate) fn assume_extra(&mut self, extra: &ExtraName) {
-        self.conflict_marker = self.conflict_marker.assume_extra(extra);
+    pub(crate) fn assume_extra(&mut self, package: &PackageName, extra: &ExtraName) {
+        self.conflict_marker = self.conflict_marker.assume_extra(package, extra);
     }
 
     /// Returns true if this universal marker will always evaluate to `true`.
@@ -135,7 +135,15 @@ impl UniversalMarker {
     /// Returns true if this universal marker is satisfied by the given
     /// marker environment and list of activated extras.
     pub(crate) fn evaluate(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool {
-        self.pep508_marker.evaluate(env, extras) && self.conflict_marker.evaluate(extras)
+        // We specifically evaluate the conflict marker with an empty set of
+        // extras because this seems to best capture the intent here. Namely,
+        // in this context, we don't know the packages associated with the
+        // given extras, which means we can't really evaluate the conflict
+        // marker. But note that evaluation here is not vacuous. By providing
+        // an empty list, conflict markers like `extra != 'package[x1]'`
+        // will evaluate to `true`, while conflict markers like
+        // `extra == 'package[x2]'` will evaluate to `false`.
+        self.pep508_marker.evaluate(env, extras) && self.conflict_marker.evaluate(&[])
     }
 
     /// Returns true if this universal marker is satisfied by the given
@@ -145,19 +153,19 @@ impl UniversalMarker {
     pub(crate) fn satisfies(
         &self,
         env: &MarkerEnvironment,
-        extras: &ExtrasSpecification,
+        activated_extras: &[(PackageName, ExtraName)],
         _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(extra_list)
+        // 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(activated_extras)
     }
 
     /// Returns the PEP 508 marker for this universal marker.
@@ -227,11 +235,11 @@ impl ConflictMarker {
         marker: MarkerTree::FALSE,
     };
 
-    /// Create a conflict marker that is true only when the given extra is
-    /// activated.
-    pub fn extra(name: ExtraName) -> ConflictMarker {
+    /// Create a conflict marker that is true only when the given extra for the
+    /// given package is activated.
+    pub fn extra(package: &PackageName, extra: &ExtraName) -> ConflictMarker {
         let operator = uv_pep508::ExtraOperator::Equal;
-        let name = uv_pep508::MarkerValueExtra::Extra(name);
+        let name = uv_pep508::MarkerValueExtra::Extra(encode_package_extra(package, extra));
         let expr = uv_pep508::MarkerExpression::Extra { operator, name };
         let marker = MarkerTree::expression(expr);
         ConflictMarker { marker }
@@ -279,11 +287,12 @@ impl ConflictMarker {
     ///
     /// This may simplify the marker.
     #[must_use]
-    pub(crate) fn assume_extra(&self, extra: &ExtraName) -> ConflictMarker {
+    pub(crate) fn assume_extra(&self, package: &PackageName, extra: &ExtraName) -> ConflictMarker {
+        let extra = encode_package_extra(package, extra);
         let marker = self
             .marker
             .clone()
-            .simplify_extras_with(|candidate| candidate == extra);
+            .simplify_extras_with(|candidate| *candidate == extra);
         ConflictMarker { marker }
     }
 
@@ -307,7 +316,7 @@ impl ConflictMarker {
 
     /// Returns true if this conflict marker is satisfied by the given
     /// list of activated extras.
-    pub(crate) fn evaluate(&self, extras: &[ExtraName]) -> bool {
+    pub(crate) fn evaluate(&self, extras: &[(PackageName, ExtraName)]) -> bool {
         static DUMMY: std::sync::LazyLock<MarkerEnvironment> = std::sync::LazyLock::new(|| {
             MarkerEnvironment::try_from(MarkerEnvironmentBuilder {
                 implementation_name: "",
@@ -324,7 +333,11 @@ impl ConflictMarker {
             })
             .unwrap()
         });
-        self.marker.evaluate(&DUMMY, extras)
+        let extras = extras
+            .iter()
+            .map(|&(ref package, ref extra)| encode_package_extra(package, extra))
+            .collect::<Vec<ExtraName>>();
+        self.marker.evaluate(&DUMMY, &extras)
     }
 }
 
@@ -382,3 +395,10 @@ impl serde::Serialize for ConflictMarker {
         serializer.serialize_str(&self.to_string())
     }
 }
+
+fn encode_package_extra(package: &PackageName, extra: &ExtraName) -> ExtraName {
+    // This is OK because `PackageName` and `ExtraName` have the same
+    // validation rules, and we combine them in a way that always results
+    // in a valid name.
+    ExtraName::new(format!("extra-{package}-{extra}")).unwrap()
+}

crates/uv/tests/it/lock.rs

@@ -4067,7 +4067,7 @@ fn lock_conflicting_mixed() -> Result<()> {
 
         [package.dev-dependencies]
         group1 = [
-            { name = "sortedcontainers", version = "2.3.0", source = { registry = "https://pypi.org/simple" }, conflict-marker = "extra != 'extra1'" },
+            { name = "sortedcontainers", version = "2.3.0", source = { registry = "https://pypi.org/simple" }, conflict-marker = "extra != 'extra-project-extra1'" },
         ]
 
         [package.metadata]