Skip to content

Commit

Permalink
Special-case reinstalls in environment update summaries
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Aug 20, 2024
1 parent 6dc05a3 commit ee3ef40
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 167 deletions.
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

0 comments on commit ee3ef40

Please sign in to comment.