Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special-case reinstalls in environment update summaries #6243

Merged
merged 1 commit into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions crates/distribution-types/src/any.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
use std::hash::Hash;

use uv_normalize::PackageName;

use crate::cached::CachedDist;
use crate::installed::InstalledDist;
use crate::{InstalledMetadata, InstalledVersion, Name};

/// A distribution which is either installable, is a wheel in our cache or is already installed.
#[derive(Debug, Clone)]
///
/// Note equality and hash operations are only based on the name and version, not the kind.
#[derive(Debug, Clone, Eq)]
#[allow(clippy::large_enum_variant)]
pub enum LocalDist<'a> {
Cached(&'a CachedDist),
Installed(&'a InstalledDist),
pub enum LocalDist {
Cached(CachedDist),
Installed(InstalledDist),
}

impl Name for LocalDist<'_> {
impl Name for LocalDist {
fn name(&self) -> &PackageName {
match self {
Self::Cached(dist) => dist.name(),
Expand All @@ -21,7 +25,7 @@ impl Name for LocalDist<'_> {
}
}

impl InstalledMetadata for LocalDist<'_> {
impl InstalledMetadata for LocalDist {
fn installed_version(&self) -> InstalledVersion {
match self {
Self::Cached(dist) => dist.installed_version(),
Expand All @@ -30,14 +34,27 @@ impl InstalledMetadata for LocalDist<'_> {
}
}

impl<'a> From<&'a CachedDist> for LocalDist<'a> {
fn from(dist: &'a CachedDist) -> Self {
impl From<CachedDist> for LocalDist {
fn from(dist: CachedDist) -> Self {
Self::Cached(dist)
}
}

impl<'a> From<&'a InstalledDist> for LocalDist<'a> {
fn from(dist: &'a InstalledDist) -> Self {
impl From<InstalledDist> for LocalDist {
fn from(dist: InstalledDist) -> Self {
Self::Installed(dist)
}
}

impl Hash for LocalDist {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.name().hash(state);
self.installed_version().hash(state);
}
}

impl PartialEq for LocalDist {
fn eq(&self, other: &Self) -> bool {
self.name() == other.name() && self.installed_version() == other.installed_version()
}
}
6 changes: 3 additions & 3 deletions crates/distribution-types/src/cached.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@ use crate::{
};

/// A built distribution (wheel) that exists in the local cache.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub enum CachedDist {
/// The distribution exists in a registry, like `PyPI`.
Registry(CachedRegistryDist),
/// The distribution exists at an arbitrary URL.
Url(CachedDirectUrlDist),
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct CachedRegistryDist {
pub filename: WheelFilename,
pub path: PathBuf,
pub hashes: Vec<HashDigest>,
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct CachedDirectUrlDist {
pub filename: WheelFilename,
pub url: VerbatimUrl,
Expand Down
12 changes: 6 additions & 6 deletions crates/distribution-types/src/installed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use uv_normalize::PackageName;
use crate::{DistributionMetadata, InstalledMetadata, InstalledVersion, Name, VersionOrUrlRef};

/// A built distribution (wheel) that is installed in a virtual environment.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub enum InstalledDist {
/// The distribution was derived from a registry, like `PyPI`.
Registry(InstalledRegistryDist),
Expand All @@ -30,14 +30,14 @@ pub enum InstalledDist {
LegacyEditable(InstalledLegacyEditable),
}

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct InstalledRegistryDist {
pub name: PackageName,
pub version: Version,
pub path: PathBuf,
}

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct InstalledDirectUrlDist {
pub name: PackageName,
pub version: Version,
Expand All @@ -47,21 +47,21 @@ pub struct InstalledDirectUrlDist {
pub path: PathBuf,
}

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct InstalledEggInfoFile {
pub name: PackageName,
pub version: Version,
pub path: PathBuf,
}

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct InstalledEggInfoDirectory {
pub name: PackageName,
pub version: Version,
pub path: PathBuf,
}

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub struct InstalledLegacyEditable {
pub name: PackageName,
pub version: Version,
Expand Down
2 changes: 1 addition & 1 deletion crates/distribution-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl std::fmt::Display for VersionOrUrlRef<'_> {
}
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum InstalledVersion<'a> {
/// A PEP 440 version specifier, used to identify a distribution in a registry.
Version(&'a Version),
Expand Down
2 changes: 1 addition & 1 deletion crates/distribution-types/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<T: DistributionMetadata> Verbatim for T {
}

// Implement `Display` for all known types that implement `Metadata`.
impl std::fmt::Display for LocalDist<'_> {
impl std::fmt::Display for LocalDist {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.installed_version())
}
Expand Down
6 changes: 4 additions & 2 deletions crates/uv/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,13 @@ pub(super) enum ChangeEventKind {
Removed,
/// The package was added to the environment.
Added,
/// The package was reinstalled without changing versions.
Reinstalled,
}

#[derive(Debug)]
pub(super) struct ChangeEvent<T: InstalledMetadata> {
dist: T,
pub(super) struct ChangeEvent<'a, T: InstalledMetadata> {
dist: &'a T,
kind: ChangeEventKind,
}

Expand Down
24 changes: 21 additions & 3 deletions crates/uv/src/commands/pip/loggers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use itertools::Itertools;
use owo_colors::OwoColorize;
use rustc_hash::{FxBuildHasher, FxHashMap};

use distribution_types::{InstalledMetadata, LocalDist, Name};
use distribution_types::{InstalledMetadata, Name};
use pep440_rs::Version;
use uv_normalize::PackageName;

Expand Down Expand Up @@ -108,13 +108,22 @@ impl InstallLogger for DefaultInstallLogger {
.uninstalled
.iter()
.map(|distribution| ChangeEvent {
dist: LocalDist::from(distribution),
dist: distribution,
kind: ChangeEventKind::Removed,
})
.chain(changelog.installed.iter().map(|distribution| ChangeEvent {
dist: LocalDist::from(distribution),
dist: distribution,
kind: ChangeEventKind::Added,
}))
.chain(
changelog
.reinstalled
.iter()
.map(|distribution| ChangeEvent {
dist: distribution,
kind: ChangeEventKind::Reinstalled,
}),
)
.sorted_unstable_by(|a, b| {
a.dist
.name()
Expand Down Expand Up @@ -142,6 +151,15 @@ impl InstallLogger for DefaultInstallLogger {
event.dist.installed_version().dimmed()
)?;
}
ChangeEventKind::Reinstalled => {
writeln!(
printer.stderr(),
" {} {}{}",
"~".yellow(),
event.dist.name().bold(),
event.dist.installed_version().dimmed()
)?;
}
}
}
Ok(())
Expand Down
48 changes: 38 additions & 10 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
use anyhow::{anyhow, Context};
use itertools::Itertools;
use owo_colors::OwoColorize;
use std::collections::BTreeSet;
use std::collections::{BTreeSet, HashSet};
use std::fmt::Write;
use std::path::PathBuf;
use tracing::debug;

use distribution_types::{
CachedDist, Diagnostic, InstalledDist, ResolutionDiagnostic, UnresolvedRequirementSpecification,
CachedDist, Diagnostic, InstalledDist, LocalDist, ResolutionDiagnostic,
UnresolvedRequirementSpecification,
};
use distribution_types::{
DistributionMetadata, IndexLocations, InstalledMetadata, Name, Resolution,
Expand Down Expand Up @@ -289,17 +290,38 @@ pub(crate) enum Modifications {
#[derive(Debug, Clone, Default)]
pub(crate) struct Changelog {
/// The distributions that were installed.
pub(crate) installed: Vec<CachedDist>,
pub(crate) installed: HashSet<LocalDist>,
/// The distributions that were uninstalled.
pub(crate) uninstalled: Vec<InstalledDist>,
pub(crate) uninstalled: HashSet<LocalDist>,
/// The distributions that were reinstalled.
pub(crate) reinstalled: HashSet<LocalDist>,
}

impl Changelog {
/// Create a [`Changelog`] from a list of installed and uninstalled distributions.
pub(crate) fn new(installed: Vec<CachedDist>, uninstalled: Vec<InstalledDist>) -> Self {
let mut uninstalled: HashSet<_> = uninstalled.into_iter().map(LocalDist::from).collect();

let (reinstalled, installed): (HashSet<_>, HashSet<_>) = installed
.into_iter()
.map(LocalDist::from)
.partition(|dist| uninstalled.contains(dist));

uninstalled.retain(|dist| !reinstalled.contains(dist));

Self {
installed,
uninstalled,
reinstalled,
}
}

/// Create a [`Changelog`] from a list of installed distributions.
pub(crate) fn from_installed(installed: Vec<CachedDist>) -> Self {
Self {
installed,
uninstalled: vec![],
installed: installed.into_iter().map(LocalDist::from).collect(),
uninstalled: HashSet::default(),
reinstalled: HashSet::default(),
}
}

Expand Down Expand Up @@ -474,10 +496,7 @@ pub(crate) async fn install(
}

// Construct a summary of the changes made to the environment.
let changelog = Changelog {
installed: installs,
uninstalled: uninstalls,
};
let changelog = Changelog::new(installs, uninstalls);

// Notify the user of any environment modifications.
logger.on_complete(&changelog, printer)?;
Expand Down Expand Up @@ -619,6 +638,15 @@ fn report_dry_run(
event.version.dimmed()
)?;
}
ChangeEventKind::Reinstalled => {
writeln!(
printer.stderr(),
" {} {}{}",
"~".yellow(),
event.name.bold(),
event.version.dimmed()
)?;
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions crates/uv/tests/cache_prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,7 @@ fn prune_unzipped() -> Result<()> {
Uninstalled 2 packages in [TIME]
Installed 1 package in [TIME]
- iniconfig==2.0.0
- source-distribution==0.0.1
+ source-distribution==0.0.1
~ source-distribution==0.0.1
"###);

requirements_txt.write_str(indoc! { r"
Expand Down
10 changes: 5 additions & 5 deletions crates/uv/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -994,12 +994,12 @@ pub fn run_and_format_with_status<T: AsRef<str>>(
// cause the set of dependencies to be the same across platforms.
if cfg!(windows) {
if let Some(windows_filters) = windows_filters {
// The optional leading +/- is for install logs, the optional next line is for lockfiles
// The optional leading +/-/~ is for install logs, the optional next line is for lockfiles
let windows_only_deps = [
("( [+-] )?colorama==\\d+(\\.[\\d+])+( \\\\\n --hash=.*)?\n( # via .*\n)?"),
("( [+-] )?colorama==\\d+(\\.[\\d+])+(\\s+# via .*)?\n"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+( \\\\\n --hash=.*)?\n( # via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+(\\s+# via .*)?\n"),
("( [+-~] )?colorama==\\d+(\\.[\\d+])+( \\\\\n --hash=.*)?\n( # via .*\n)?"),
("( [+-~] )?colorama==\\d+(\\.[\\d+])+(\\s+# via .*)?\n"),
("( [+-~] )?tzdata==\\d+(\\.[\\d+])+( \\\\\n --hash=.*)?\n( # via .*\n)?"),
("( [+-~] )?tzdata==\\d+(\\.[\\d+])+(\\s+# via .*)?\n"),
];
let mut removed_packages = 0;
for windows_only_dep in windows_only_deps {
Expand Down
Loading
Loading