From 8b711d2e4de322c0b970d70d085a7fd4a68ec157 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 23 Apr 2024 22:00:27 -0400 Subject: [PATCH] Avoid adding extras when expanding constraints (#3232) ## Summary See the diff in the tests. If you have a constraint with an extra, we should respect it, but we shouldn't _add_ the extra to the requirements. --- .../uv-resolver/src/pubgrub/dependencies.rs | 34 +++++++------------ crates/uv/tests/pip_compile.rs | 4 +-- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/crates/uv-resolver/src/pubgrub/dependencies.rs b/crates/uv-resolver/src/pubgrub/dependencies.rs index 6920fb0c1..65d923fa3 100644 --- a/crates/uv-resolver/src/pubgrub/dependencies.rs +++ b/crates/uv-resolver/src/pubgrub/dependencies.rs @@ -50,10 +50,10 @@ impl PubGrubDependencies { .into_iter() .map(|extra| to_pubgrub(requirement, Some(extra), urls, locals)), ) { - let (mut package, version) = result?; + let (package, version) = result?; // Detect self-dependencies. - if let PubGrubPackage::Package(name, extra, ..) = &mut package { + 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() { @@ -76,29 +76,21 @@ impl PubGrubDependencies { continue; } - // Add the package, plus any extra variants. - for result in std::iter::once(to_pubgrub(constraint, None, urls, locals)).chain( - constraint - .extras - .clone() - .into_iter() - .map(|extra| to_pubgrub(constraint, Some(extra), urls, locals)), - ) { - let (mut package, version) = result?; + // Add the package. + let (package, version) = to_pubgrub(constraint, None, urls, locals)?; - // Detect self-dependencies. - if let PubGrubPackage::Package(name, extra, ..) = &mut 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; - } + // 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())); } + + dependencies.push((package.clone(), version.clone())); } } } diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index 228967c27..23b3ac153 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -323,13 +323,11 @@ fn compile_constraint_extra() -> Result<()> { # via # jinja2 # werkzeug - python-dotenv==1.0.1 - # via flask werkzeug==3.0.1 # via flask ----- stderr ----- - Resolved 8 packages in [TIME] + Resolved 7 packages in [TIME] "### );