From eb1a630db2fe77305b78eb5ad8dd6be7a900647c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 13 Dec 2023 12:36:27 -0500 Subject: [PATCH] Avoid hard-error for non-existent extras (#627) ## Summary When resolving `transformers[tensorboard]`, the `[tensorboard]` extra doesn't exist. Previously, we returned "unknown" dependencies for this variant, which leads the resolution to try all versions, then fail. This PR instead warns, but returns the base dependencies for the package, which matches `pip`. (Poetry doesn't even warn, it just proceeds as normal.) Arguably, it would be better to return a custom incompatibility here and then propagate... But this PR is better than the status quo, and I don't know if we have support for that behavior yet...? (\cc @zanieb) Closes #386. Closes https://github.com/astral-sh/puffin/issues/423. --- crates/puffin-cli/src/commands/pip_compile.rs | 12 ++ crates/puffin-cli/src/commands/pip_install.rs | 13 +- crates/puffin-cli/tests/pip_compile.rs | 100 ++++++++++++ crates/puffin-resolver/src/finder.rs | 13 +- crates/puffin-resolver/src/lib.rs | 2 +- .../src/pubgrub/distribution.rs | 38 +++++ crates/puffin-resolver/src/pubgrub/mod.rs | 6 +- crates/puffin-resolver/src/resolution.rs | 151 ++++++++++++++---- crates/puffin-resolver/src/resolver.rs | 67 ++------ crates/puffin-resolver/tests/resolver.rs | 29 +++- .../resolver__black_tensorboard.snap | 16 ++ 11 files changed, 351 insertions(+), 96 deletions(-) create mode 100644 crates/puffin-resolver/src/pubgrub/distribution.rs create mode 100644 crates/puffin-resolver/tests/snapshots/resolver__black_tensorboard.snap diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index 6966ccbcf..5ed5031bd 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -178,6 +178,18 @@ pub(crate) async fn pip_compile( .dimmed() )?; + // Notify the user of any diagnostics. + for diagnostic in resolution.diagnostics() { + writeln!( + printer, + "{}{} {}", + "warning".yellow().bold(), + ":".bold(), + diagnostic.message().bold() + )?; + } + + // Write the resolved dependencies to the output channel. let mut writer: Box = if let Some(output_file) = output_file { Box::new(AutoStream::auto(fs::File::create(output_file)?)) } else { diff --git a/crates/puffin-cli/src/commands/pip_install.rs b/crates/puffin-cli/src/commands/pip_install.rs index 58d78c4f4..940d8f26c 100644 --- a/crates/puffin-cli/src/commands/pip_install.rs +++ b/crates/puffin-cli/src/commands/pip_install.rs @@ -18,7 +18,8 @@ use puffin_dispatch::BuildDispatch; use puffin_installer::{Downloader, InstallPlan, Reinstall, SitePackages}; use puffin_interpreter::Virtualenv; use puffin_resolver::{ - Graph, Manifest, PreReleaseMode, Resolution, ResolutionMode, ResolutionOptions, Resolver, + Manifest, PreReleaseMode, ResolutionGraph, ResolutionManifest, ResolutionMode, + ResolutionOptions, Resolver, }; use puffin_traits::OnceMap; use pypi_types::IndexUrls; @@ -180,7 +181,7 @@ async fn resolve( cache: &Cache, venv: &Virtualenv, mut printer: Printer, -) -> Result { +) -> Result { let start = std::time::Instant::now(); // Create a manifest of the requirements. @@ -271,7 +272,7 @@ async fn resolve( /// Install a set of requirements into the current environment. #[allow(clippy::too_many_arguments)] async fn install( - resolution: &Resolution, + resolution: &ResolutionManifest, reinstall: &Reinstall, link_mode: LinkMode, index_urls: IndexUrls, @@ -455,7 +456,11 @@ async fn install( } /// Validate the installed packages in the virtual environment. -fn validate(resolution: &Resolution, venv: &Virtualenv, mut printer: Printer) -> Result<()> { +fn validate( + resolution: &ResolutionManifest, + venv: &Virtualenv, + mut printer: Printer, +) -> Result<()> { let site_packages = SitePackages::from_executable(venv)?; let diagnostics = site_packages.diagnostics()?; for diagnostic in diagnostics { diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index f1ff99a3e..9eaaa62e2 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -1720,3 +1720,103 @@ fn override_multi_dependency() -> Result<()> { Ok(()) } + +/// Request an extra that doesn't exist on the specified package. +#[test] +fn missing_registry_extra() -> Result<()> { + let temp_dir = TempDir::new()?; + let cache_dir = TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.write_str("black[tensorboard]==23.10.1")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("requirements.in") + .arg("--cache-dir") + .arg(cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by Puffin v0.0.1 via the following command: + # puffin pip-compile requirements.in --cache-dir [CACHE_DIR] + black==23.10.1 + click==8.1.7 + # via black + mypy-extensions==1.0.0 + # via black + packaging==23.2 + # via black + pathspec==0.11.2 + # via black + platformdirs==4.0.0 + # via black + + ----- stderr ----- + Resolved 6 packages in [TIME] + warning: The package `black==23.10.1` does not have an extra named `tensorboard`. + "###); + }); + + Ok(()) +} + +/// Request an extra that doesn't exist on the specified package. +#[test] +fn missing_url_extra() -> Result<()> { + let temp_dir = TempDir::new()?; + let cache_dir = TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.write_str("flask[tensorboard] @ https://files.pythonhosted.org/packages/36/42/015c23096649b908c809c69388a805a571a3bea44362fe87e33fc3afa01f/flask-3.0.0-py3-none-any.whl")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("requirements.in") + .arg("--cache-dir") + .arg(cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by Puffin v0.0.1 via the following command: + # puffin pip-compile requirements.in --cache-dir [CACHE_DIR] + blinker==1.7.0 + # via flask + click==8.1.7 + # via flask + flask @ https://files.pythonhosted.org/packages/36/42/015c23096649b908c809c69388a805a571a3bea44362fe87e33fc3afa01f/flask-3.0.0-py3-none-any.whl + itsdangerous==2.1.2 + # via flask + jinja2==3.1.2 + # via flask + markupsafe==2.1.3 + # via + # jinja2 + # werkzeug + werkzeug==3.0.1 + # via flask + + ----- stderr ----- + Resolved 7 packages in [TIME] + warning: The package `flask @ https://files.pythonhosted.org/packages/36/42/015c23096649b908c809c69388a805a571a3bea44362fe87e33fc3afa01f/flask-3.0.0-py3-none-any.whl` does not have an extra named `tensorboard`. + "###); + }); + + Ok(()) +} diff --git a/crates/puffin-resolver/src/finder.rs b/crates/puffin-resolver/src/finder.rs index 2660f7221..8df87e360 100644 --- a/crates/puffin-resolver/src/finder.rs +++ b/crates/puffin-resolver/src/finder.rs @@ -18,7 +18,7 @@ use puffin_normalize::PackageName; use pypi_types::IndexUrl; use crate::error::ResolveError; -use crate::resolution::Resolution; +use crate::resolution::ResolutionManifest; pub struct DistFinder<'a> { tags: &'a Tags, @@ -48,9 +48,12 @@ impl<'a> DistFinder<'a> { } /// Resolve a set of pinned packages into a set of wheels. - pub async fn resolve(&self, requirements: &[Requirement]) -> Result { + pub async fn resolve( + &self, + requirements: &[Requirement], + ) -> Result { if requirements.is_empty() { - return Ok(Resolution::default()); + return Ok(ResolutionManifest::default()); } // A channel to fetch package metadata (e.g., given `flask`, fetch all versions). @@ -96,7 +99,7 @@ impl<'a> DistFinder<'a> { if let Some(reporter) = self.reporter.as_ref() { reporter.on_complete(); } - return Ok(Resolution::new(resolution)); + return Ok(ResolutionManifest::new(resolution)); } // Otherwise, wait for the package stream to complete. @@ -130,7 +133,7 @@ impl<'a> DistFinder<'a> { reporter.on_complete(); } - Ok(Resolution::new(resolution)) + Ok(ResolutionManifest::new(resolution)) } /// select a version that satisfies the requirement, preferring wheels to source distributions. diff --git a/crates/puffin-resolver/src/lib.rs b/crates/puffin-resolver/src/lib.rs index 8bd3096b1..b68e61a42 100644 --- a/crates/puffin-resolver/src/lib.rs +++ b/crates/puffin-resolver/src/lib.rs @@ -3,7 +3,7 @@ pub use finder::{DistFinder, Reporter as FinderReporter}; pub use manifest::Manifest; pub use prerelease_mode::PreReleaseMode; pub use pubgrub::PubGrubReportFormatter; -pub use resolution::{Graph, Resolution}; +pub use resolution::{ResolutionGraph, ResolutionManifest}; pub use resolution_mode::ResolutionMode; pub use resolution_options::ResolutionOptions; pub use resolver::{ diff --git a/crates/puffin-resolver/src/pubgrub/distribution.rs b/crates/puffin-resolver/src/pubgrub/distribution.rs new file mode 100644 index 000000000..3d2550e58 --- /dev/null +++ b/crates/puffin-resolver/src/pubgrub/distribution.rs @@ -0,0 +1,38 @@ +use url::Url; + +use distribution_types::{Metadata, VersionOrUrl}; +use puffin_normalize::PackageName; + +use crate::pubgrub::PubGrubVersion; + +#[derive(Debug)] +pub(crate) enum PubGrubDistribution<'a> { + Registry(&'a PackageName, &'a PubGrubVersion), + Url(&'a PackageName, &'a Url), +} + +impl<'a> PubGrubDistribution<'a> { + pub(crate) fn from_registry(name: &'a PackageName, version: &'a PubGrubVersion) -> Self { + Self::Registry(name, version) + } + + pub(crate) fn from_url(name: &'a PackageName, url: &'a Url) -> Self { + Self::Url(name, url) + } +} + +impl Metadata for PubGrubDistribution<'_> { + fn name(&self) -> &PackageName { + match self { + Self::Registry(name, _) => name, + Self::Url(name, _) => name, + } + } + + fn version_or_url(&self) -> VersionOrUrl { + match self { + Self::Registry(_, version) => VersionOrUrl::Version((*version).into()), + Self::Url(_, url) => VersionOrUrl::Url(url), + } + } +} diff --git a/crates/puffin-resolver/src/pubgrub/mod.rs b/crates/puffin-resolver/src/pubgrub/mod.rs index 2e58d5262..4d0ba0721 100644 --- a/crates/puffin-resolver/src/pubgrub/mod.rs +++ b/crates/puffin-resolver/src/pubgrub/mod.rs @@ -1,12 +1,12 @@ +pub(crate) use crate::pubgrub::dependencies::PubGrubDependencies; +pub(crate) use crate::pubgrub::distribution::PubGrubDistribution; pub(crate) use crate::pubgrub::package::PubGrubPackage; pub(crate) use crate::pubgrub::priority::{PubGrubPriorities, PubGrubPriority}; pub use crate::pubgrub::report::PubGrubReportFormatter; - pub(crate) use crate::pubgrub::version::{PubGrubVersion, MIN_VERSION}; -pub(crate) use crate::pubgrub::dependencies::PubGrubDependencies; - mod dependencies; +mod distribution; mod package; mod priority; mod report; diff --git a/crates/puffin-resolver/src/resolution.rs b/crates/puffin-resolver/src/resolution.rs index c2ceffd39..6c63970c6 100644 --- a/crates/puffin-resolver/src/resolution.rs +++ b/crates/puffin-resolver/src/resolution.rs @@ -14,18 +14,19 @@ use url::Url; use distribution_types::{BuiltDist, Dist, Metadata, SourceDist}; use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers}; use pep508_rs::{Requirement, VersionOrUrl}; -use puffin_normalize::PackageName; +use puffin_normalize::{ExtraName, PackageName}; use puffin_traits::OnceMap; +use pypi_types::Metadata21; use crate::pins::FilePins; -use crate::pubgrub::{PubGrubPackage, PubGrubPriority, PubGrubVersion}; +use crate::pubgrub::{PubGrubDistribution, PubGrubPackage, PubGrubPriority, PubGrubVersion}; use crate::ResolveError; /// A set of packages pinned at specific versions. #[derive(Debug, Default)] -pub struct Resolution(FxHashMap); +pub struct ResolutionManifest(FxHashMap); -impl Resolution { +impl ResolutionManifest { /// Create a new resolution from the given pinned packages. pub(crate) fn new(packages: FxHashMap) -> Self { Self(packages) @@ -69,19 +70,26 @@ impl Resolution { /// A complete resolution graph in which every node represents a pinned package and every edge /// represents a dependency between two pinned packages. #[derive(Debug)] -pub struct Graph(pub petgraph::graph::Graph, petgraph::Directed>); +pub struct ResolutionGraph { + /// The underlying graph. + petgraph: petgraph::graph::Graph, petgraph::Directed>, + /// Any diagnostics that were encountered while building the graph. + diagnostics: Vec, +} -impl Graph { +impl ResolutionGraph { /// Create a new graph from the resolved `PubGrub` state. pub(crate) fn from_state( selection: &SelectedDependencies, pins: &FilePins, + distributions: &OnceMap, redirects: &OnceMap, state: &State, PubGrubPriority>, ) -> Result { // TODO(charlie): petgraph is a really heavy and unnecessary dependency here. We should // write our own graph, given that our requirements are so simple. - let mut graph = petgraph::graph::Graph::with_capacity(selection.len(), selection.len()); + let mut petgraph = petgraph::graph::Graph::with_capacity(selection.len(), selection.len()); + let mut diagnostics = Vec::new(); // Add every package to the graph. let mut inverse = @@ -97,7 +105,7 @@ impl Graph { let pinned_package = Dist::from_registry(package_name.clone(), version, file, index); - let index = graph.add_node(pinned_package); + let index = petgraph.add_node(pinned_package); inverse.insert(package_name, index); } PubGrubPackage::Package(package_name, None, Some(url)) => { @@ -106,9 +114,52 @@ impl Graph { .map_or_else(|| url.clone(), |url| url.value().clone()); let pinned_package = Dist::from_url(package_name.clone(), url)?; - let index = graph.add_node(pinned_package); + let index = petgraph.add_node(pinned_package); inverse.insert(package_name, index); } + PubGrubPackage::Package(package_name, Some(extra), None) => { + // Validate that the `extra` exists. + let dist = PubGrubDistribution::from_registry(package_name, version); + let entry = distributions + .get(&dist.package_id()) + .expect("Every package should have metadata"); + let metadata = entry.value(); + + if !metadata.provides_extras.contains(extra) { + let version = Version::from(version.clone()); + let (index, file) = pins + .get(package_name, &version) + .expect("Every package should be pinned") + .clone(); + let pinned_package = + Dist::from_registry(package_name.clone(), version, file, index); + + diagnostics.push(Diagnostic::MissingExtra { + dist: pinned_package, + extra: extra.clone(), + }); + } + } + PubGrubPackage::Package(package_name, Some(extra), Some(url)) => { + // Validate that the `extra` exists. + let dist = PubGrubDistribution::from_url(package_name, url); + let entry = distributions + .get(&dist.package_id()) + .expect("Every package should have metadata"); + let metadata = entry.value(); + + if !metadata.provides_extras.contains(extra) { + let url = redirects + .get(url) + .map_or_else(|| url.clone(), |url| url.value().clone()); + let pinned_package = Dist::from_url(package_name.clone(), url)?; + + diagnostics.push(Diagnostic::MissingExtra { + dist: pinned_package, + extra: extra.clone(), + }); + } + } _ => {} }; } @@ -134,56 +185,68 @@ impl Graph { if self_version.contains(version) { let self_index = &inverse[self_package]; let dependency_index = &inverse[dependency_package]; - graph.update_edge(*self_index, *dependency_index, dependency_range.clone()); + petgraph.update_edge( + *self_index, + *dependency_index, + dependency_range.clone(), + ); } } } } - Ok(Self(graph)) + Ok(Self { + petgraph, + diagnostics, + }) } /// Return the number of packages in the graph. pub fn len(&self) -> usize { - self.0.node_count() + self.petgraph.node_count() } /// Return `true` if there are no packages in the graph. pub fn is_empty(&self) -> bool { - self.0.node_count() == 0 + self.petgraph.node_count() == 0 } /// Return the set of [`Requirement`]s that this graph represents. pub fn requirements(&self) -> Vec { // Collect and sort all packages. let mut nodes = self - .0 + .petgraph .node_indices() - .map(|node| (node, &self.0[node])) + .map(|node| (node, &self.petgraph[node])) .collect::>(); nodes.sort_unstable_by_key(|(_, package)| package.name()); - self.0 + self.petgraph .node_indices() - .map(|node| as_requirement(&self.0[node])) + .map(|node| as_requirement(&self.petgraph[node])) .collect() } + /// Return the [`Diagnostic`]s that were encountered while building the graph. + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics + } + /// Return the underlying graph. pub fn petgraph( &self, ) -> &petgraph::graph::Graph, petgraph::Directed> { - &self.0 + &self.petgraph } } /// Write the graph in the `{name}=={version}` format of requirements.txt that pip uses. -impl std::fmt::Display for Graph { +impl std::fmt::Display for ResolutionGraph { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Collect and sort all packages. let mut nodes = self - .0 + .petgraph .node_indices() - .map(|node| (node, &self.0[node])) + .map(|node| (node, &self.petgraph[node])) .collect::>(); nodes.sort_unstable_by_key(|(_, package)| package.name()); @@ -192,9 +255,9 @@ impl std::fmt::Display for Graph { writeln!(f, "{package}")?; let mut edges = self - .0 + .petgraph .edges_directed(index, Direction::Incoming) - .map(|edge| &self.0[edge.source()]) + .map(|edge| &self.petgraph[edge.source()]) .collect::>(); edges.sort_unstable_by_key(|package| package.name()); @@ -218,13 +281,18 @@ impl std::fmt::Display for Graph { } } -impl From for Resolution { - fn from(graph: Graph) -> Self { +impl From for ResolutionManifest { + fn from(graph: ResolutionGraph) -> Self { Self( graph - .0 + .petgraph .node_indices() - .map(|node| (graph.0[node].name().clone(), graph.0[node].clone())) + .map(|node| { + ( + graph.petgraph[node].name().clone(), + graph.petgraph[node].clone(), + ) + }) .collect(), ) } @@ -281,3 +349,32 @@ fn as_requirement(dist: &Dist) -> Requirement { }, } } + +#[derive(Debug)] +pub enum Diagnostic { + MissingExtra { + /// The distribution that was requested with an non-existent extra. For example, + /// `black==23.10.0`. + dist: Dist, + /// The extra that was requested. For example, `colorama` in `black[colorama]`. + extra: ExtraName, + }, +} + +impl Diagnostic { + /// Convert the diagnostic into a user-facing message. + pub fn message(&self) -> String { + match self { + Self::MissingExtra { dist, extra } => { + format!("The package `{dist}` does not have an extra named `{extra}`.") + } + } + } + + /// Returns `true` if the [`PackageName`] is involved in this diagnostic. + pub fn includes(&self, name: &PackageName) -> bool { + match self { + Self::MissingExtra { dist, .. } => name == dist.name(), + } + } +} diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index cdac1e712..47ed01302 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -34,9 +34,10 @@ use crate::manifest::Manifest; use crate::overrides::Overrides; use crate::pins::FilePins; use crate::pubgrub::{ - PubGrubDependencies, PubGrubPackage, PubGrubPriorities, PubGrubVersion, MIN_VERSION, + PubGrubDependencies, PubGrubDistribution, PubGrubPackage, PubGrubPriorities, PubGrubVersion, + MIN_VERSION, }; -use crate::resolution::Graph; +use crate::resolution::ResolutionGraph; use crate::version_map::VersionMap; use crate::yanks::AllowedYanks; use crate::ResolutionOptions; @@ -230,7 +231,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { } /// Resolve a set of requirements into a set of pinned versions. - pub async fn resolve(self) -> Result { + pub async fn resolve(self) -> Result { // A channel to fetch package metadata (e.g., given `flask`, fetch all versions) and version // metadata (e.g., given `flask==1.0.0`, fetch the metadata for that version). let (request_sink, request_stream) = futures::channel::mpsc::unbounded(); @@ -271,7 +272,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { async fn solve( &self, request_sink: &futures::channel::mpsc::UnboundedSender, - ) -> Result { + ) -> Result { let root = PubGrubPackage::Root(self.project.clone()); // Keep track of the packages for which we've requested metadata. @@ -301,7 +302,13 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { }) else { let selection = state.partial_solution.extract_solution(); - return Graph::from_state(&selection, &pins, &self.index.redirects, &state); + return ResolutionGraph::from_state( + &selection, + &pins, + &self.index.distributions, + &self.index.redirects, + &state, + ); }; next = highest_priority_pkg; @@ -350,13 +357,6 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { .get_dependencies(package, &version, &mut priorities, &index, request_sink) .await? { - Dependencies::Unknown => { - state.add_incompatibility(Incompatibility::unavailable_dependencies( - package.clone(), - version.clone(), - )); - continue; - } Dependencies::Unusable(reason) => { state.add_incompatibility(Incompatibility::unusable_dependencies( package.clone(), @@ -639,14 +639,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { Self::visit_package(package, priorities, index, request_sink)?; } - if let Some(extra) = extra { - if !metadata - .provides_extras - .iter() - .any(|provided_extra| provided_extra == extra) - { - return Ok(Dependencies::Unknown); - } + if extra.is_some() { constraints.insert( PubGrubPackage::Package(package_name.clone(), None, None), Range::singleton(version.clone()), @@ -854,42 +847,8 @@ impl<'a> FromIterator<&'a Url> for AllowedUrls { /// For each [Package] there is a set of versions allowed as a dependency. #[derive(Clone)] enum Dependencies { - /// Package dependencies are unavailable. - Unknown, /// Package dependencies are not usable Unusable(Option), /// Container for all available package versions. Known(DependencyConstraints>), } - -#[derive(Debug)] -enum PubGrubDistribution<'a> { - Registry(&'a PackageName, &'a PubGrubVersion), - Url(&'a PackageName, &'a Url), -} - -impl<'a> PubGrubDistribution<'a> { - fn from_registry(name: &'a PackageName, version: &'a PubGrubVersion) -> Self { - Self::Registry(name, version) - } - - fn from_url(name: &'a PackageName, url: &'a Url) -> Self { - Self::Url(name, url) - } -} - -impl Metadata for PubGrubDistribution<'_> { - fn name(&self) -> &PackageName { - match self { - Self::Registry(name, _) => name, - Self::Url(name, _) => name, - } - } - - fn version_or_url(&self) -> VersionOrUrl { - match self { - Self::Registry(_, version) => VersionOrUrl::Version((*version).into()), - Self::Url(_, url) => VersionOrUrl::Url(url), - } - } -} diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index fd830824a..e520af3a0 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -19,7 +19,7 @@ use puffin_cache::Cache; use puffin_client::RegistryClientBuilder; use puffin_interpreter::{Interpreter, Virtualenv}; use puffin_resolver::{ - Graph, Manifest, PreReleaseMode, ResolutionMode, ResolutionOptions, Resolver, + Manifest, PreReleaseMode, ResolutionGraph, ResolutionMode, ResolutionOptions, Resolver, }; use puffin_traits::{BuildContext, SourceBuildTrait}; @@ -97,7 +97,7 @@ async fn resolve( options: ResolutionOptions, markers: &'static MarkerEnvironment, tags: &Tags, -) -> Result { +) -> Result { let client = RegistryClientBuilder::new(Cache::temp()?).build(); let build_context = DummyContext { cache: Cache::temp()?, @@ -161,6 +161,31 @@ async fn black_colorama() -> Result<()> { Ok(()) } +/// Resolve Black with an invalid extra. The resolver should ignore the extra. +#[tokio::test] +async fn black_tensorboard() -> Result<()> { + colored::control::set_override(false); + + let manifest = Manifest::new( + vec![Requirement::from_str("black[tensorboard]<=23.9.1").unwrap()], + vec![], + vec![], + vec![], + None, + ); + let options = ResolutionOptions::new( + ResolutionMode::default(), + PreReleaseMode::default(), + Some(*EXCLUDE_NEWER), + ); + + let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?; + + insta::assert_display_snapshot!(resolution); + + Ok(()) +} + #[tokio::test] async fn black_python_310() -> Result<()> { colored::control::set_override(false); diff --git a/crates/puffin-resolver/tests/snapshots/resolver__black_tensorboard.snap b/crates/puffin-resolver/tests/snapshots/resolver__black_tensorboard.snap new file mode 100644 index 000000000..754dd7b39 --- /dev/null +++ b/crates/puffin-resolver/tests/snapshots/resolver__black_tensorboard.snap @@ -0,0 +1,16 @@ +--- +source: crates/puffin-resolver/tests/resolver.rs +expression: resolution +--- +black==23.9.1 +click==8.1.7 + # via black +mypy-extensions==1.0.0 + # via black +packaging==23.2 + # via black +pathspec==0.11.2 + # via black +platformdirs==4.0.0 + # via black +