From 6392961f44cf1502da13f6b53311863afb6de1b8 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 16 Feb 2024 18:48:35 -0500 Subject: [PATCH] Add support for extras in editable requirements (#1531) ## Summary If you're developing on a package like `attrs` locally, and it has a recursive extra like `attrs[dev]`, it turns out that we then try to find the `attrs` in `attrs[dev]` from the registry, rather than recognizing that it's part of the editable. This PR fixes the issue by making editables slightly more first-class throughout the resolver. Instead of mocking metadata, we explicitly check for extras in various places. Part of the problem here is that we treated editables as URL dependencies, but when we saw an _extra_ like `attrs[dev]`, we didn't map that back to the URL. So now, we treat them as registry dependencies, but with the appropriate guardrails throughout. Closes https://github.com/astral-sh/uv/issues/1447. ## Test Plan - Cloned `attrs`. - Ran `cargo run venv && cargo run pip install -e ".[dev]" -v`. --- crates/uv-resolver/src/resolution.rs | 54 +++++++++------ crates/uv-resolver/src/resolver/mod.rs | 65 ++++++++++++++----- crates/uv/tests/pip_compile.rs | 6 +- .../black_editable/pyproject.toml | 4 ++ 4 files changed, 92 insertions(+), 37 deletions(-) diff --git a/crates/uv-resolver/src/resolution.rs b/crates/uv-resolver/src/resolution.rs index 4d001d7477a1..f3705584d212 100644 --- a/crates/uv-resolver/src/resolution.rs +++ b/crates/uv-resolver/src/resolution.rs @@ -65,10 +65,13 @@ impl ResolutionGraph { match package { PubGrubPackage::Package(package_name, None, None) => { // Create the distribution. - let pinned_package = pins - .get(package_name, version) - .expect("Every package should be pinned") - .clone(); + let pinned_package = if let Some((editable, _)) = editables.get(package_name) { + Dist::from_editable(package_name.clone(), editable.clone())? + } else { + pins.get(package_name, version) + .expect("Every package should be pinned") + .clone() + }; // Add its hashes to the index. if let Some(versions_response) = packages.get(package_name) { @@ -87,9 +90,7 @@ impl ResolutionGraph { } PubGrubPackage::Package(package_name, None, Some(url)) => { // Create the distribution. - let pinned_package = if let Some((editable, _)) = editables.get(package_name) { - Dist::from_editable(package_name.clone(), editable.clone())? - } else { + let pinned_package = { let url = redirects.get(url).map_or_else( || url.clone(), |url| VerbatimUrl::unknown(url.value().clone()), @@ -115,20 +116,35 @@ impl ResolutionGraph { PubGrubPackage::Package(package_name, Some(extra), None) => { // Validate that the `extra` exists. let dist = PubGrubDistribution::from_registry(package_name, version); - let metadata = distributions - .get(&dist.package_id()) - .expect("Every package should have metadata"); - if !metadata.provides_extras.contains(extra) { - let pinned_package = pins - .get(package_name, version) - .expect("Every package should be pinned") - .clone(); + if let Some((_, metadata)) = editables.get(package_name) { + if !metadata.provides_extras.contains(extra) { + let pinned_package = pins + .get(package_name, version) + .expect("Every package should be pinned") + .clone(); - diagnostics.push(Diagnostic::MissingExtra { - dist: pinned_package, - extra: extra.clone(), - }); + diagnostics.push(Diagnostic::MissingExtra { + dist: pinned_package, + extra: extra.clone(), + }); + } + } else { + let metadata = distributions + .get(&dist.package_id()) + .expect("Every package should have metadata"); + + if !metadata.provides_extras.contains(extra) { + let pinned_package = pins + .get(package_name, version) + .expect("Every package should be pinned") + .clone(); + + diagnostics.push(Diagnostic::MissingExtra { + dist: pinned_package, + extra: extra.clone(), + }); + } } } PubGrubPackage::Package(package_name, Some(extra), Some(url)) => { diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 501ee001d9c5..1c97734836f0 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -160,16 +160,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // Determine all the editable requirements. let mut editables = FxHashMap::default(); for (editable_requirement, metadata) in &manifest.editables { - // Convert the editable requirement into a distribution. - let dist = Dist::from_editable(metadata.name.clone(), editable_requirement.clone()) - .expect("This is a valid distribution"); - - // Mock editable responses. - let package_id = dist.package_id(); - index.distributions.register(package_id.clone()); - index.distributions.done(package_id, metadata.clone()); editables.insert( - dist.name().clone(), + metadata.name.clone(), (editable_requirement.clone(), metadata.clone()), ); } @@ -633,6 +625,16 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { } PubGrubPackage::Package(package_name, extra, None) => { + // If the dist is an editable, return the version from the editable metadata. + if let Some((_local, metadata)) = self.editables.get(package_name) { + let version = metadata.version.clone(); + return if range.contains(&version) { + Ok(Some(ResolverVersion::Available(version))) + } else { + Ok(None) + }; + } + // Wait for the metadata to be available. let versions_response = self .index @@ -798,11 +800,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // Add a dependency on each editable. for (editable, metadata) in self.editables.values() { constraints.insert( - PubGrubPackage::Package( - metadata.name.clone(), - None, - Some(editable.url().clone()), - ), + PubGrubPackage::Package(metadata.name.clone(), None, None), Range::singleton(metadata.version.clone()), ); for extra in &editable.extras { @@ -810,7 +808,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { PubGrubPackage::Package( metadata.name.clone(), Some(extra.clone()), - Some(editable.url().clone()), + None, ), Range::singleton(metadata.version.clone()), ); @@ -830,7 +828,37 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { return Ok(Dependencies::Available(DependencyConstraints::default())); } - // Determine the distribution to lookup + // Determine if the distribution is editable. + if let Some((_local, metadata)) = self.editables.get(package_name) { + let mut constraints = PubGrubDependencies::from_requirements( + &metadata.requires_dist, + &self.constraints, + &self.overrides, + Some(package_name), + extra.as_ref(), + self.markers, + )?; + + for (package, version) in constraints.iter() { + debug!("Adding transitive dependency: {package}{version}"); + + // Emit a request to fetch the metadata for this package. + self.visit_package(package, priorities, request_sink) + .await?; + } + + // If a package has an extra, insert a constraint on the base package. + if extra.is_some() { + constraints.insert( + PubGrubPackage::Package(package_name.clone(), None, None), + Range::singleton(version.clone()), + ); + } + + return Ok(Dependencies::Available(constraints.into())); + } + + // Determine the distribution to lookup. let dist = match url { Some(url) => PubGrubDistribution::from_url(package_name, url), None => PubGrubDistribution::from_registry(package_name, version), @@ -984,6 +1012,11 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // Pre-fetch the package and distribution metadata. Request::Prefetch(package_name, range) => { + // Ignore editables. + if self.editables.contains_key(&package_name) { + return Ok(None); + } + // Wait for the package metadata to become available. let versions_response = self .index diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index cbd6585d1bf1..91be8214126e 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -2075,7 +2075,7 @@ fn compile_editable() -> Result<()> { requirements_in.write_str(indoc! {r" -e ../../scripts/editable-installs/poetry_editable -e ${PROJECT_ROOT}/../../scripts/editable-installs/maturin_editable - -e file://../../scripts/editable-installs/black_editable[d] + -e file://../../scripts/editable-installs/black_editable[dev] boltons # normal dependency for comparison " })?; @@ -2122,12 +2122,14 @@ fn compile_editable() -> Result<()> { # yarl numpy==1.26.2 # via poetry-editable + uvloop==0.19.0 + # via black yarl==1.9.2 # via aiohttp ----- stderr ----- Built 3 editables in [TIME] - Resolved 12 packages in [TIME] + Resolved 13 packages in [TIME] "###); Ok(()) diff --git a/scripts/editable-installs/black_editable/pyproject.toml b/scripts/editable-installs/black_editable/pyproject.toml index 1415b2dca02a..4f8bd991b624 100644 --- a/scripts/editable-installs/black_editable/pyproject.toml +++ b/scripts/editable-installs/black_editable/pyproject.toml @@ -20,6 +20,10 @@ jupyter = [ "ipython>=7.8.0", "tokenize-rt>=3.2.0", ] +dev = [ + "black[d]", + "black[uvloop]", +] [build-system] requires = ["flit_core>=3.4,<4"]