Skip to content

Refactor pip::operations::install #16985

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.

Sign up for GitHub

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

Closed
EliteTK wants to merge 1 commit into from
Closed

Refactor pip::operations::install #16985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 24 additions & 2 deletions crates/uv/src/commands/pip/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,14 +639,35 @@ pub(crate) async fn pip_install(
preview,
);

// Sync the environment.
match operations::install(
let start = std::time::Instant::now();

// Make a plan
let plan = match operations::plan(
&resolution,
site_packages,
InstallationStrategy::Permissive,
modifications,
&reinstall,
&build_options,
&hasher,
&tags,
&build_dispatch,
&cache,
&environment,
) {
Ok(plan) => plan,
Err(err) => {
return diagnostics::OperationDiagnostic::native_tls(client_builder.is_native_tls())
.report(err)
.map_or(Ok(ExitStatus::Failure), |err| Err(err.into()));
}
};

// Sync the environment.
match operations::install(
&resolution,
plan,
&build_options,
link_mode,
compile,
&hasher,
Expand All @@ -662,6 +683,7 @@ pub(crate) async fn pip_install(
dry_run,
printer,
preview,
start,
)
.await
{
Expand Down
107 changes: 52 additions & 55 deletions crates/uv/src/commands/pip/operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use uv_tool::InstalledTools;
use uv_types::{BuildContext, HashStrategy, InFlight, InstalledPackagesProvider};
use uv_warnings::warn_user;

use crate::commands::pip::loggers::{DefaultInstallLogger, InstallLogger, ResolveLogger};
use crate::commands::pip::loggers::{InstallLogger, ResolveLogger};
use crate::commands::reporters::{InstallReporter, PrepareReporter, ResolverReporter};
use crate::commands::{ChangeEventKind, DryRunEvent, compile_bytecode};
use crate::printer::Printer;
Expand Down Expand Up @@ -436,34 +436,21 @@ impl Changelog {
}
}

/// Install a set of requirements into the current environment.
///
/// Returns a [`Changelog`] summarizing the changes made to the environment.
pub(crate) async fn install(
/// Produce a [`Plan`] for how to approach installing a set of packages
#[allow(clippy::result_large_err)]
pub(crate) fn plan(
resolution: &Resolution,
site_packages: SitePackages,
installation: InstallationStrategy,
modifications: Modifications,
reinstall: &Reinstall,
build_options: &BuildOptions,
link_mode: LinkMode,
compile: bool,
hasher: &HashStrategy,
tags: &Tags,
client: &RegistryClient,
in_flight: &InFlight,
concurrency: Concurrency,
build_dispatch: &BuildDispatch<'_>,
cache: &Cache,
venv: &PythonEnvironment,
logger: Box<dyn InstallLogger>,
installer_metadata: bool,
dry_run: DryRun,
printer: Printer,
preview: Preview,
) -> Result<Changelog, Error> {
let start = std::time::Instant::now();

) -> Result<Plan, Error> {
// 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 = Planner::new(resolution)
Expand All @@ -484,11 +471,6 @@ pub(crate) async fn install(
)
.context("Failed to determine installation plan")?;

if dry_run.enabled() {
report_dry_run(dry_run, resolution, plan, modifications, start, printer)?;
return Ok(Changelog::default());
}

let Plan {
cached,
remote,
Expand All @@ -502,27 +484,62 @@ pub(crate) async fn install(
Modifications::Exact => extraneous,
};

Ok(Plan {
cached,
remote,
reinstalls,
extraneous,
})
}

/// Install a set of requirements into the current environment.
///
/// Returns a [`Changelog`] summarizing the changes made to the environment.
pub(crate) async fn install(
resolution: &Resolution,
plan: Plan,
build_options: &BuildOptions,
link_mode: LinkMode,
compile: bool,
hasher: &HashStrategy,
tags: &Tags,
client: &RegistryClient,
in_flight: &InFlight,
concurrency: Concurrency,
build_dispatch: &BuildDispatch<'_>,
cache: &Cache,
venv: &PythonEnvironment,
logger: Box<dyn InstallLogger>,
installer_metadata: bool,
dry_run: DryRun,
printer: Printer,
preview: Preview,
start: std::time::Instant,
) -> Result<Changelog, Error> {
// Nothing to do.
if remote.is_empty()
&& cached.is_empty()
&& reinstalls.is_empty()
&& extraneous.is_empty()
if plan.remote.is_empty()
&& plan.cached.is_empty()
&& plan.reinstalls.is_empty()
&& plan.extraneous.is_empty()
&& !compile
{
logger.on_audit(resolution.len(), start, printer)?;
if dry_run.enabled() {
writeln!(printer.stderr(), "Would make no changes")?;
}
return Ok(Changelog::default());
}

if dry_run.enabled() {
report_dry_run(dry_run, plan, printer)?;
return Ok(Changelog::default());
}

// Partition into two sets: those that require build isolation, and those that disable it. This
// is effectively a heuristic to make `--no-build-isolation` work "more often" by way of giving
// `--no-build-isolation` packages "access" to the rest of the environment.
let (isolated_phase, shared_phase) = Plan {
cached,
remote,
reinstalls,
extraneous,
}
.partition(|name| build_dispatch.build_isolation().is_isolated(Some(name)));
let (isolated_phase, shared_phase) =
plan.partition(|name| build_dispatch.build_isolation().is_isolated(Some(name)));

let has_isolated_phase = !isolated_phase.is_empty();
let has_shared_phase = !shared_phase.is_empty();
Expand Down Expand Up @@ -834,34 +851,14 @@ pub(crate) fn report_target_environment(

/// Report on the results of a dry-run installation.
#[allow(clippy::result_large_err)]
fn report_dry_run(
dry_run: DryRun,
resolution: &Resolution,
plan: Plan,
modifications: Modifications,
start: std::time::Instant,
printer: Printer,
) -> Result<(), Error> {
fn report_dry_run(dry_run: DryRun, plan: Plan, printer: Printer) -> Result<(), Error> {
let Plan {
cached,
remote,
reinstalls,
extraneous,
} = plan;

// If we're in `install` mode, ignore any extraneous distributions.
let extraneous = match modifications {
Modifications::Sufficient => vec![],
Modifications::Exact => extraneous,
};

// Nothing to do.
if remote.is_empty() && cached.is_empty() && reinstalls.is_empty() && extraneous.is_empty() {
DefaultInstallLogger.on_audit(resolution.len(), start, printer)?;
writeln!(printer.stderr(), "Would make no changes")?;
return Ok(());
}

// Download, build, and unzip any missing distributions.
let wheels = if remote.is_empty() {
vec![]
Expand Down
26 changes: 24 additions & 2 deletions crates/uv/src/commands/pip/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,14 +544,35 @@ pub(crate) async fn pip_sync(
preview,
);

// Sync the environment.
match operations::install(
let start = std::time::Instant::now();

// Make a plan
let plan = match operations::plan(
&resolution,
site_packages,
InstallationStrategy::Permissive,
Modifications::Exact,
&reinstall,
&build_options,
&hasher,
&tags,
&build_dispatch,
&cache,
&environment,
) {
Ok(plan) => plan,
Err(err) => {
return diagnostics::OperationDiagnostic::native_tls(client_builder.is_native_tls())
.report(err)
.map_or(Ok(ExitStatus::Failure), |err| Err(err.into()));
}
};

// Sync the environment.
match operations::install(
&resolution,
plan,
&build_options,
link_mode,
compile,
&hasher,
Expand All @@ -567,6 +588,7 @@ pub(crate) async fn pip_sync(
dry_run,
printer,
preview,
start,
)
.await
{
Expand Down
37 changes: 33 additions & 4 deletions crates/uv/src/commands/project/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2140,14 +2140,27 @@ pub(crate) async fn sync_environment(
preview,
);

// Sync the environment.
pip::operations::install(
let start = std::time::Instant::now();

let plan = pip::operations::plan(
resolution,
site_packages,
InstallationStrategy::Permissive,
modifications,
reinstall,
build_options,
&hasher,
tags,
&build_dispatch,
cache,
&venv,
)?;

// Sync the environment.
pip::operations::install(
resolution,
plan,
build_options,
link_mode,
compile_bytecode,
&hasher,
Expand All @@ -2163,6 +2176,7 @@ pub(crate) async fn sync_environment(
dry_run,
printer,
preview,
start,
)
.await?;

Expand Down Expand Up @@ -2408,14 +2422,28 @@ pub(crate) async fn update_environment(
Err(err) => return Err(err.into()),
};

// Sync the environment.
let changelog = pip::operations::install(
let start = std::time::Instant::now();

// Make a plan
let plan = pip::operations::plan(
&resolution,
site_packages,
InstallationStrategy::Permissive,
modifications,
reinstall,
build_options,
&hasher,
&tags,
&build_dispatch,
cache,
&venv,
)?;

// Sync the environment.
let changelog = pip::operations::install(
&resolution,
plan,
build_options,
*link_mode,
*compile_bytecode,
&hasher,
Expand All @@ -2431,6 +2459,7 @@ pub(crate) async fn update_environment(
dry_run,
printer,
preview,
start,
)
.await?;

Expand Down
18 changes: 16 additions & 2 deletions crates/uv/src/commands/project/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,14 +824,27 @@ pub(super) async fn do_sync(

let site_packages = SitePackages::from_environment(venv)?;

// Sync the environment.
operations::install(
let start = std::time::Instant::now();

let plan = operations::plan(
&resolution,
site_packages,
InstallationStrategy::Strict,
modifications,
reinstall,
build_options,
&hasher,
&tags,
&build_dispatch,
cache,
venv,
)?;

// Sync the environment.
operations::install(
&resolution,
plan,
build_options,
link_mode,
compile_bytecode,
&hasher,
Expand All @@ -847,6 +860,7 @@ pub(super) async fn do_sync(
dry_run,
printer,
preview,
start,
)
.await?;

Expand Down
Loading