From 11556db2a0a0f284ec3810725f89249754ab906f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 9 Feb 2025 14:51:29 -0500 Subject: [PATCH] Use stable environments for remote and stdin scripts --- Cargo.lock | 1 + crates/uv-python/src/environment.rs | 4 +- crates/uv-scripts/Cargo.toml | 1 + crates/uv-scripts/src/lib.rs | 21 ++- crates/uv/src/commands/project/environment.rs | 5 - crates/uv/src/commands/project/mod.rs | 124 +++++++----- crates/uv/src/commands/project/run.rs | 178 ++++++------------ crates/uv/src/lib.rs | 20 +- 8 files changed, 157 insertions(+), 197 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1f2cec50874..53bbd6146a74 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5569,6 +5569,7 @@ dependencies = [ "serde", "thiserror 2.0.11", "toml", + "url", "uv-pep440", "uv-pep508", "uv-pypi-types", diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index 26e9c449e403..04cae0df93ee 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -1,10 +1,12 @@ -use owo_colors::OwoColorize; use std::borrow::Cow; use std::env; use std::fmt; use std::path::{Path, PathBuf}; use std::sync::Arc; + +use owo_colors::OwoColorize; use tracing::debug; + use uv_cache::Cache; use uv_cache_key::cache_digest; use uv_fs::{LockedFile, Simplified}; diff --git a/crates/uv-scripts/Cargo.toml b/crates/uv-scripts/Cargo.toml index a92848d4d83c..2554e6b23700 100644 --- a/crates/uv-scripts/Cargo.toml +++ b/crates/uv-scripts/Cargo.toml @@ -23,3 +23,4 @@ memchr = { workspace = true } serde = { workspace = true, features = ["derive"] } thiserror = { workspace = true } toml = { workspace = true } +url = { workspace = true } diff --git a/crates/uv-scripts/src/lib.rs b/crates/uv-scripts/src/lib.rs index c837988aa3c4..377cc4a541ee 100644 --- a/crates/uv-scripts/src/lib.rs +++ b/crates/uv-scripts/src/lib.rs @@ -7,6 +7,7 @@ use std::sync::LazyLock; use memchr::memmem::Finder; use serde::Deserialize; use thiserror::Error; +use url::Url; use uv_pep440::VersionSpecifiers; use uv_pep508::PackageName; @@ -24,7 +25,7 @@ pub enum Pep723Item { /// A PEP 723 script provided via `stdin`. Stdin(Pep723Metadata), /// A PEP 723 script provided via a remote URL. - Remote(Pep723Metadata), + Remote(Pep723Metadata, Url), } impl Pep723Item { @@ -33,7 +34,7 @@ impl Pep723Item { match self { Self::Script(script) => &script.metadata, Self::Stdin(metadata) => metadata, - Self::Remote(metadata) => metadata, + Self::Remote(metadata, ..) => metadata, } } @@ -42,7 +43,7 @@ impl Pep723Item { match self { Self::Script(script) => script.metadata, Self::Stdin(metadata) => metadata, - Self::Remote(metadata) => metadata, + Self::Remote(metadata, ..) => metadata, } } @@ -50,8 +51,8 @@ impl Pep723Item { pub fn path(&self) -> Option<&Path> { match self { Self::Script(script) => Some(&script.path), - Self::Stdin(_) => None, - Self::Remote(_) => None, + Self::Stdin(..) => None, + Self::Remote(..) => None, } } @@ -72,7 +73,7 @@ pub enum Pep723ItemRef<'item> { /// A PEP 723 script provided via `stdin`. Stdin(&'item Pep723Metadata), /// A PEP 723 script provided via a remote URL. - Remote(&'item Pep723Metadata), + Remote(&'item Pep723Metadata, &'item Url), } impl Pep723ItemRef<'_> { @@ -81,7 +82,7 @@ impl Pep723ItemRef<'_> { match self { Self::Script(script) => &script.metadata, Self::Stdin(metadata) => metadata, - Self::Remote(metadata) => metadata, + Self::Remote(metadata, ..) => metadata, } } @@ -89,8 +90,8 @@ impl Pep723ItemRef<'_> { pub fn path(&self) -> Option<&Path> { match self { Self::Script(script) => Some(&script.path), - Self::Stdin(_) => None, - Self::Remote(_) => None, + Self::Stdin(..) => None, + Self::Remote(..) => None, } } } @@ -100,7 +101,7 @@ impl<'item> From<&'item Pep723Item> for Pep723ItemRef<'item> { match item { Pep723Item::Script(script) => Self::Script(script), Pep723Item::Stdin(metadata) => Self::Stdin(metadata), - Pep723Item::Remote(metadata) => Self::Remote(metadata), + Pep723Item::Remote(metadata, url) => Self::Remote(metadata, url), } } } diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index a22b3bee1100..0bff1ca60cdc 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -157,11 +157,6 @@ impl CachedEnvironment { Ok(()) } - /// Convert the [`CachedEnvironment`] into an [`Interpreter`]. - pub(crate) fn into_interpreter(self) -> Interpreter { - self.0.into_interpreter() - } - /// Return the [`Interpreter`] to use for the cached environment, based on a given /// [`Interpreter`]. /// diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 06a691110f8f..d63736b70dd8 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -37,7 +37,7 @@ use uv_resolver::{ FlatIndex, Lock, OptionsBuilder, PythonRequirement, RequiresPython, ResolverEnvironment, ResolverOutput, }; -use uv_scripts::{Pep723ItemRef, Pep723Script}; +use uv_scripts::Pep723ItemRef; use uv_settings::PythonInstallMirrors; use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_warnings::{warn_user, warn_user_once}; @@ -525,14 +525,21 @@ pub(crate) enum ScriptInterpreter { impl ScriptInterpreter { /// Return the expected virtual environment path for the [`Pep723Script`]. - pub(crate) fn root(script: &Pep723Script, cache: &Cache) -> PathBuf { - let digest = cache_digest(&script.path); - let entry = if let Some(file_name) = script.path.file_stem().and_then(|name| name.to_str()) - { - // Replace any non-ASCII characters with underscores. - format!("{file_name}-{digest}",) - } else { - digest + pub(crate) fn root(script: Pep723ItemRef<'_>, cache: &Cache) -> PathBuf { + let entry = match script { + // For local scripts, use a hash of the path to the script. + Pep723ItemRef::Script(script) => { + let digest = cache_digest(&script.path); + if let Some(file_name) = script.path.file_stem().and_then(|name| name.to_str()) { + format!("{file_name}-{digest}") + } else { + digest + } + } + // For remote scripts, use a hash of the URL. + Pep723ItemRef::Remote(.., url) => cache_digest(url), + // Otherwise, use a hash of the metadata. + Pep723ItemRef::Stdin(metadata) => cache_digest(&metadata.raw), }; cache .shard(CacheBucket::Environments, entry) @@ -562,42 +569,39 @@ impl ScriptInterpreter { requires_python, } = ScriptPython::from_request(python_request, workspace, script, no_config).await?; - // If this is a local script, use a stable virtual environment. - if let Pep723ItemRef::Script(script) = script { - let root = Self::root(script, cache); - match PythonEnvironment::from_root(&root, cache) { - Ok(venv) => { - if python_request.as_ref().map_or(true, |request| { - if request.satisfied(venv.interpreter(), cache) { - debug!( - "The script environment's Python version satisfies `{}`", - request.to_canonical_string() - ); - true - } else { - debug!( - "The script environment's Python version does not satisfy `{}`", - request.to_canonical_string() - ); - false - } - }) { - if let Some((requires_python, ..)) = requires_python.as_ref() { - if requires_python.contains(venv.interpreter().python_version()) { - return Ok(Self::Environment(venv)); - } - debug!( - "The script environment's Python version does not meet the script's Python requirement: `{requires_python}`" - ); - } else { + let root = Self::root(script, cache); + match PythonEnvironment::from_root(&root, cache) { + Ok(venv) => { + if python_request.as_ref().map_or(true, |request| { + if request.satisfied(venv.interpreter(), cache) { + debug!( + "The script environment's Python version satisfies `{}`", + request.to_canonical_string() + ); + true + } else { + debug!( + "The script environment's Python version does not satisfy `{}`", + request.to_canonical_string() + ); + false + } + }) { + if let Some((requires_python, ..)) = requires_python.as_ref() { + if requires_python.contains(venv.interpreter().python_version()) { return Ok(Self::Environment(venv)); } + debug!( + "The script environment's Python version does not meet the script's Python requirement: `{requires_python}`" + ); + } else { + return Ok(Self::Environment(venv)); } } - Err(uv_python::Error::MissingEnvironment(_)) => {} - Err(err) => warn!("Ignoring existing script environment: {err}"), - }; - } + } + Err(uv_python::Error::MissingEnvironment(_)) => {} + Err(err) => warn!("Ignoring existing script environment: {err}"), + }; let client_builder = BaseClientBuilder::new() .connectivity(connectivity) @@ -644,12 +648,30 @@ impl ScriptInterpreter { } /// Grab a file lock for the script to prevent concurrent writes across processes. - pub(crate) async fn lock(script: &Pep723Script) -> Result { - LockedFile::acquire( - std::env::temp_dir().join(format!("uv-{}.lock", cache_digest(&script.path))), - script.path.simplified_display(), - ) - .await + pub(crate) async fn lock(script: Pep723ItemRef<'_>) -> Result { + match script { + Pep723ItemRef::Script(script) => { + LockedFile::acquire( + std::env::temp_dir().join(format!("uv-{}.lock", cache_digest(&script.path))), + script.path.simplified_display(), + ) + .await + } + Pep723ItemRef::Remote(.., url) => { + LockedFile::acquire( + std::env::temp_dir().join(format!("uv-{}.lock", cache_digest(url))), + url.to_string(), + ) + .await + } + Pep723ItemRef::Stdin(metadata) => { + LockedFile::acquire( + std::env::temp_dir().join(format!("uv-{}.lock", cache_digest(&metadata.raw))), + "stdin".to_string(), + ) + .await + } + } } } @@ -1175,7 +1197,7 @@ struct ScriptEnvironment(PythonEnvironment); impl ScriptEnvironment { /// Initialize a virtual environment for a PEP 723 script. pub(crate) async fn get_or_init( - script: &Pep723Script, + script: Pep723ItemRef<'_>, python_request: Option, python_preference: PythonPreference, python_downloads: PythonDownloads, @@ -1191,7 +1213,7 @@ impl ScriptEnvironment { let _lock = ScriptInterpreter::lock(script).await?; match ScriptInterpreter::discover( - Pep723ItemRef::Script(script), + script, python_request, python_preference, python_downloads, @@ -1234,8 +1256,8 @@ impl ScriptEnvironment { // 1) The name of the script // 2) No prompt let prompt = script - .path - .file_name() + .path() + .and_then(|path| path.file_name()) .map(|f| f.to_string_lossy().to_string()) .map(uv_virtualenv::Prompt::Static) .unwrap_or(uv_virtualenv::Prompt::None); diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index f036209b6b39..faff07141cda 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -180,14 +180,14 @@ pub(crate) async fn run( script.path.user_display() ); } - Pep723Item::Stdin(_) => { + Pep723Item::Stdin(..) => { if requirements_from_stdin { bail!("Cannot read both requirements file and script from stdin"); } - debug!("Reading inline script metadata from `{}`", "stdin"); + debug!("Reading inline script metadata from stdin"); } - Pep723Item::Remote(_) => { - debug!("Reading inline script metadata from `{}`", "remote URL"); + Pep723Item::Remote(..) => { + debug!("Reading inline script metadata from remote URL"); } } @@ -201,7 +201,7 @@ pub(crate) async fn run( // Discover the interpreter for the script. let environment = ScriptEnvironment::get_or_init( - script.as_script().unwrap(), + (&script).into(), python.as_deref().map(PythonRequest::parse), python_preference, python_downloads, @@ -409,119 +409,57 @@ pub(crate) async fn run( let spec = RequirementsSpecification::from_overrides(requirements, constraints, overrides); - if let Some(script) = script.as_script() { - // If the script is a local file, use a persistent environment. - let environment = ScriptEnvironment::get_or_init( - script, - python.as_deref().map(PythonRequest::parse), - python_preference, - python_downloads, - connectivity, - native_tls, - allow_insecure_host, - &install_mirrors, - no_config, - cache, - printer, - ) - .await? - .into_environment(); - - match update_environment( - environment, - spec, - modifications, - &settings, - &sync_state, - if show_resolution { - Box::new(DefaultResolveLogger) - } else { - Box::new(SummaryResolveLogger) - }, - if show_resolution { - Box::new(DefaultInstallLogger) - } else { - Box::new(SummaryInstallLogger) - }, - installer_metadata, - connectivity, - concurrency, - native_tls, - allow_insecure_host, - cache, - printer, - preview, - ) - .await - { - Ok(update) => Some(update.into_environment().into_interpreter()), - Err(ProjectError::Operation(err)) => { - return diagnostics::OperationDiagnostic::native_tls(native_tls) - .with_context("script") - .report(err) - .map_or(Ok(ExitStatus::Failure), |err| Err(err.into())) - } - Err(err) => return Err(err.into()), - } - } else { - // Otherwise, use an ephemeral environment. - let interpreter = ScriptInterpreter::discover( - (&script).into(), - python.as_deref().map(PythonRequest::parse), - python_preference, - python_downloads, - connectivity, - native_tls, - allow_insecure_host, - &install_mirrors, - no_config, - cache, - printer, - ) - .await? - .into_interpreter(); - - let result = CachedEnvironment::from_spec( - EnvironmentSpecification::from(spec), - &interpreter, - &settings, - &sync_state, - if show_resolution { - Box::new(DefaultResolveLogger) - } else { - Box::new(SummaryResolveLogger) - }, - if show_resolution { - Box::new(DefaultInstallLogger) - } else { - Box::new(SummaryInstallLogger) - }, - installer_metadata, - connectivity, - concurrency, - native_tls, - allow_insecure_host, - cache, - printer, - preview, - ) - .await; - - let environment = match result { - Ok(resolution) => resolution, - Err(ProjectError::Operation(err)) => { - return diagnostics::OperationDiagnostic::native_tls(native_tls) - .with_context("script") - .report(err) - .map_or(Ok(ExitStatus::Failure), |err| Err(err.into())) - } - Err(err) => return Err(err.into()), - }; - - // Clear any existing overlay. - environment.clear_overlay()?; + let environment = ScriptEnvironment::get_or_init( + (&script).into(), + python.as_deref().map(PythonRequest::parse), + python_preference, + python_downloads, + connectivity, + native_tls, + allow_insecure_host, + &install_mirrors, + no_config, + cache, + printer, + ) + .await? + .into_environment(); - Some(environment.into_interpreter()) + match update_environment( + environment, + spec, + modifications, + &settings, + &sync_state, + if show_resolution { + Box::new(DefaultResolveLogger) + } else { + Box::new(SummaryResolveLogger) + }, + if show_resolution { + Box::new(DefaultInstallLogger) + } else { + Box::new(SummaryInstallLogger) + }, + installer_metadata, + connectivity, + concurrency, + native_tls, + allow_insecure_host, + cache, + printer, + preview, + ) + .await + { + Ok(update) => Some(update.into_environment().into_interpreter()), + Err(ProjectError::Operation(err)) => { + return diagnostics::OperationDiagnostic::native_tls(native_tls) + .with_context("script") + .report(err) + .map_or(Ok(ExitStatus::Failure), |err| Err(err.into())) + } + Err(err) => return Err(err.into()), } } else { // Create a virtual environment. @@ -1266,7 +1204,7 @@ pub(crate) enum RunCommand { /// Execute a `pythonw` script provided via `stdin`. PythonGuiStdin(Vec, Vec), /// Execute a Python script provided via a remote URL. - PythonRemote(tempfile::NamedTempFile, Vec), + PythonRemote(Url, tempfile::NamedTempFile, Vec), /// Execute an external command. External(OsString, Vec), /// Execute an empty command (in practice, `python` with no arguments). @@ -1319,7 +1257,7 @@ impl RunCommand { process.args(args); process } - Self::PythonRemote(target, args) => { + Self::PythonRemote(.., target, args) => { let mut process = Command::new(interpreter.sys_executable()); process.arg(target.path()); process.args(args); @@ -1535,7 +1473,7 @@ impl RunCommand { writer.write_all(&chunk?)?; } - return Ok(Self::PythonRemote(file, args.to_vec())); + return Ok(Self::PythonRemote(url, file, args.to_vec())); } } diff --git a/crates/uv/src/lib.rs b/crates/uv/src/lib.rs index e63eee833b00..cb7f9d3dd155 100644 --- a/crates/uv/src/lib.rs +++ b/crates/uv/src/lib.rs @@ -178,9 +178,9 @@ async fn run(mut cli: Cli) -> Result { Err(Pep723Error::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => None, Err(err) => return Err(err.into()), }, - Some(RunCommand::PythonRemote(script, _)) => { + Some(RunCommand::PythonRemote(url, script, _)) => { match Pep723Metadata::read(&script).await { - Ok(Some(metadata)) => Some(Pep723Item::Remote(metadata)), + Ok(Some(metadata)) => Some(Pep723Item::Remote(metadata, url.clone())), Ok(None) => None, Err(Pep723Error::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => { None @@ -1571,8 +1571,8 @@ async fn run_project( // Unwrap the script. let script = script.map(|script| match script { Pep723Item::Script(script) => script, - Pep723Item::Stdin(_) => unreachable!("`uv lock` does not support stdin"), - Pep723Item::Remote(_) => unreachable!("`uv lock` does not support remote files"), + Pep723Item::Stdin(..) => unreachable!("`uv lock` does not support stdin"), + Pep723Item::Remote(..) => unreachable!("`uv lock` does not support remote files"), }); Box::pin(commands::lock( @@ -1669,8 +1669,8 @@ async fn run_project( // Unwrap the script. let script = script.map(|script| match script { Pep723Item::Script(script) => script, - Pep723Item::Stdin(_) => unreachable!("`uv remove` does not support stdin"), - Pep723Item::Remote(_) => unreachable!("`uv remove` does not support remote files"), + Pep723Item::Stdin(..) => unreachable!("`uv remove` does not support stdin"), + Pep723Item::Remote(..) => unreachable!("`uv remove` does not support remote files"), }); Box::pin(commands::remove( @@ -1711,8 +1711,8 @@ async fn run_project( // Unwrap the script. let script = script.map(|script| match script { Pep723Item::Script(script) => script, - Pep723Item::Stdin(_) => unreachable!("`uv tree` does not support stdin"), - Pep723Item::Remote(_) => unreachable!("`uv tree` does not support remote files"), + Pep723Item::Stdin(..) => unreachable!("`uv tree` does not support stdin"), + Pep723Item::Remote(..) => unreachable!("`uv tree` does not support remote files"), }); Box::pin(commands::tree( @@ -1757,8 +1757,8 @@ async fn run_project( // Unwrap the script. let script = script.map(|script| match script { Pep723Item::Script(script) => script, - Pep723Item::Stdin(_) => unreachable!("`uv export` does not support stdin"), - Pep723Item::Remote(_) => unreachable!("`uv export` does not support remote files"), + Pep723Item::Stdin(..) => unreachable!("`uv export` does not support stdin"), + Pep723Item::Remote(..) => unreachable!("`uv export` does not support remote files"), }); commands::export(