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

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

Merged
merged 1 commit into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 16 additions & 11 deletions crates/uv-installer/src/site_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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> {
Self::from_interpreter(environment.interpreter())
}

/// 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 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) => {
Expand All @@ -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,
Expand Down Expand Up @@ -102,7 +107,7 @@ impl SitePackages {
}

Ok(Self {
venv: venv.clone(),
interpreter: interpreter.clone(),
distributions,
by_name,
by_url,
Expand Down Expand Up @@ -199,18 +204,18 @@ 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(),
});
}
}

// 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;
}

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down
21 changes: 1 addition & 20 deletions crates/uv-python/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = Cow<Path>> {
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.
Expand Down
46 changes: 35 additions & 11 deletions crates/uv-python/src/interpreter.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::io;
use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus};
Expand Down Expand Up @@ -430,17 +431,40 @@ impl Interpreter {
}
}

/// Return an iterator over the `site-packages` directories inside the environment.
pub fn site_packages(&self) -> impl Iterator<Item = &Path> {
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<Item = Cow<Path>> {
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.
Expand Down
65 changes: 60 additions & 5 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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(
Expand Down
72 changes: 72 additions & 0 deletions crates/uv/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Loading