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

Add --dry-run flag to uv pip install #1436

Merged
merged 23 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ea54bee
feat(pip-install): add `--dry-run` flag
JacobCoffee Feb 16, 2024
9588802
feat(pip-install): properly print versions
JacobCoffee Feb 16, 2024
ff02255
test(pip-install): attempt to add (probably shitty) test
JacobCoffee Feb 16, 2024
5cd1473
feat(pip-install): move dry run into `install` function
JacobCoffee Feb 20, 2024
6892b17
feat(pip-install): add tests, match prod formatting, add DryRunEvent
JacobCoffee Feb 25, 2024
fee46b3
Fix duplicate `==` signs in display
zanieb Mar 4, 2024
7eb0f93
Append tests to `main`
zanieb Mar 4, 2024
44f5e8e
Merge branch 'main' into 1244-pip-install-dry-run-flag
zanieb Mar 4, 2024
0f5a423
Remove unncessary `to_string()`
zanieb Mar 4, 2024
d5ef665
Clean up test cases
zanieb Mar 4, 2024
1b4ca0b
Fix display of added URLs
zanieb Mar 6, 2024
861f805
Add uninstall URL dependency test case
zanieb Mar 6, 2024
91ad98b
Use `installed_version` display for uninstalls
zanieb Mar 6, 2024
3f75e84
Display message when no changes would be mad
zanieb Mar 6, 2024
33e723a
Include tests from `main`
zanieb Mar 6, 2024
a12d9b5
Merge branch 'main' into 1244-pip-install-dry-run-flag
zanieb Mar 6, 2024
22dcb9e
Merge branch 'main' into 1244-pip-install-dry-run-flag
zanieb Mar 6, 2024
415a4f4
Ignore lint
zanieb Mar 6, 2024
73c0984
Fix extraneous comma
zanieb Mar 6, 2024
85b06e9
Copy tests from `main`
zanieb Mar 7, 2024
89f723e
Merge branch 'main' into 1244-pip-install-dry-run-flag
zanieb Mar 7, 2024
4dd46df
Copy tests from `main`
zanieb Mar 12, 2024
e6c8442
Merge branch 'main' into 1244-pip-install-dry-run-flag
zanieb Mar 12, 2024
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
10 changes: 9 additions & 1 deletion crates/uv/src/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::process::ExitCode;
use std::time::Duration;
use std::{fmt::Display, process::ExitCode};

pub(crate) use cache_clean::cache_clean;
pub(crate) use cache_dir::cache_dir;
Expand All @@ -10,6 +10,7 @@ pub(crate) use pip_install::pip_install;
pub(crate) use pip_list::pip_list;
pub(crate) use pip_sync::pip_sync;
pub(crate) use pip_uninstall::pip_uninstall;
use uv_normalize::PackageName;
pub(crate) use venv::venv;
pub(crate) use version::version;

Expand Down Expand Up @@ -77,6 +78,13 @@ pub(super) struct ChangeEvent<T: InstalledMetadata> {
kind: ChangeEventKind,
}

#[derive(Debug)]
pub(super) struct DryRunEvent<T: Display> {
name: PackageName,
version: T,
kind: ChangeEventKind,
}

#[derive(Debug, Clone, Copy, clap::ValueEnum)]
pub(crate) enum VersionFormat {
Text,
Expand Down
158 changes: 148 additions & 10 deletions crates/uv/src/commands/pip_install.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::HashSet;
use std::fmt::Write;
use std::path::Path;
use std::time::Instant;

use anstream::eprint;
use anyhow::{anyhow, Context, Result};
Expand Down Expand Up @@ -39,7 +40,7 @@ use crate::commands::{elapsed, ChangeEvent, ChangeEventKind, ExitStatus};
use crate::printer::Printer;
use crate::requirements::{ExtrasSpecification, RequirementsSource, RequirementsSpecification};

use super::Upgrade;
use super::{DryRunEvent, Upgrade};

/// Install packages into the current environment.
#[allow(clippy::too_many_arguments)]
Expand All @@ -66,6 +67,7 @@ pub(crate) async fn pip_install(
system: bool,
cache: Cache,
mut printer: Printer,
dry_run: bool,
) -> Result<ExitStatus> {
let start = std::time::Instant::now();

Expand Down Expand Up @@ -301,6 +303,7 @@ pub(crate) async fn pip_install(
&cache,
&venv,
printer,
dry_run,
)
.await?;

Expand Down Expand Up @@ -363,7 +366,7 @@ async fn build_editables(
build_dispatch: &BuildDispatch<'_>,
mut printer: Printer,
) -> Result<Vec<BuiltEditable>, Error> {
let start = std::time::Instant::now();
let start = Instant::now();

let downloader = Downloader::new(cache, tags, client, build_dispatch)
.with_reporter(DownloadReporter::from(printer).with_length(editables.len() as u64));
Expand Down Expand Up @@ -514,6 +517,7 @@ async fn install(
cache: &Cache,
venv: &PythonEnvironment,
mut printer: Printer,
dry_run: bool,
) -> Result<(), Error> {
let start = std::time::Instant::now();

Expand All @@ -527,12 +531,7 @@ async fn install(

// Partition into those that should be linked from the cache (`local`), those that need to be
// downloaded (`remote`), and those that should be removed (`extraneous`).
let Plan {
local,
remote,
reinstalls,
extraneous: _,
} = Planner::with_requirements(&requirements)
let plan = Planner::with_requirements(&requirements)
.with_editable_requirements(&editables)
.build(
site_packages,
Expand All @@ -545,6 +544,17 @@ async fn install(
)
.context("Failed to determine installation plan")?;

if dry_run {
return report_dry_run(resolution, plan, start, printer);
}

let Plan {
local,
remote,
reinstalls,
extraneous: _,
} = plan;

// Nothing to do.
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() {
let s = if resolution.len() == 1 { "" } else { "s" };
Expand All @@ -558,7 +568,6 @@ async fn install(
)
.dimmed()
)?;

return Ok(());
}

Expand All @@ -577,7 +586,7 @@ async fn install(
let wheels = if remote.is_empty() {
vec![]
} else {
let start = std::time::Instant::now();
let start = Instant::now();

let downloader = Downloader::new(cache, tags, client, build_dispatch)
.with_reporter(DownloadReporter::from(printer).with_length(remote.len() as u64));
Expand Down Expand Up @@ -679,6 +688,135 @@ async fn install(
}
}

#[allow(clippy::items_after_statements)]
fn report_dry_run(
resolution: &Resolution,
plan: Plan,
start: Instant,
mut printer: Printer,
) -> Result<(), Error> {
let Plan {
local,
remote,
reinstalls,
extraneous: _,
} = plan;

// Nothing to do.
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() {
let s = if resolution.len() == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!(
"Audited {} in {}",
format!("{} package{}", resolution.len(), s).bold(),
elapsed(start.elapsed())
)
.dimmed()
)?;
writeln!(printer, "Would make no changes")?;
return Ok(());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is redundant as it is handled before this function is called


// Map any registry-based requirements back to those returned by the resolver.
let remote = remote
.iter()
.map(|dist| {
resolution
.get(&dist.name)
.cloned()
.expect("Resolution should contain all packages")
})
.collect::<Vec<_>>();

// Download, build, and unzip any missing distributions.
let wheels = if remote.is_empty() {
vec![]
} else {
let s = if remote.len() == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!(
"Would download {}",
format!("{} package{}", remote.len(), s).bold(),
)
.dimmed()
)?;
remote
};

// Remove any existing installations.
if !reinstalls.is_empty() {
let s = if reinstalls.len() == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!(
"Would uninstall {}",
format!("{} package{}", reinstalls.len(), s).bold(),
)
.dimmed()
)?;
}

// Install the resolved distributions.
let installs = wheels.len() + local.len();

if installs > 0 {
let s = if installs == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!("Would install {}", format!("{installs} package{s}").bold(),).dimmed()
zanieb marked this conversation as resolved.
Show resolved Hide resolved
)?;
}

for event in reinstalls
.into_iter()
.map(|distribution| DryRunEvent {
name: distribution.name().clone(),
version: format!("=={}", distribution.version()),
kind: ChangeEventKind::Removed,
})
.chain(wheels.into_iter().map(|distribution| DryRunEvent {
name: distribution.name().clone(),
version: format!("=={}", distribution.version().unwrap()),
kind: ChangeEventKind::Added,
}))
.chain(local.into_iter().map(|distribution| DryRunEvent {
name: distribution.name().clone(),
version: distribution.installed_version().to_string(),
kind: ChangeEventKind::Added,
}))
.sorted_unstable_by(|a, b| a.name.cmp(&b.name).then_with(|| a.kind.cmp(&b.kind)))
{
match event.kind {
ChangeEventKind::Added => {
writeln!(
printer,
" {} {}{}",
"+".green(),
event.name.as_ref().bold(),
event.version.dimmed()
)?;
}
ChangeEventKind::Removed => {
writeln!(
printer,
" {} {}{}",
"-".red(),
event.name.as_ref().bold(),
event.version.dimmed()
)?;
}
}
}

Ok(())
}

// 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 {
Expand Down
6 changes: 6 additions & 0 deletions crates/uv/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,11 @@ struct PipInstallArgs {
/// format (e.g., `2006-12-02`).
#[arg(long, value_parser = date_or_datetime)]
exclude_newer: Option<DateTime<Utc>>,

/// Perform a dry run, i.e., don't actually install anything but resolve the dependencies and
/// print the resulting plan.
#[clap(long)]
dry_run: bool,
}

#[derive(Args)]
Expand Down Expand Up @@ -1361,6 +1366,7 @@ async fn run() -> Result<ExitStatus> {
args.system,
cache,
printer,
args.dry_run,
)
.await
}
Expand Down
Loading
Loading