From ae9c5c849dbb6a2efe2f173f7939efaeb7faef29 Mon Sep 17 00:00:00 2001
From: Andrew Gallant <andrew@astral.sh>
Date: Wed, 22 Jan 2025 13:36:14 -0500
Subject: [PATCH] uv-resolver: remove the conservative checking of
 unconditional extras

This removes the error that was causing folks problems.

This does result in some snapshot updates that are arguably wrong, or at
least sub-optimal. However, it's actually intended. Because the approach
we're going to take is going to permit the creation of uninstallable
lock files as a side effect. In the future, we will modify this test to
check that, while `uv lock` succeeds, `uv sync` will always fail.
---
 crates/uv-resolver/src/error.rs        |  9 +-----
 crates/uv-resolver/src/resolver/mod.rs | 40 --------------------------
 crates/uv/tests/it/lock_conflict.rs    | 18 ++++++------
 3 files changed, 10 insertions(+), 57 deletions(-)

diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs
index ef1a1689a..fc84e0e74 100644
--- a/crates/uv-resolver/src/error.rs
+++ b/crates/uv-resolver/src/error.rs
@@ -12,7 +12,7 @@ use tracing::trace;
 use uv_distribution_types::{
     DerivationChain, DistErrorKind, IndexCapabilities, IndexLocations, IndexUrl, RequestedDist,
 };
-use uv_normalize::{ExtraName, PackageName};
+use uv_normalize::PackageName;
 use uv_pep440::{LocalVersionSlice, Version};
 use uv_platform_tags::Tags;
 use uv_static::EnvVars;
@@ -48,13 +48,6 @@ pub enum ResolveError {
     #[error("Attempted to wait on an unregistered task: `{_0}`")]
     UnregisteredTask(String),
 
-    #[error("Found conflicting extra `{extra}` unconditionally enabled in `{requirement}`")]
-    ConflictingExtra {
-        // Boxed because `Requirement` is large.
-        requirement: Box<uv_pypi_types::Requirement>,
-        extra: ExtraName,
-    },
-
     #[error(
         "Requirements contain conflicting URLs for package `{package_name}`{}:\n- {}",
         if env.marker_environment().is_some() {
diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs
index d5f424e72..d479ec9fb 100644
--- a/crates/uv-resolver/src/resolver/mod.rs
+++ b/crates/uv-resolver/src/resolver/mod.rs
@@ -1682,16 +1682,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                     }
                 };
 
-                if let Some(err) = find_conflicting_extra(&self.conflicts, &metadata.requires_dist)
-                {
-                    return Err(err);
-                }
-                for dependencies in metadata.dependency_groups.values() {
-                    if let Some(err) = find_conflicting_extra(&self.conflicts, dependencies) {
-                        return Err(err);
-                    }
-                }
-
                 let requirements = self.flatten_requirements(
                     &metadata.requires_dist,
                     &metadata.dependency_groups,
@@ -3610,36 +3600,6 @@ pub(crate) struct VersionFork {
     version: Option<Version>,
 }
 
-/// Returns an error if a conflicting extra is found in the given requirements.
-///
-/// Specifically, if there is any conflicting extra (just one is enough) that
-/// is unconditionally enabled as part of a dependency specification, then this
-/// returns an error.
-///
-/// The reason why we're so conservative here is because it avoids us needing
-/// the look at the entire dependency tree at once.
-///
-/// For example, consider packages `root`, `a`, `b` and `c`, where `c` has
-/// declared conflicting extras of `x1` and `x2`.
-///
-/// Now imagine `root` depends on `a` and `b`, `a` depends on `c[x1]` and `b`
-/// depends on `c[x2]`. That's a conflict, but not easily detectable unless
-/// you reject either `c[x1]` or `c[x2]` on the grounds that `x1` and `x2` are
-/// conflicting and thus cannot be enabled unconditionally.
-fn find_conflicting_extra(conflicting: &Conflicts, reqs: &[Requirement]) -> Option<ResolveError> {
-    for req in reqs {
-        for extra in &req.extras {
-            if conflicting.contains(&req.name, extra) {
-                return Some(ResolveError::ConflictingExtra {
-                    requirement: Box::new(req.clone()),
-                    extra: extra.clone(),
-                });
-            }
-        }
-    }
-    None
-}
-
 #[derive(Debug, Default, Clone)]
 struct ConflictTracker {
     /// How often a decision on the package was discarded due to another package decided earlier.
diff --git a/crates/uv/tests/it/lock_conflict.rs b/crates/uv/tests/it/lock_conflict.rs
index c69d8cb98..902c86c48 100644
--- a/crates/uv/tests/it/lock_conflict.rs
+++ b/crates/uv/tests/it/lock_conflict.rs
@@ -1047,12 +1047,12 @@ fn extra_unconditional() -> Result<()> {
     )?;
 
     uv_snapshot!(context.filters(), context.lock(), @r###"
-    success: false
-    exit_code: 2
+    success: true
+    exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    error: Found conflicting extra `extra1` unconditionally enabled in `proxy1[extra1,extra2] @ file://[TEMP_DIR]/proxy1`
+    Resolved 6 packages in [TIME]
     "###);
 
     // An error should occur even when only one conflicting extra is enabled.
@@ -1078,12 +1078,12 @@ fn extra_unconditional() -> Result<()> {
         "#,
     )?;
     uv_snapshot!(context.filters(), context.lock(), @r###"
-    success: false
-    exit_code: 2
+    success: true
+    exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    error: Found conflicting extra `extra1` unconditionally enabled in `proxy1[extra1] @ file://[TEMP_DIR]/proxy1`
+    Resolved 6 packages in [TIME]
     "###);
 
     // And same thing for the other extra.
@@ -1110,12 +1110,12 @@ fn extra_unconditional() -> Result<()> {
         "#,
     )?;
     uv_snapshot!(context.filters(), context.lock(), @r###"
-    success: false
-    exit_code: 2
+    success: true
+    exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    error: Found conflicting extra `extra2` unconditionally enabled in `proxy1[extra2] @ file://[TEMP_DIR]/proxy1`
+    Resolved 6 packages in [TIME]
     "###);
 
     Ok(())