From b2f1fb5a9953fb65593098c760faac081c30c464 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 7 Jul 2024 20:24:12 -0400 Subject: [PATCH] Skip installing --with requirements if present in base environment --- crates/uv-installer/src/site_packages.rs | 27 +++++---- crates/uv-python/src/environment.rs | 21 +------ crates/uv-python/src/interpreter.rs | 46 +++++++++++---- crates/uv/src/commands/project/run.rs | 65 +++++++++++++++++++-- crates/uv/tests/run.rs | 72 ++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 47 deletions(-) diff --git a/crates/uv-installer/src/site_packages.rs b/crates/uv-installer/src/site_packages.rs index 84d31b3d5ae5..d70ff6c1d9b2 100644 --- a/crates/uv-installer/src/site_packages.rs +++ b/crates/uv-installer/src/site_packages.rs @@ -13,7 +13,7 @@ use distribution_types::{ use pep440_rs::{Version, VersionSpecifiers}; use pypi_types::{Requirement, VerbatimParsedUrl}; use uv_normalize::PackageName; -use uv_python::PythonEnvironment; +use uv_python::{Interpreter, PythonEnvironment}; use uv_types::InstalledPackagesProvider; use crate::satisfies::RequirementSatisfaction; @@ -23,7 +23,7 @@ use crate::satisfies::RequirementSatisfaction; /// Packages are indexed by both name and (for editable installs) URL. #[derive(Debug, Clone)] pub struct SitePackages { - venv: PythonEnvironment, + interpreter: Interpreter, /// The vector of all installed distributions. The `by_name` and `by_url` indices index into /// this vector. The vector may contain `None` values, which represent distributions that were /// removed from the virtual environment. @@ -37,13 +37,18 @@ pub struct SitePackages { } impl SitePackages { + /// Build an index of installed packages from the given Python environment. + pub fn from_environment(environment: &PythonEnvironment) -> Result { + Self::from_interpreter(environment.interpreter()) + } + /// Build an index of installed packages from the given Python executable. - pub fn from_environment(venv: &PythonEnvironment) -> Result { + pub fn from_interpreter(interpreter: &Interpreter) -> Result { let mut distributions: Vec> = Vec::new(); let mut by_name = FxHashMap::default(); let mut by_url = FxHashMap::default(); - for site_packages in venv.site_packages() { + for site_packages in interpreter.site_packages() { // Read the site-packages directory. let site_packages = match fs::read_dir(site_packages) { Ok(site_packages) => { @@ -66,7 +71,7 @@ impl SitePackages { } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { return Ok(Self { - venv: venv.clone(), + interpreter: interpreter.clone(), distributions, by_name, by_url, @@ -102,7 +107,7 @@ impl SitePackages { } Ok(Self { - venv: venv.clone(), + interpreter: interpreter.clone(), distributions, by_name, by_url, @@ -199,10 +204,10 @@ impl SitePackages { // Verify that the package is compatible with the current Python version. if let Some(requires_python) = metadata.requires_python.as_ref() { - if !requires_python.contains(self.venv.interpreter().python_version()) { + if !requires_python.contains(self.interpreter.python_version()) { diagnostics.push(SitePackagesDiagnostic::IncompatiblePythonVersion { package: package.clone(), - version: self.venv.interpreter().python_version().clone(), + version: self.interpreter.python_version().clone(), requires_python: requires_python.clone(), }); } @@ -210,7 +215,7 @@ impl SitePackages { // Verify that the dependencies are installed. for dependency in &metadata.requires_dist { - if !dependency.evaluate_markers(self.venv.interpreter().markers(), &[]) { + if !dependency.evaluate_markers(self.interpreter.markers(), &[]) { continue; } @@ -268,7 +273,7 @@ impl SitePackages { for entry in requirements { if entry .requirement - .evaluate_markers(Some(self.venv.interpreter().markers()), &[]) + .evaluate_markers(Some(self.interpreter.markers()), &[]) { if seen.insert(entry.clone()) { stack.push(entry.clone()); @@ -323,7 +328,7 @@ impl SitePackages { // Add the dependencies to the queue. for dependency in metadata.requires_dist { if dependency.evaluate_markers( - self.venv.interpreter().markers(), + self.interpreter.markers(), entry.requirement.extras(), ) { let dependency = UnresolvedRequirementSpecification { diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index 96a213dfdcc6..14b1bc15e2e9 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -177,26 +177,7 @@ impl PythonEnvironment { /// Some distributions also create symbolic links from `purelib` to `platlib`; in such cases, we /// still deduplicate the entries, returning a single path. pub fn site_packages(&self) -> impl Iterator> { - let target = self.0.interpreter.target().map(Target::site_packages); - - let prefix = self - .0 - .interpreter - .prefix() - .map(|prefix| prefix.site_packages(self.0.interpreter.virtualenv())); - - let interpreter = if target.is_none() && prefix.is_none() { - Some(self.0.interpreter.site_packages()) - } else { - None - }; - - target - .into_iter() - .flatten() - .map(Cow::Borrowed) - .chain(prefix.into_iter().flatten().map(Cow::Owned)) - .chain(interpreter.into_iter().flatten().map(Cow::Borrowed)) + self.0.interpreter.site_packages() } /// Returns the path to the `bin` directory inside this environment. diff --git a/crates/uv-python/src/interpreter.rs b/crates/uv-python/src/interpreter.rs index 0dfbe76f5c3e..1abf15cf9e8a 100644 --- a/crates/uv-python/src/interpreter.rs +++ b/crates/uv-python/src/interpreter.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; @@ -430,17 +431,40 @@ impl Interpreter { } } - /// Return an iterator over the `site-packages` directories inside the environment. - pub fn site_packages(&self) -> impl Iterator { - let purelib = self.purelib(); - let platlib = self.platlib(); - std::iter::once(purelib).chain( - if purelib == platlib || is_same_file(purelib, platlib).unwrap_or(false) { - None - } else { - Some(platlib) - }, - ) + /// Returns an iterator over the `site-packages` directories inside the environment. + /// + /// In most cases, `purelib` and `platlib` will be the same, and so the iterator will contain + /// a single element; however, in some distributions, they may be different. + /// + /// Some distributions also create symbolic links from `purelib` to `platlib`; in such cases, we + /// still deduplicate the entries, returning a single path. + pub fn site_packages(&self) -> impl Iterator> { + let target = self.target().map(Target::site_packages); + + let prefix = self + .prefix() + .map(|prefix| prefix.site_packages(self.virtualenv())); + + let interpreter = if target.is_none() && prefix.is_none() { + let purelib = self.purelib(); + let platlib = self.platlib(); + Some(std::iter::once(purelib).chain( + if purelib == platlib || is_same_file(purelib, platlib).unwrap_or(false) { + None + } else { + Some(platlib) + }, + )) + } else { + None + }; + + target + .into_iter() + .flatten() + .map(Cow::Borrowed) + .chain(prefix.into_iter().flatten().map(Cow::Owned)) + .chain(interpreter.into_iter().flatten().map(Cow::Borrowed)) } /// Check if the interpreter matches the given Python version. diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 2a2440abfadc..ee0e38db6b2f 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -3,6 +3,7 @@ use std::ffi::OsString; use std::path::PathBuf; use anyhow::{Context, Result}; +use itertools::Itertools; use tokio::process::Command; use tracing::debug; @@ -12,6 +13,7 @@ use uv_cli::ExternalCommand; use uv_client::{BaseClientBuilder, Connectivity}; use uv_configuration::{Concurrency, ExtrasSpecification, PreviewMode}; use uv_distribution::{VirtualProject, Workspace, WorkspaceError}; +use uv_installer::{SatisfiesResult, SitePackages}; use uv_normalize::PackageName; use uv_python::{ request_from_version_file, EnvironmentPreference, Interpreter, PythonEnvironment, PythonFetch, @@ -236,11 +238,65 @@ pub(crate) async fn run( ); } + // Read the `--with` requirements. + let spec = if requirements.is_empty() { + None + } else { + let client_builder = BaseClientBuilder::new() + .connectivity(connectivity) + .native_tls(native_tls); + + let spec = + RequirementsSpecification::from_simple_sources(&requirements, &client_builder).await?; + + Some(spec) + }; + + // Determine whether the base environment satisfies the ephemeral requirements. If we don't have + // any `--with` requirements, and we already have a base environment, then there's no need to + // create an additional environment. + let skip_ephemeral = base_interpreter.as_ref().is_some_and(|base_interpreter| { + let Some(spec) = spec.as_ref() else { + return true; + }; + + let Ok(site_packages) = SitePackages::from_interpreter(base_interpreter) else { + return false; + }; + + if !(settings.reinstall.is_none() && settings.reinstall.is_none()) { + return false; + } + + match site_packages.satisfies(&spec.requirements, &spec.constraints) { + // If the requirements are already satisfied, we're done. + Ok(SatisfiesResult::Fresh { + recursive_requirements, + }) => { + debug!( + "Base environment satisfies requirements: {}", + recursive_requirements + .iter() + .map(|entry| entry.requirement.to_string()) + .sorted() + .join(" | ") + ); + true + } + Ok(SatisfiesResult::Unsatisfied(requirement)) => { + debug!("At least one requirement is not satisfied in the base environment: {requirement}"); + false + } + Err(err) => { + debug!("Failed to check requirements against base environment: {err}"); + false + } + } + }); + // If necessary, create an environment for the ephemeral requirements or command. let temp_dir; - let ephemeral_env = if requirements.is_empty() && base_interpreter.is_some() { - // If we don't have any `--with` requirements, and we already have a base environment, then - // there's no need to create an additional environment. + let ephemeral_env = if skip_ephemeral { None } else { debug!("Creating ephemeral environment"); @@ -351,8 +407,7 @@ pub(crate) async fn run( .as_ref() .map(Interpreter::site_packages) .into_iter() - .flatten() - .map(Cow::Borrowed), + .flatten(), ) .map(PathBuf::from) .chain( diff --git a/crates/uv/tests/run.rs b/crates/uv/tests/run.rs index 4ca7583e3a3c..b55ed0d25882 100644 --- a/crates/uv/tests/run.rs +++ b/crates/uv/tests/run.rs @@ -326,3 +326,75 @@ fn run_managed_false() -> Result<()> { Ok(()) } + +#[test] +fn run_with() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! { r#" + [project] + name = "foo" + version = "1.0.0" + requires-python = ">=3.8" + dependencies = ["anyio", "sniffio==1.3.1"] + "# + })?; + + let test_script = context.temp_dir.child("main.py"); + test_script.write_str(indoc! { r" + import sniffio + " + })?; + + // Requesting an unsatisfied requirement should install it. + uv_snapshot!(context.filters(), context.run().arg("--with").arg("iniconfig").arg("main.py"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv run` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Prepared 4 packages in [TIME] + Installed 4 packages in [TIME] + + anyio==4.3.0 + + foo==1.0.0 (from file://[TEMP_DIR]/) + + idna==3.6 + + sniffio==1.3.1 + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==2.0.0 + "###); + + // Requesting a satisfied requirement should use the base environment. + uv_snapshot!(context.filters(), context.run().arg("--with").arg("sniffio").arg("main.py"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv run` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Audited 4 packages in [TIME] + "###); + + // Unless the user requests a different version. + uv_snapshot!(context.filters(), context.run().arg("--with").arg("sniffio<1.3.1").arg("main.py"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv run` is experimental and may change without warning. + Resolved 6 packages in [TIME] + Audited 4 packages in [TIME] + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + sniffio==1.3.0 + "###); + + Ok(()) +}