Report yanks for cached and resolved packages (#3772)

## Summary

We now show yanks as part of the resolution diagnostics, so they now
appear for `sync`, `install`, `compile`, and any other operations.
Further, they'll also appear for cached packages (but not packages that
are _already_ installed).

Closes https://github.com/astral-sh/uv/issues/3768.

Closes #3766.
This commit is contained in:
Charlie Marsh 2024-05-22 17:21:37 -04:00 committed by GitHub
parent 1fc71ea736
commit 74c494d7dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 153 additions and 117 deletions

1
Cargo.lock generated
View file

@ -4503,7 +4503,6 @@ dependencies = [
"pep508_rs",
"platform-tags",
"predicates",
"pypi-types",
"rayon",
"regex",
"requirements-txt",

View file

@ -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<PackageName, ResolvedDist>);
pub struct Resolution {
packages: BTreeMap<PackageName, ResolvedDist>,
diagnostics: Vec<Diagnostic>,
}
impl Resolution {
/// Create a new resolution from the given pinned packages.
pub fn new(packages: BTreeMap<PackageName, ResolvedDist>) -> Self {
Self(packages)
pub fn new(
packages: BTreeMap<PackageName, ResolvedDist>,
diagnostics: Vec<Diagnostic>,
) -> 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<Item = &PackageName> {
self.0.keys()
self.packages.keys()
}
/// Iterate over the [`ResolvedDist`] entities in this resolution.
pub fn distributions(&self) -> impl Iterator<Item = &ResolvedDist> {
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<Item = Requirement> + '_ {
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<Item = LocalEditable> + '_ {
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<String>,
},
}
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 {

View file

@ -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<'_> {

View file

@ -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,

View file

@ -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

View file

@ -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<ResolutionGraph> 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(),
}
}
}

View file

@ -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;

View file

@ -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"] }

View file

@ -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)
}

View file

@ -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)

View file

@ -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<InstalledPackages: InstalledPackagesProvider>(
.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)) => {
/// Report any diagnostics on resolved distributions.
pub(crate) fn diagnose_resolution(
diagnostics: &[Diagnostic],
printer: Printer,
) -> Result<(), Error> {
for diagnostic in diagnostics {
writeln!(
printer.stderr(),
"{}{} {dist} is yanked.",
"{}{} {}",
"warning".yellow().bold(),
":".bold(),
diagnostic.message().bold()
)?;
}
Some(Yanked::Reason(reason)) => {
writeln!(
printer.stderr(),
"{}{} {dist} is yanked (reason: \"{reason}\").",
"warning".yellow().bold(),
":".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,

View file

@ -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)

View file

@ -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)
}

View file

@ -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)
}

View file

@ -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.").
"###
);

View file

@ -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

View file

@ -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").
"###
);