diff --git a/Cargo.lock b/Cargo.lock index 02be89306..3624f2daf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4503,7 +4503,6 @@ dependencies = [ "pep508_rs", "platform-tags", "predicates", - "pypi-types", "rayon", "regex", "requirements-txt", diff --git a/crates/distribution-types/src/resolution.rs b/crates/distribution-types/src/resolution.rs index c47339bbd..a590054e4 100644 --- a/crates/distribution-types/src/resolution.rs +++ b/crates/distribution-types/src/resolution.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use pep508_rs::VerbatimUrl; -use uv_normalize::PackageName; +use uv_normalize::{ExtraName, PackageName}; use crate::{ BuiltDist, DirectorySourceDist, Dist, InstalledDirectUrlDist, InstalledDist, LocalEditable, @@ -10,17 +10,26 @@ use crate::{ /// A set of packages pinned at specific versions. #[derive(Debug, Default, Clone)] -pub struct Resolution(BTreeMap); +pub struct Resolution { + packages: BTreeMap, + diagnostics: Vec, +} impl Resolution { /// Create a new resolution from the given pinned packages. - pub fn new(packages: BTreeMap) -> Self { - Self(packages) + pub fn new( + packages: BTreeMap, + diagnostics: Vec, + ) -> Self { + Self { + packages, + diagnostics, + } } /// Return the remote distribution for the given package name, if it exists. pub fn get_remote(&self, package_name: &PackageName) -> Option<&Dist> { - match self.0.get(package_name) { + match self.packages.get(package_name) { Some(dist) => match dist { ResolvedDist::Installable(dist) => Some(dist), ResolvedDist::Installed(_) => None, @@ -31,32 +40,37 @@ impl Resolution { /// Iterate over the [`PackageName`] entities in this resolution. pub fn packages(&self) -> impl Iterator { - self.0.keys() + self.packages.keys() } /// Iterate over the [`ResolvedDist`] entities in this resolution. pub fn distributions(&self) -> impl Iterator { - self.0.values() + self.packages.values() } /// Return the number of distributions in this resolution. pub fn len(&self) -> usize { - self.0.len() + self.packages.len() } /// Return `true` if there are no pinned packages in this resolution. pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.packages.is_empty() } /// Return the set of [`Requirement`]s that this resolution represents. pub fn requirements(&self) -> impl Iterator + '_ { - self.0.values().map(Requirement::from) + self.packages.values().map(Requirement::from) + } + + /// Return the [`Diagnostic`]s that were produced during resolution. + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics } /// Return an iterator over the [`LocalEditable`] entities in this resolution. pub fn editables(&self) -> impl Iterator + '_ { - self.0.values().filter_map(|dist| match dist { + self.packages.values().filter_map(|dist| match dist { ResolvedDist::Installable(Dist::Source(SourceDist::Directory( DirectorySourceDist { path, @@ -84,6 +98,49 @@ impl Resolution { } } +#[derive(Debug, Clone)] +pub enum Diagnostic { + MissingExtra { + /// The distribution that was requested with a non-existent extra. For example, + /// `black==23.10.0`. + dist: ResolvedDist, + /// The extra that was requested. For example, `colorama` in `black[colorama]`. + extra: ExtraName, + }, + YankedVersion { + /// The package that was requested with a yanked version. For example, `black==23.10.0`. + dist: ResolvedDist, + /// The reason that the version was yanked, if any. + reason: Option, + }, +} + +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}`.") + } + Self::YankedVersion { dist, reason } => { + if let Some(reason) = reason { + format!("`{dist}` is yanked (reason: \"{reason}\").") + } else { + format!("`{dist}` is yanked.") + } + } + } + } + + /// 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(), + Self::YankedVersion { dist, .. } => name == dist.name(), + } + } +} + impl From<&ResolvedDist> for Requirement { fn from(resolved_dist: &ResolvedDist) -> Self { let source = match resolved_dist { diff --git a/crates/distribution-types/src/resolved.rs b/crates/distribution-types/src/resolved.rs index b4d3af7a0..288addf42 100644 --- a/crates/distribution-types/src/resolved.rs +++ b/crates/distribution-types/src/resolved.rs @@ -1,6 +1,7 @@ use std::fmt::{Display, Formatter}; use pep508_rs::PackageName; +use pypi_types::Yanked; use crate::{ BuiltDist, Dist, DistributionId, DistributionMetadata, Identifier, IndexUrl, InstalledDist, @@ -51,6 +52,18 @@ impl ResolvedDist { Self::Installed(_) => None, } } + + /// Returns the [`Yanked`] status of the distribution, if available. + pub fn yanked(&self) -> Option<&Yanked> { + match self { + Self::Installable(dist) => match dist { + Dist::Source(SourceDist::Registry(sdist)) => sdist.file.yanked.as_ref(), + Dist::Built(BuiltDist::Registry(wheel)) => wheel.best_wheel().file.yanked.as_ref(), + _ => None, + }, + Self::Installed(_) => None, + } + } } impl ResolvedDistRef<'_> { diff --git a/crates/uv-resolver/src/lib.rs b/crates/uv-resolver/src/lib.rs index bab5812a5..8418cd2a0 100644 --- a/crates/uv-resolver/src/lib.rs +++ b/crates/uv-resolver/src/lib.rs @@ -10,7 +10,7 @@ pub use options::{Options, OptionsBuilder}; pub use preferences::{Preference, PreferenceError}; pub use prerelease_mode::PreReleaseMode; pub use python_requirement::PythonRequirement; -pub use resolution::{AnnotationStyle, Diagnostic, DisplayResolutionGraph, ResolutionGraph}; +pub use resolution::{AnnotationStyle, DisplayResolutionGraph, ResolutionGraph}; pub use resolution_mode::ResolutionMode; pub use resolver::{ BuildId, DefaultResolverProvider, InMemoryIndex, MetadataResponse, PackageVersionsResult, diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index 8c7063499..2f80e0b41 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -81,7 +81,8 @@ impl Lock { let resolved_dist = ResolvedDist::Installable(dist.to_dist(marker_env, tags)); map.insert(name, resolved_dist); } - Resolution::new(map) + let diagnostics = vec![]; + Resolution::new(map, diagnostics) } /// Returns the distribution with the given name. If there are multiple diff --git a/crates/uv-resolver/src/resolution/graph.rs b/crates/uv-resolver/src/resolution/graph.rs index 5e240c997..ce7d7c0d7 100644 --- a/crates/uv-resolver/src/resolution/graph.rs +++ b/crates/uv-resolver/src/resolution/graph.rs @@ -7,12 +7,13 @@ use pubgrub::type_aliases::SelectedDependencies; use rustc_hash::{FxHashMap, FxHashSet}; use distribution_types::{ - Dist, DistributionMetadata, Name, ParsedUrlError, Requirement, ResolvedDist, VersionId, - VersionOrUrlRef, + Diagnostic, Dist, DistributionMetadata, Name, ParsedUrlError, Requirement, ResolvedDist, + VersionId, VersionOrUrlRef, }; use pep440_rs::{Version, VersionSpecifier}; use pep508_rs::MarkerEnvironment; -use uv_normalize::{ExtraName, PackageName}; +use pypi_types::Yanked; +use uv_normalize::PackageName; use crate::dependency_provider::UvDependencyProvider; use crate::editables::Editables; @@ -172,6 +173,23 @@ impl ResolutionGraph { .expect("Every package should be pinned") .clone(); + // Track yanks for any registry distributions. + match dist.yanked() { + None | Some(Yanked::Bool(false)) => {} + Some(Yanked::Bool(true)) => { + diagnostics.push(Diagnostic::YankedVersion { + dist: dist.clone(), + reason: None, + }); + } + Some(Yanked::Reason(reason)) => { + diagnostics.push(Diagnostic::YankedVersion { + dist: dist.clone(), + reason: Some(reason.clone()), + }); + } + } + // Extract the hashes, preserving those that were already present in the // lockfile if necessary. let hashes = if let Some(digests) = preferences @@ -573,35 +591,7 @@ impl From for distribution_types::Resolution { ) }) .collect(), + graph.diagnostics, ) } } - -#[derive(Debug)] -pub enum Diagnostic { - MissingExtra { - /// The distribution that was requested with an non-existent extra. For example, - /// `black==23.10.0`. - dist: ResolvedDist, - /// 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/uv-resolver/src/resolution/mod.rs b/crates/uv-resolver/src/resolution/mod.rs index ebefefceb..ae12508f0 100644 --- a/crates/uv-resolver/src/resolution/mod.rs +++ b/crates/uv-resolver/src/resolution/mod.rs @@ -10,7 +10,7 @@ use pypi_types::{HashDigest, Metadata23}; use uv_normalize::{ExtraName, PackageName}; pub use crate::resolution::display::{AnnotationStyle, DisplayResolutionGraph}; -pub use crate::resolution::graph::{Diagnostic, ResolutionGraph}; +pub use crate::resolution::graph::ResolutionGraph; mod display; mod graph; diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index 6de38b0ff..e9d5b33a6 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -19,7 +19,6 @@ install-wheel-rs = { workspace = true, features = ["clap"], default-features = f pep440_rs = { workspace = true } pep508_rs = { workspace = true } platform-tags = { workspace = true } -pypi-types = { workspace = true } requirements-txt = { workspace = true, features = ["http"] } uv-auth = { workspace = true } uv-cache = { workspace = true, features = ["clap"] } diff --git a/crates/uv/src/commands/pip/compile.rs b/crates/uv/src/commands/pip/compile.rs index 6768c25b8..918e263dc 100644 --- a/crates/uv/src/commands/pip/compile.rs +++ b/crates/uv/src/commands/pip/compile.rs @@ -53,6 +53,7 @@ use uv_resolver::{ use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight}; use uv_warnings::warn_user; +use crate::commands::pip::operations; use crate::commands::reporters::{DownloadReporter, ResolverReporter}; use crate::commands::{elapsed, ExitStatus}; use crate::printer::Printer; @@ -589,17 +590,6 @@ pub(crate) async fn pip_compile( .dimmed() )?; - // Notify the user of any diagnostics. - for diagnostic in resolution.diagnostics() { - writeln!( - printer.stderr(), - "{}{} {}", - "warning".yellow().bold(), - ":".bold(), - diagnostic.message().bold() - )?; - } - // Write the resolved dependencies to the output channel. let mut writer = OutputWriter::new(!quiet || output_file.is_none(), output_file)?; @@ -700,6 +690,9 @@ pub(crate) async fn pip_compile( } } + // Notify the user of any resolution diagnostics. + operations::diagnose_resolution(resolution.diagnostics(), printer)?; + Ok(ExitStatus::Success) } diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs index 2746b8e12..ff0ae26a3 100644 --- a/crates/uv/src/commands/pip/install.rs +++ b/crates/uv/src/commands/pip/install.rs @@ -444,9 +444,12 @@ pub(crate) async fn pip_install( ) .await?; - // Validate the environment. - if strict { - operations::report_diagnostics(&resolution, &venv, printer)?; + // Notify the user of any resolution diagnostics. + operations::diagnose_resolution(resolution.diagnostics(), printer)?; + + // Notify the user of any environment diagnostics. + if strict && !dry_run { + operations::diagnose_environment(&resolution, &venv, printer)?; } Ok(ExitStatus::Success) diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index d3bf702b5..b0cf0caba 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -9,7 +9,7 @@ use owo_colors::OwoColorize; use tracing::debug; use distribution_types::{ - CachedDist, Dist, InstalledDist, Requirement, UnresolvedRequirementSpecification, + CachedDist, Diagnostic, InstalledDist, Requirement, UnresolvedRequirementSpecification, }; use distribution_types::{ DistributionMetadata, IndexLocations, InstalledMetadata, InstalledVersion, LocalDist, Name, @@ -19,7 +19,6 @@ use install_wheel_rs::linker::LinkMode; use pep440_rs::{VersionSpecifier, VersionSpecifiers}; use pep508_rs::{MarkerEnvironment, VerbatimUrl}; use platform_tags::Tags; -use pypi_types::Yanked; use uv_cache::Cache; use uv_client::{BaseClientBuilder, RegistryClient}; use uv_configuration::{ @@ -286,17 +285,6 @@ pub(crate) async fn resolve( .dimmed() )?; - // Notify the user of any diagnostics. - for diagnostic in resolution.diagnostics() { - writeln!( - printer.stderr(), - "{}{} {}", - "warning".yellow().bold(), - ":".bold(), - diagnostic.message().bold() - )?; - } - Ok(resolution) } @@ -521,10 +509,9 @@ pub(crate) async fn install( compile_bytecode(venv, cache, printer).await?; } + // Notify the user of any environment modifications. report_modifications(wheels, reinstalls, extraneous, printer)?; - report_yanks(&remote, printer)?; - Ok(()) } @@ -666,8 +653,6 @@ fn report_dry_run( } } - report_yanks(&remote, printer)?; - Ok(()) } @@ -721,39 +706,25 @@ pub(crate) fn report_modifications( Ok(()) } -/// Report on any yanked distributions in the resolution. -pub(crate) fn report_yanks(remote: &[Dist], printer: Printer) -> Result<(), Error> { - // TODO(konstin): Also check the cache whether any cached or installed dist is already known to - // have been yanked, we currently don't show this message on the second run anymore - for dist in remote { - let Some(file) = dist.file() else { - continue; - }; - match &file.yanked { - None | Some(Yanked::Bool(false)) => {} - Some(Yanked::Bool(true)) => { - writeln!( - printer.stderr(), - "{}{} {dist} is yanked.", - "warning".yellow().bold(), - ":".bold(), - )?; - } - Some(Yanked::Reason(reason)) => { - writeln!( - printer.stderr(), - "{}{} {dist} is yanked (reason: \"{reason}\").", - "warning".yellow().bold(), - ":".bold(), - )?; - } - } +/// Report any diagnostics on resolved distributions. +pub(crate) fn diagnose_resolution( + diagnostics: &[Diagnostic], + printer: Printer, +) -> Result<(), Error> { + for diagnostic in diagnostics { + writeln!( + printer.stderr(), + "{}{} {}", + "warning".yellow().bold(), + ":".bold(), + diagnostic.message().bold() + )?; } Ok(()) } /// Report any diagnostics on installed distributions in the Python environment. -pub(crate) fn report_diagnostics( +pub(crate) fn diagnose_environment( resolution: &Resolution, venv: &PythonEnvironment, printer: Printer, diff --git a/crates/uv/src/commands/pip/sync.rs b/crates/uv/src/commands/pip/sync.rs index 3f19d6c9e..5906c7cda 100644 --- a/crates/uv/src/commands/pip/sync.rs +++ b/crates/uv/src/commands/pip/sync.rs @@ -389,9 +389,12 @@ pub(crate) async fn pip_sync( ) .await?; - // Validate the environment. - if strict { - operations::report_diagnostics(&resolution, &venv, printer)?; + // Notify the user of any resolution diagnostics. + operations::diagnose_resolution(resolution.diagnostics(), printer)?; + + // Notify the user of any environment diagnostics. + if strict && !dry_run { + operations::diagnose_environment(&resolution, &venv, printer)?; } Ok(ExitStatus::Success) diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index a32e59837..ba9eaf290 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -5,6 +5,7 @@ use itertools::Itertools; use owo_colors::OwoColorize; use tracing::debug; +use crate::commands::pip; use distribution_types::{IndexLocations, Resolution}; use install_wheel_rs::linker::LinkMode; use uv_cache::Cache; @@ -23,7 +24,6 @@ use uv_requirements::{ use uv_resolver::{FlatIndex, InMemoryIndex, Options}; use uv_types::{BuildIsolation, HashStrategy, InFlight}; -use crate::commands::pip; use crate::editables::ResolvedEditables; use crate::printer::Printer; @@ -292,5 +292,8 @@ pub(crate) async fn update_environment( ) .await?; + // Notify the user of any resolution diagnostics. + pip::operations::diagnose_resolution(resolution.diagnostics(), printer)?; + Ok(venv) } diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 3e3c6a0c9..1834aa3d2 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -132,5 +132,8 @@ pub(crate) async fn sync( ) .await?; + // Notify the user of any resolution diagnostics. + pip::operations::diagnose_resolution(resolution.diagnostics(), printer)?; + Ok(ExitStatus::Success) } diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index 5a009d8eb..d0367d14c 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -2805,6 +2805,7 @@ fn compile_yanked_version_direct() -> Result<()> { ----- stderr ----- Resolved 1 package in [TIME] + warning: `attrs==21.1.0` is yanked (reason: "Installable but not importable on Python 3.4."). "### ); diff --git a/crates/uv/tests/pip_install_scenarios.rs b/crates/uv/tests/pip_install_scenarios.rs index edd36a1e0..7f1c8cb4a 100644 --- a/crates/uv/tests/pip_install_scenarios.rs +++ b/crates/uv/tests/pip_install_scenarios.rs @@ -708,10 +708,10 @@ fn missing_extra() { ----- stderr ----- Resolved 1 package in [TIME] - warning: The package `package-a==1.0.0` does not have an extra named `extra`. Downloaded 1 package in [TIME] Installed 1 package in [TIME] + package-a==1.0.0 + warning: The package `package-a==1.0.0` does not have an extra named `extra`. "###); // Missing extras are ignored during resolution. @@ -1081,10 +1081,10 @@ fn extra_does_not_exist_backtrack() { ----- stderr ----- Resolved 1 package in [TIME] - warning: The package `package-a==3.0.0` does not have an extra named `extra`. Downloaded 1 package in [TIME] Installed 1 package in [TIME] + package-a==3.0.0 + warning: The package `package-a==3.0.0` does not have an extra named `extra`. "###); // The resolver should not backtrack to `a==1.0.0` because missing extras are @@ -4779,7 +4779,7 @@ fn transitive_package_only_yanked_in_range_opt_in() { Installed 2 packages in [TIME] + package-a==0.1.0 + package-b==1.0.0 - warning: package-b==1.0.0 is yanked (reason: "Yanked for testing"). + warning: `package-b==1.0.0` is yanked (reason: "Yanked for testing"). "###); // Since the user included a dependency on `b` with an exact specifier, the yanked @@ -4911,7 +4911,7 @@ fn transitive_yanked_and_unyanked_dependency_opt_in() { + package-a==1.0.0 + package-b==1.0.0 + package-c==2.0.0 - warning: package-c==2.0.0 is yanked (reason: "Yanked for testing"). + warning: `package-c==2.0.0` is yanked (reason: "Yanked for testing"). "###); // Since the user explicitly selected the yanked version of `c`, it can be diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 75ea2e3aa..b0d6ac3ca 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -921,7 +921,7 @@ fn warn_on_yanked() -> Result<()> { Downloaded 1 package in [TIME] Installed 1 package in [TIME] + colorama==0.4.2 - warning: colorama==0.4.2 is yanked (reason: "Bad build, missing files, will not install"). + warning: `colorama==0.4.2` is yanked (reason: "Bad build, missing files, will not install"). "### ); @@ -949,7 +949,7 @@ fn warn_on_yanked_dry_run() -> Result<()> { Would download 1 package Would install 1 package + colorama==0.4.2 - warning: colorama==0.4.2 is yanked (reason: "Bad build, missing files, will not install"). + warning: `colorama==0.4.2` is yanked (reason: "Bad build, missing files, will not install"). "### );