Skip installing --with requirements if present in base environment (#4879)

## Summary

Closes #4547.
This commit is contained in:
Charlie Marsh 2024-07-07 20:23:59 -05:00 committed by GitHub
parent a76d04b159
commit 98a720ec08
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 184 additions and 47 deletions

View file

@ -13,7 +13,7 @@ use distribution_types::{
use pep440_rs::{Version, VersionSpecifiers}; use pep440_rs::{Version, VersionSpecifiers};
use pypi_types::{Requirement, VerbatimParsedUrl}; use pypi_types::{Requirement, VerbatimParsedUrl};
use uv_normalize::PackageName; use uv_normalize::PackageName;
use uv_python::PythonEnvironment; use uv_python::{Interpreter, PythonEnvironment};
use uv_types::InstalledPackagesProvider; use uv_types::InstalledPackagesProvider;
use crate::satisfies::RequirementSatisfaction; use crate::satisfies::RequirementSatisfaction;
@ -23,7 +23,7 @@ use crate::satisfies::RequirementSatisfaction;
/// Packages are indexed by both name and (for editable installs) URL. /// Packages are indexed by both name and (for editable installs) URL.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct SitePackages { pub struct SitePackages {
venv: PythonEnvironment, interpreter: Interpreter,
/// The vector of all installed distributions. The `by_name` and `by_url` indices index into /// 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 /// this vector. The vector may contain `None` values, which represent distributions that were
/// removed from the virtual environment. /// removed from the virtual environment.
@ -37,13 +37,18 @@ pub struct SitePackages {
} }
impl SitePackages { impl SitePackages {
/// Build an index of installed packages from the given Python environment.
pub fn from_environment(environment: &PythonEnvironment) -> Result<Self> {
Self::from_interpreter(environment.interpreter())
}
/// Build an index of installed packages from the given Python executable. /// Build an index of installed packages from the given Python executable.
pub fn from_environment(venv: &PythonEnvironment) -> Result<SitePackages> { pub fn from_interpreter(interpreter: &Interpreter) -> Result<Self> {
let mut distributions: Vec<Option<InstalledDist>> = Vec::new(); let mut distributions: Vec<Option<InstalledDist>> = Vec::new();
let mut by_name = FxHashMap::default(); let mut by_name = FxHashMap::default();
let mut by_url = 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. // Read the site-packages directory.
let site_packages = match fs::read_dir(site_packages) { let site_packages = match fs::read_dir(site_packages) {
Ok(site_packages) => { Ok(site_packages) => {
@ -66,7 +71,7 @@ impl SitePackages {
} }
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return Ok(Self { return Ok(Self {
venv: venv.clone(), interpreter: interpreter.clone(),
distributions, distributions,
by_name, by_name,
by_url, by_url,
@ -102,7 +107,7 @@ impl SitePackages {
} }
Ok(Self { Ok(Self {
venv: venv.clone(), interpreter: interpreter.clone(),
distributions, distributions,
by_name, by_name,
by_url, by_url,
@ -199,10 +204,10 @@ impl SitePackages {
// Verify that the package is compatible with the current Python version. // Verify that the package is compatible with the current Python version.
if let Some(requires_python) = metadata.requires_python.as_ref() { 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 { diagnostics.push(SitePackagesDiagnostic::IncompatiblePythonVersion {
package: package.clone(), package: package.clone(),
version: self.venv.interpreter().python_version().clone(), version: self.interpreter.python_version().clone(),
requires_python: requires_python.clone(), requires_python: requires_python.clone(),
}); });
} }
@ -210,7 +215,7 @@ impl SitePackages {
// Verify that the dependencies are installed. // Verify that the dependencies are installed.
for dependency in &metadata.requires_dist { for dependency in &metadata.requires_dist {
if !dependency.evaluate_markers(self.venv.interpreter().markers(), &[]) { if !dependency.evaluate_markers(self.interpreter.markers(), &[]) {
continue; continue;
} }
@ -268,7 +273,7 @@ impl SitePackages {
for entry in requirements { for entry in requirements {
if entry if entry
.requirement .requirement
.evaluate_markers(Some(self.venv.interpreter().markers()), &[]) .evaluate_markers(Some(self.interpreter.markers()), &[])
{ {
if seen.insert(entry.clone()) { if seen.insert(entry.clone()) {
stack.push(entry.clone()); stack.push(entry.clone());
@ -323,7 +328,7 @@ impl SitePackages {
// Add the dependencies to the queue. // Add the dependencies to the queue.
for dependency in metadata.requires_dist { for dependency in metadata.requires_dist {
if dependency.evaluate_markers( if dependency.evaluate_markers(
self.venv.interpreter().markers(), self.interpreter.markers(),
entry.requirement.extras(), entry.requirement.extras(),
) { ) {
let dependency = UnresolvedRequirementSpecification { let dependency = UnresolvedRequirementSpecification {

View file

@ -177,26 +177,7 @@ impl PythonEnvironment {
/// Some distributions also create symbolic links from `purelib` to `platlib`; in such cases, we /// Some distributions also create symbolic links from `purelib` to `platlib`; in such cases, we
/// still deduplicate the entries, returning a single path. /// still deduplicate the entries, returning a single path.
pub fn site_packages(&self) -> impl Iterator<Item = Cow<Path>> { pub fn site_packages(&self) -> impl Iterator<Item = Cow<Path>> {
let target = self.0.interpreter.target().map(Target::site_packages); self.0.interpreter.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))
} }
/// Returns the path to the `bin` directory inside this environment. /// Returns the path to the `bin` directory inside this environment.

View file

@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::io; use std::io;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus}; use std::process::{Command, ExitStatus};
@ -430,17 +431,40 @@ impl Interpreter {
} }
} }
/// Return an iterator over the `site-packages` directories inside the environment. /// Returns an iterator over the `site-packages` directories inside the environment.
pub fn site_packages(&self) -> impl Iterator<Item = &Path> { ///
let purelib = self.purelib(); /// In most cases, `purelib` and `platlib` will be the same, and so the iterator will contain
let platlib = self.platlib(); /// a single element; however, in some distributions, they may be different.
std::iter::once(purelib).chain( ///
if purelib == platlib || is_same_file(purelib, platlib).unwrap_or(false) { /// Some distributions also create symbolic links from `purelib` to `platlib`; in such cases, we
None /// still deduplicate the entries, returning a single path.
} else { pub fn site_packages(&self) -> impl Iterator<Item = Cow<Path>> {
Some(platlib) 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. /// Check if the interpreter matches the given Python version.

View file

@ -3,6 +3,7 @@ use std::ffi::OsString;
use std::path::PathBuf; use std::path::PathBuf;
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use itertools::Itertools;
use tokio::process::Command; use tokio::process::Command;
use tracing::debug; use tracing::debug;
@ -12,6 +13,7 @@ use uv_cli::ExternalCommand;
use uv_client::{BaseClientBuilder, Connectivity}; use uv_client::{BaseClientBuilder, Connectivity};
use uv_configuration::{Concurrency, ExtrasSpecification, PreviewMode}; use uv_configuration::{Concurrency, ExtrasSpecification, PreviewMode};
use uv_distribution::{VirtualProject, Workspace, WorkspaceError}; use uv_distribution::{VirtualProject, Workspace, WorkspaceError};
use uv_installer::{SatisfiesResult, SitePackages};
use uv_normalize::PackageName; use uv_normalize::PackageName;
use uv_python::{ use uv_python::{
request_from_version_file, EnvironmentPreference, Interpreter, PythonEnvironment, PythonFetch, 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. // If necessary, create an environment for the ephemeral requirements or command.
let temp_dir; let temp_dir;
let ephemeral_env = if requirements.is_empty() && base_interpreter.is_some() { let ephemeral_env = if skip_ephemeral {
// 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.
None None
} else { } else {
debug!("Creating ephemeral environment"); debug!("Creating ephemeral environment");
@ -351,8 +407,7 @@ pub(crate) async fn run(
.as_ref() .as_ref()
.map(Interpreter::site_packages) .map(Interpreter::site_packages)
.into_iter() .into_iter()
.flatten() .flatten(),
.map(Cow::Borrowed),
) )
.map(PathBuf::from) .map(PathBuf::from)
.chain( .chain(

View file

@ -326,3 +326,75 @@ fn run_managed_false() -> Result<()> {
Ok(()) 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(())
}