Unroll self-dependencies via extras (#3230)

## Summary

We now recursively expand any self-dependencies via extras, which lets
us detect conflicts sooner and avoid building unnecessary versions of
packages that are excluded via the extra.

Closes https://github.com/astral-sh/uv/issues/3135.

crates/uv-resolver/src/pubgrub/dependencies.rs

@@ -1,5 +1,6 @@
 use itertools::Itertools;
 use pubgrub::range::Range;
+use rustc_hash::FxHashSet;
 use tracing::warn;
 
 use distribution_types::Verbatim;
@@ -30,70 +31,20 @@ impl PubGrubDependencies {
         env: &MarkerEnvironment,
     ) -> Result<Self, ResolveError> {
         let mut dependencies = Vec::default();
+        let mut seen = FxHashSet::default();
 
-        // Iterate over all declared requirements.
+        add_requirements(
-        for requirement in overrides.apply(requirements) {
+            requirements,
-            // If the requirement isn't relevant for the current platform, skip it.
+            constraints,
-            if let Some(extra) = source_extra {
+            overrides,
-                if !requirement.evaluate_markers(env, std::slice::from_ref(extra)) {
+            source_name,
-                    continue;
+            source_extra,
-                }
+            urls,
-            } else if !requirement.evaluate_markers(env, &[]) {
+            locals,
-                continue;
+            env,
-            }
+            &mut dependencies,
-
+            &mut seen,
-            // Add the package, plus any extra variants.
+        )?;
-            for result in std::iter::once(to_pubgrub(requirement, None, urls, locals)).chain(
-                requirement
-                    .extras
-                    .clone()
-                    .into_iter()
-                    .map(|extra| to_pubgrub(requirement, Some(extra), urls, locals)),
-            ) {
-                let (package, version) = result?;
-
-                // Detect self-dependencies.
-                if let PubGrubPackage::Package(name, extra, ..) = &package {
-                    if source_name.is_some_and(|source_name| source_name == name) {
-                        // Allow, e.g., `black` to depend on `black[colorama]`.
-                        if source_extra == extra.as_ref() {
-                            warn!("{name} has a dependency on itself");
-                            continue;
-                        }
-                    }
-                }
-
-                dependencies.push((package.clone(), version.clone()));
-
-                // If the requirement was constrained, add those constraints.
-                for constraint in constraints.get(&requirement.name).into_iter().flatten() {
-                    // If the requirement isn't relevant for the current platform, skip it.
-                    if let Some(extra) = source_extra {
-                        if !constraint.evaluate_markers(env, std::slice::from_ref(extra)) {
-                            continue;
-                        }
-                    } else if !constraint.evaluate_markers(env, &[]) {
-                        continue;
-                    }
-
-                    // Add the package.
-                    let (package, version) = to_pubgrub(constraint, None, urls, locals)?;
-
-                    // Detect self-dependencies.
-                    if let PubGrubPackage::Package(name, extra, ..) = &package {
-                        if source_name.is_some_and(|source_name| source_name == name) {
-                            // Allow, e.g., `black` to depend on `black[colorama]`.
-                            if source_extra == extra.as_ref() {
-                                warn!("{name} has a dependency on itself");
-                                continue;
-                            }
-                        }
-                    }
-
-                    dependencies.push((package.clone(), version.clone()));
-                }
-            }
-        }
 
         Ok(Self(dependencies))
     }
@@ -109,6 +60,117 @@ impl PubGrubDependencies {
     }
 }
 
+/// Add a set of requirements to a list of dependencies.
+#[allow(clippy::too_many_arguments)]
+fn add_requirements(
+    requirements: &[Requirement],
+    constraints: &Constraints,
+    overrides: &Overrides,
+    source_name: Option<&PackageName>,
+    source_extra: Option<&ExtraName>,
+    urls: &Urls,
+    locals: &Locals,
+    env: &MarkerEnvironment,
+    dependencies: &mut Vec<(PubGrubPackage, Range<Version>)>,
+    seen: &mut FxHashSet<ExtraName>,
+) -> Result<(), ResolveError> {
+    // Iterate over all declared requirements.
+    for requirement in overrides.apply(requirements) {
+        // If the requirement isn't relevant for the current platform, skip it.
+        match source_extra {
+            Some(source_extra) => {
+                if !requirement.evaluate_markers(env, std::slice::from_ref(source_extra)) {
+                    continue;
+                }
+            }
+            None => {
+                if !requirement.evaluate_markers(env, &[]) {
+                    continue;
+                }
+            }
+        }
+
+        // Add the package, plus any extra variants.
+        for result in std::iter::once(to_pubgrub(requirement, None, urls, locals)).chain(
+            requirement
+                .extras
+                .clone()
+                .into_iter()
+                .map(|extra| to_pubgrub(requirement, Some(extra), urls, locals)),
+        ) {
+            let (package, version) = result?;
+
+            match &package {
+                PubGrubPackage::Package(name, ..) => {
+                    // Detect self-dependencies.
+                    if source_name.is_some_and(|source_name| source_name == name) {
+                        warn!("{name} has a dependency on itself");
+                        continue;
+                    }
+
+                    dependencies.push((package.clone(), version.clone()));
+                }
+                PubGrubPackage::Extra(name, extra, ..) => {
+                    // Recursively add the dependencies of the current package (e.g., `black` depending on
+                    // `black[colorama]`).
+                    if source_name.is_some_and(|source_name| source_name == name) {
+                        if seen.insert(extra.clone()) {
+                            add_requirements(
+                                requirements,
+                                constraints,
+                                overrides,
+                                source_name,
+                                Some(extra),
+                                urls,
+                                locals,
+                                env,
+                                dependencies,
+                                seen,
+                            )?;
+                        }
+                    } else {
+                        dependencies.push((package.clone(), version.clone()));
+                    }
+                }
+                _ => {}
+            }
+
+            // If the requirement was constrained, add those constraints.
+            for constraint in constraints.get(&requirement.name).into_iter().flatten() {
+                // If the requirement isn't relevant for the current platform, skip it.
+                match source_extra {
+                    Some(source_extra) => {
+                        if !constraint.evaluate_markers(env, std::slice::from_ref(source_extra)) {
+                            continue;
+                        }
+                    }
+                    None => {
+                        if !constraint.evaluate_markers(env, &[]) {
+                            continue;
+                        }
+                    }
+                }
+
+                // Add the package.
+                let (package, version) = to_pubgrub(constraint, None, urls, locals)?;
+
+                // Ignore self-dependencies.
+                if let PubGrubPackage::Package(name, ..) = &package {
+                    // Detect self-dependencies.
+                    if source_name.is_some_and(|source_name| source_name == name) {
+                        warn!("{name} has a dependency on itself");
+                        continue;
+                    }
+                }
+
+                dependencies.push((package.clone(), version.clone()));
+            }
+        }
+    }
+
+    Ok(())
+}
+
 /// Convert a [`PubGrubDependencies`] to a [`DependencyConstraints`].
 impl From<PubGrubDependencies> for Vec<(PubGrubPackage, Range<Version>)> {
     fn from(dependencies: PubGrubDependencies) -> Self {

crates/uv/tests/pip_compile.rs

@@ -3985,6 +3985,56 @@ fn avoid_irrelevant_extras() -> Result<()> {
     Ok(())
 }
 
+/// `extras==0.0.2` fails to build (i.e., it always throws). `extras==0.0.1` is the only version
+/// that resolves the constraints, but if we don't visit `example[test]` prior to `extras==0.0.2`,
+/// we'll end up with a broken build.
+#[test]
+fn avoid_irrelevant_recursive_extras() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    // Create an editable package with an optional URL dependency.
+    let pyproject_toml = context.temp_dir.child("pyproject.toml");
+    pyproject_toml.write_str(
+        r#"[project]
+name = "example"
+version = "0.0.0"
+dependencies = []
+requires-python = '>=3.8'
+
+[project.optional-dependencies]
+test = ["extras<0.0.2"]
+coverage = ["example[test]", "extras>=0.0.1,<=0.0.2"]
+"#,
+    )?;
+
+    // Write to a requirements file.
+    let requirements_in = context.temp_dir.child("requirements.in");
+    requirements_in.write_str("-e .[test,coverage]")?;
+
+    uv_snapshot!(context.compile()
+        .arg("requirements.in")
+        .arg("--find-links")
+        .arg(context.workspace_root.join("scripts").join("links")), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    # This file was autogenerated by uv via the following command:
+    #    uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in
+    -e .
+    extras==0.0.1
+        # via example
+    iniconfig==2.0.0
+        # via extras
+
+    ----- stderr -----
+    Built 1 editable in [TIME]
+    Resolved 3 packages in [TIME]
+    "###
+    );
+
+    Ok(())
+}
+
 /// Use an existing resolution for `black==23.10.1`, with stale versions of `click` and `pathspec`.
 /// Nothing should change.
 #[test]