From 505b99d9b67d2c7bd02c4776c11cf6bfedfc75ab Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 19 Feb 2024 21:56:21 -0500 Subject: [PATCH] Support recursive extras for URL dependencies (#1729) Closes https://github.com/astral-sh/uv/issues/1680. --- .../uv-resolver/src/pubgrub/dependencies.rs | 47 ++++++++++------ crates/uv-resolver/src/resolver/mod.rs | 35 ++++++------ crates/uv/tests/pip_compile.rs | 55 +++++++++++++++++++ 3 files changed, 103 insertions(+), 34 deletions(-) diff --git a/crates/uv-resolver/src/pubgrub/dependencies.rs b/crates/uv-resolver/src/pubgrub/dependencies.rs index 7bb6e083d..ecba33013 100644 --- a/crates/uv-resolver/src/pubgrub/dependencies.rs +++ b/crates/uv-resolver/src/pubgrub/dependencies.rs @@ -1,10 +1,10 @@ use itertools::Itertools; -use pep440_rs::Version; use pubgrub::range::Range; use pubgrub::type_aliases::DependencyConstraints; use tracing::warn; -use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl}; +use pep440_rs::Version; +use pep508_rs::{MarkerEnvironment, Requirement, VerbatimUrl, VersionOrUrl}; use uv_normalize::{ExtraName, PackageName}; use crate::overrides::Overrides; @@ -22,6 +22,7 @@ impl PubGrubDependencies { constraints: &[Requirement], overrides: &Overrides, source_name: Option<&PackageName>, + source_url: Option<&VerbatimUrl>, source_extra: Option<&ExtraName>, env: &MarkerEnvironment, ) -> Result { @@ -48,15 +49,20 @@ impl PubGrubDependencies { .into_iter() .map(|extra| to_pubgrub(requirement, Some(extra))), ) { - let (package, version) = result?; + let (mut package, version) = result?; - // Ignore self-dependencies. - if let PubGrubPackage::Package(name, extra, None) = &package { - if source_name.is_some_and(|source_name| source_name == name) - && source_extra == extra.as_ref() - { - warn!("{name} has a dependency on itself"); - continue; + // Detect self-dependencies. + if let PubGrubPackage::Package(name, extra, url) = &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; + } + // Propagate the source URL. + if source_url.is_some() { + *url = source_url.cloned(); + } } } @@ -103,15 +109,20 @@ impl PubGrubDependencies { .into_iter() .map(|extra| to_pubgrub(constraint, Some(extra))), ) { - let (package, version) = result?; + let (mut package, version) = result?; - // Ignore self-dependencies. - if let PubGrubPackage::Package(name, extra, None) = &package { - if source_name.is_some_and(|source_name| source_name == name) - && source_extra == extra.as_ref() - { - warn!("{name} has a dependency on itself"); - continue; + // Detect self-dependencies. + if let PubGrubPackage::Package(name, extra, url) = &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; + } + // Propagate the source URL. + if source_url.is_some() { + *url = source_url.cloned(); + } } } diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 3eaa5161c..d10e1cccb 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -13,12 +13,26 @@ use pubgrub::range::Range; use pubgrub::solver::{Incompatibility, State}; use pubgrub::type_aliases::DependencyConstraints; use rustc_hash::{FxHashMap, FxHashSet}; - use tokio::select; use tokio_stream::wrappers::ReceiverStream; use tracing::{debug, info_span, instrument, trace, warn, Instrument}; use url::Url; +use distribution_filename::WheelFilename; +use distribution_types::{ + BuiltDist, Dist, DistributionMetadata, IncompatibleWheel, LocalEditable, Name, RemoteSource, + SourceDist, VersionOrUrl, +}; +use pep440_rs::{Version, VersionSpecifiers, MIN_VERSION}; +use pep508_rs::{MarkerEnvironment, Requirement}; +use platform_tags::{IncompatibleTag, Tags}; +use pypi_types::{Metadata21, Yanked}; +use uv_client::{FlatIndex, RegistryClient}; +use uv_distribution::DistributionDatabase; +use uv_interpreter::Interpreter; +use uv_normalize::PackageName; +use uv_traits::BuildContext; + use crate::candidate_selector::{CandidateDist, CandidateSelector}; use crate::error::ResolveError; use crate::manifest::Manifest; @@ -39,20 +53,6 @@ use crate::resolver::reporter::Facade; pub use crate::resolver::reporter::{BuildId, Reporter}; use crate::yanks::AllowedYanks; use crate::{DependencyMode, Options}; -use distribution_filename::WheelFilename; -use distribution_types::{ - BuiltDist, Dist, DistributionMetadata, IncompatibleWheel, LocalEditable, Name, RemoteSource, - SourceDist, VersionOrUrl, -}; -use pep440_rs::{Version, VersionSpecifiers, MIN_VERSION}; -use pep508_rs::{MarkerEnvironment, Requirement}; -use platform_tags::{IncompatibleTag, Tags}; -use pypi_types::{Metadata21, Yanked}; -use uv_client::{FlatIndex, RegistryClient}; -use uv_distribution::DistributionDatabase; -use uv_interpreter::Interpreter; -use uv_normalize::PackageName; -use uv_traits::BuildContext; mod allowed_urls; mod index; @@ -784,6 +784,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { &self.overrides, None, None, + None, self.markers, ); if let Err( @@ -841,6 +842,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { &self.constraints, &self.overrides, Some(package_name), + url.as_ref(), extra.as_ref(), self.markers, )?; @@ -896,6 +898,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { &self.constraints, &self.overrides, Some(package_name), + url.as_ref(), extra.as_ref(), self.markers, )?; @@ -911,7 +914,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // 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), + PubGrubPackage::Package(package_name.clone(), None, url.clone()), Range::singleton(version.clone()), ); } diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index b3af6e014..03c44616f 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -2247,6 +2247,61 @@ fn compile_editable() -> Result<()> { Ok(()) } +#[test] +fn recursive_extras_direct_url() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("black[dev] @ ../../scripts/editable-installs/black_editable")?; + + let filter_path = regex::escape(&requirements_in.normalized_display().to_string()); + let filters: Vec<_> = [(filter_path.as_str(), "requirements.in")] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect(); + + uv_snapshot!(filters, Command::new(get_bin()) + .arg("pip") + .arg("compile") + .arg(requirements_in.path()) + .arg("--cache-dir") + .arg(context.cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", context.venv.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile requirements.in --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z + aiohttp==3.9.0 + # via black + aiosignal==1.3.1 + # via aiohttp + attrs==23.1.0 + # via aiohttp + black @ ../../scripts/editable-installs/black_editable + frozenlist==1.4.0 + # via + # aiohttp + # aiosignal + idna==3.4 + # via yarl + multidict==6.0.4 + # via + # aiohttp + # yarl + uvloop==0.19.0 + # via black + yarl==1.9.2 + # via aiohttp + + ----- stderr ----- + Resolved 9 packages in [TIME] + "###); + + Ok(()) +} + /// Compile an editable package with a direct URL requirement. #[test] fn compile_editable_url_requirement() -> Result<()> {