Encapsulate tempfile within uv run (#8114)

## Summary

Some follow-up refactors to confine all the remote URL downloading and
parsing to within the `uv run` target abstractions.
This commit is contained in:
Charlie Marsh 2024-10-11 01:41:38 +02:00 committed by GitHub
parent d864e1dbe5
commit 455b64cd77
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 154 additions and 136 deletions

View file

@ -22,7 +22,9 @@ pub enum Pep723Item {
/// A PEP 723 script read from disk. /// A PEP 723 script read from disk.
Script(Pep723Script), Script(Pep723Script),
/// A PEP 723 script provided via `stdin`. /// A PEP 723 script provided via `stdin`.
Stdin(Pep723Stdin), Stdin(Pep723Metadata),
/// A PEP 723 script provided via a remote URL.
Remote(Pep723Metadata),
} }
impl Pep723Item { impl Pep723Item {
@ -30,7 +32,8 @@ impl Pep723Item {
pub fn metadata(&self) -> &Pep723Metadata { pub fn metadata(&self) -> &Pep723Metadata {
match self { match self {
Self::Script(script) => &script.metadata, Self::Script(script) => &script.metadata,
Self::Stdin(stdin) => &stdin.metadata, Self::Stdin(metadata) => metadata,
Self::Remote(metadata) => metadata,
} }
} }
@ -38,7 +41,8 @@ impl Pep723Item {
pub fn into_metadata(self) -> Pep723Metadata { pub fn into_metadata(self) -> Pep723Metadata {
match self { match self {
Self::Script(script) => script.metadata, Self::Script(script) => script.metadata,
Self::Stdin(stdin) => stdin.metadata, Self::Stdin(metadata) => metadata,
Self::Remote(metadata) => metadata,
} }
} }
} }
@ -182,29 +186,6 @@ impl Pep723Script {
} }
} }
/// A PEP 723 script, provided via `stdin`.
#[derive(Debug)]
pub struct Pep723Stdin {
metadata: Pep723Metadata,
}
impl Pep723Stdin {
/// Parse the PEP 723 `script` metadata from `stdin`.
pub fn parse(contents: &[u8]) -> Result<Option<Self>, Pep723Error> {
// Extract the `script` tag.
let ScriptTag { metadata, .. } = match ScriptTag::parse(contents) {
Ok(Some(tag)) => tag,
Ok(None) => return Ok(None),
Err(err) => return Err(err),
};
// Parse the metadata.
let metadata = Pep723Metadata::from_str(&metadata)?;
Ok(Some(Self { metadata }))
}
}
/// PEP 723 metadata as parsed from a `script` comment block. /// PEP 723 metadata as parsed from a `script` comment block.
/// ///
/// See: <https://peps.python.org/pep-0723/> /// See: <https://peps.python.org/pep-0723/>
@ -219,6 +200,42 @@ pub struct Pep723Metadata {
pub raw: String, pub raw: String,
} }
impl Pep723Metadata {
/// Parse the PEP 723 metadata from `stdin`.
pub fn parse(contents: &[u8]) -> Result<Option<Self>, Pep723Error> {
// Extract the `script` tag.
let ScriptTag { metadata, .. } = match ScriptTag::parse(contents) {
Ok(Some(tag)) => tag,
Ok(None) => return Ok(None),
Err(err) => return Err(err),
};
// Parse the metadata.
Ok(Some(Self::from_str(&metadata)?))
}
/// Read the PEP 723 `script` metadata from a Python file, if it exists.
///
/// See: <https://peps.python.org/pep-0723/>
pub async fn read(file: impl AsRef<Path>) -> Result<Option<Self>, Pep723Error> {
let contents = match fs_err::tokio::read(&file).await {
Ok(contents) => contents,
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
Err(err) => return Err(err.into()),
};
// Extract the `script` tag.
let ScriptTag { metadata, .. } = match ScriptTag::parse(&contents) {
Ok(Some(tag)) => tag,
Ok(None) => return Ok(None),
Err(err) => return Err(err),
};
// Parse the metadata.
Ok(Some(Self::from_str(&metadata)?))
}
}
impl FromStr for Pep723Metadata { impl FromStr for Pep723Metadata {
type Err = Pep723Error; type Err = Pep723Error;

View file

@ -7,10 +7,12 @@ use std::path::{Path, PathBuf};
use anstream::eprint; use anstream::eprint;
use anyhow::{anyhow, bail, Context}; use anyhow::{anyhow, bail, Context};
use futures::StreamExt;
use itertools::Itertools; use itertools::Itertools;
use owo_colors::OwoColorize; use owo_colors::OwoColorize;
use tokio::process::Command; use tokio::process::Command;
use tracing::{debug, warn}; use tracing::{debug, warn};
use url::Url;
use uv_cache::Cache; use uv_cache::Cache;
use uv_cli::ExternalCommand; use uv_cli::ExternalCommand;
use uv_client::{BaseClientBuilder, Connectivity}; use uv_client::{BaseClientBuilder, Connectivity};
@ -107,18 +109,28 @@ pub(crate) async fn run(
// Determine whether the command to execute is a PEP 723 script. // Determine whether the command to execute is a PEP 723 script.
let temp_dir; let temp_dir;
let script_interpreter = if let Some(script) = script { let script_interpreter = if let Some(script) = script {
if let Pep723Item::Script(script) = &script { match &script {
writeln!( Pep723Item::Script(script) => {
printer.stderr(), writeln!(
"Reading inline script metadata from: {}", printer.stderr(),
script.path.user_display().cyan() "Reading inline script metadata from `{}`",
)?; script.path.user_display().cyan()
} else { )?;
writeln!( }
printer.stderr(), Pep723Item::Stdin(_) => {
"Reading inline script metadata from: `{}`", writeln!(
"stdin".cyan() printer.stderr(),
)?; "Reading inline script metadata from `{}`",
"stdin".cyan()
)?;
}
Pep723Item::Remote(_) => {
writeln!(
printer.stderr(),
"Reading inline script metadata from {}",
"remote URL".cyan()
)?;
}
} }
let (source, python_request) = if let Some(request) = python.as_deref() { let (source, python_request) = if let Some(request) = python.as_deref() {
@ -196,7 +208,7 @@ pub(crate) async fn run(
.parent() .parent()
.expect("script path has no parent") .expect("script path has no parent")
.to_owned(), .to_owned(),
Pep723Item::Stdin(..) => std::env::current_dir()?, Pep723Item::Stdin(..) | Pep723Item::Remote(..) => std::env::current_dir()?,
}; };
let script = script.into_metadata(); let script = script.into_metadata();
@ -962,6 +974,8 @@ pub(crate) enum RunCommand {
PythonZipapp(PathBuf, Vec<OsString>), PythonZipapp(PathBuf, Vec<OsString>),
/// Execute a `python` script provided via `stdin`. /// Execute a `python` script provided via `stdin`.
PythonStdin(Vec<u8>), PythonStdin(Vec<u8>),
/// Execute a Python script provided via a remote URL.
PythonRemote(tempfile::NamedTempFile, Vec<OsString>),
/// Execute an external command. /// Execute an external command.
External(OsString, Vec<OsString>), External(OsString, Vec<OsString>),
/// Execute an empty command (in practice, `python` with no arguments). /// Execute an empty command (in practice, `python` with no arguments).
@ -972,10 +986,11 @@ impl RunCommand {
/// Return the name of the target executable, for display purposes. /// Return the name of the target executable, for display purposes.
fn display_executable(&self) -> Cow<'_, str> { fn display_executable(&self) -> Cow<'_, str> {
match self { match self {
Self::Python(_) => Cow::Borrowed("python"), Self::Python(_)
Self::PythonScript(..) | Self::PythonScript(..)
| Self::PythonPackage(..) | Self::PythonPackage(..)
| Self::PythonZipapp(..) | Self::PythonZipapp(..)
| Self::PythonRemote(..)
| Self::Empty => Cow::Borrowed("python"), | Self::Empty => Cow::Borrowed("python"),
Self::PythonModule(..) => Cow::Borrowed("python -m"), Self::PythonModule(..) => Cow::Borrowed("python -m"),
Self::PythonGuiScript(..) => Cow::Borrowed("pythonw"), Self::PythonGuiScript(..) => Cow::Borrowed("pythonw"),
@ -1000,6 +1015,12 @@ impl RunCommand {
process.args(args); process.args(args);
process process
} }
Self::PythonRemote(target, args) => {
let mut process = Command::new(interpreter.sys_executable());
process.arg(target.path());
process.args(args);
process
}
Self::PythonModule(module, args) => { Self::PythonModule(module, args) => {
let mut process = Command::new(interpreter.sys_executable()); let mut process = Command::new(interpreter.sys_executable());
process.arg("-m"); process.arg("-m");
@ -1088,7 +1109,7 @@ impl std::fmt::Display for RunCommand {
} }
Ok(()) Ok(())
} }
Self::PythonStdin(_) => { Self::PythonStdin(..) | Self::PythonRemote(..) => {
write!(f, "python -c")?; write!(f, "python -c")?;
Ok(()) Ok(())
} }
@ -1108,23 +1129,64 @@ impl std::fmt::Display for RunCommand {
} }
impl RunCommand { impl RunCommand {
pub(crate) fn from_args( /// Determine the [`RunCommand`] for a given set of arguments.
pub(crate) async fn from_args(
command: &ExternalCommand, command: &ExternalCommand,
module: bool, module: bool,
script: bool, script: bool,
connectivity: Connectivity,
native_tls: bool,
) -> anyhow::Result<Self> { ) -> anyhow::Result<Self> {
let (target, args) = command.split(); let (target, args) = command.split();
let Some(target) = target else { let Some(target) = target else {
return Ok(Self::Empty); return Ok(Self::Empty);
}; };
let target_path = PathBuf::from(target);
// Determine whether the user provided a remote script.
if target_path.starts_with("http://") || target_path.starts_with("https://") {
// Only continue if we are absolutely certain no local file exists.
//
// We don't do this check on Windows since the file path would
// be invalid anyway, and thus couldn't refer to a local file.
if !cfg!(unix) || matches!(target_path.try_exists(), Ok(false)) {
let url = Url::parse(&target.to_string_lossy())?;
let file_stem = url
.path_segments()
.and_then(Iterator::last)
.and_then(|segment| segment.strip_suffix(".py"))
.unwrap_or("script");
let file = tempfile::Builder::new()
.prefix(file_stem)
.suffix(".py")
.tempfile()?;
let client = BaseClientBuilder::new()
.connectivity(connectivity)
.native_tls(native_tls)
.build();
let response = client.client().get(url.clone()).send().await?;
// Stream the response to the file.
let mut writer = file.as_file();
let mut reader = response.bytes_stream();
while let Some(chunk) = reader.next().await {
use std::io::Write;
writer.write_all(&chunk?)?;
}
return Ok(Self::PythonRemote(file, args.to_vec()));
}
}
if module { if module {
return Ok(Self::PythonModule(target.clone(), args.to_vec())); return Ok(Self::PythonModule(target.clone(), args.to_vec()));
} else if script { } else if script {
return Ok(Self::PythonScript(target.clone().into(), args.to_vec())); return Ok(Self::PythonScript(target.clone().into(), args.to_vec()));
} }
let target_path = PathBuf::from(target);
let metadata = target_path.metadata(); let metadata = target_path.metadata();
let is_file = metadata.as_ref().map_or(false, std::fs::Metadata::is_file); let is_file = metadata.as_ref().map_or(false, std::fs::Metadata::is_file);
let is_dir = metadata.as_ref().map_or(false, std::fs::Metadata::is_dir); let is_dir = metadata.as_ref().map_or(false, std::fs::Metadata::is_dir);

View file

@ -19,16 +19,12 @@ use uv_cli::{
compat::CompatArgs, BuildBackendCommand, CacheCommand, CacheNamespace, Cli, Commands, compat::CompatArgs, BuildBackendCommand, CacheCommand, CacheNamespace, Cli, Commands,
PipCommand, PipNamespace, ProjectCommand, PipCommand, PipNamespace, ProjectCommand,
}; };
use uv_cli::{ use uv_cli::{PythonCommand, PythonNamespace, ToolCommand, ToolNamespace, TopLevelArgs};
ExternalCommand, GlobalArgs, PythonCommand, PythonNamespace, ToolCommand, ToolNamespace,
TopLevelArgs,
};
#[cfg(feature = "self-update")] #[cfg(feature = "self-update")]
use uv_cli::{SelfCommand, SelfNamespace, SelfUpdateArgs}; use uv_cli::{SelfCommand, SelfNamespace, SelfUpdateArgs};
use uv_client::BaseClientBuilder;
use uv_fs::CWD; use uv_fs::CWD;
use uv_requirements::RequirementsSource; use uv_requirements::RequirementsSource;
use uv_scripts::{Pep723Item, Pep723Script, Pep723Stdin}; use uv_scripts::{Pep723Item, Pep723Metadata, Pep723Script};
use uv_settings::{Combine, FilesystemOptions, Options}; use uv_settings::{Combine, FilesystemOptions, Options};
use uv_warnings::{warn_user, warn_user_once}; use uv_warnings::{warn_user, warn_user_once};
use uv_workspace::{DiscoveryOptions, Workspace}; use uv_workspace::{DiscoveryOptions, Workspace};
@ -47,62 +43,6 @@ pub(crate) mod printer;
pub(crate) mod settings; pub(crate) mod settings;
pub(crate) mod version; pub(crate) mod version;
/// Resolves the script target for a run command.
///
/// If it's a local file, does nothing. If it's a URL, downloads the content
/// to a temporary file and updates the command. Prioritizes local files over URLs.
/// Returns Some(NamedTempFile) if a remote script was downloaded, None otherwise.
async fn resolve_script_target(
command: &mut ExternalCommand,
global_args: &GlobalArgs,
filesystem: Option<&FilesystemOptions>,
) -> Result<Option<tempfile::NamedTempFile>> {
use std::io::Write;
let Some(target) = command.get_mut(0) else {
return Ok(None);
};
// Only continue if we are absolutely certain no local file exists.
//
// We don't do this check on Windows since the file path would
// be invalid anyway, and thus couldn't refer to a local file.
if cfg!(unix) && !matches!(Path::new(target).try_exists(), Ok(false)) {
return Ok(None);
}
let maybe_url = target.to_string_lossy();
// Only continue if the target truly looks like a URL.
if !(maybe_url.starts_with("http://") || maybe_url.starts_with("https://")) {
return Ok(None);
}
let script_url = url::Url::parse(&maybe_url)?;
let file_stem = script_url
.path_segments()
.and_then(std::iter::Iterator::last)
.and_then(|segment| segment.strip_suffix(".py"))
.unwrap_or("script");
let mut temp_file = tempfile::Builder::new()
.prefix(file_stem)
.suffix(".py")
.tempfile()?;
// Respect cli flags and workspace settings.
let settings = GlobalSettings::resolve(global_args, filesystem);
let client = BaseClientBuilder::new()
.connectivity(settings.connectivity)
.native_tls(settings.native_tls)
.build();
let response = client.client().get(script_url).send().await?;
let contents = response.bytes().await?;
temp_file.write_all(&contents)?;
*target = temp_file.path().into();
Ok(Some(temp_file))
}
#[instrument(skip_all)] #[instrument(skip_all)]
async fn run(mut cli: Cli) -> Result<ExitStatus> { async fn run(mut cli: Cli) -> Result<ExitStatus> {
// Enable flag to pick up warnings generated by workspace loading. // Enable flag to pick up warnings generated by workspace loading.
@ -191,7 +131,6 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
}; };
// Parse the external command, if necessary. // Parse the external command, if necessary.
let mut maybe_tempfile: Option<tempfile::NamedTempFile> = None;
let run_command = if let Commands::Project(command) = &mut *cli.command { let run_command = if let Commands::Project(command) = &mut *cli.command {
if let ProjectCommand::Run(uv_cli::RunArgs { if let ProjectCommand::Run(uv_cli::RunArgs {
command: Some(command), command: Some(command),
@ -200,10 +139,17 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
.. ..
}) = &mut **command }) = &mut **command
{ {
maybe_tempfile = let settings = GlobalSettings::resolve(&cli.top_level.global_args, filesystem.as_ref());
resolve_script_target(command, &cli.top_level.global_args, filesystem.as_ref()) Some(
.await?; RunCommand::from_args(
Some(RunCommand::from_args(command, *module, *script)?) command,
*module,
*script,
settings.connectivity,
settings.native_tls,
)
.await?,
)
} else { } else {
None None
} }
@ -218,8 +164,11 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
Some( Some(
RunCommand::PythonScript(script, _) | RunCommand::PythonGuiScript(script, _), RunCommand::PythonScript(script, _) | RunCommand::PythonGuiScript(script, _),
) => Pep723Script::read(&script).await?.map(Pep723Item::Script), ) => Pep723Script::read(&script).await?.map(Pep723Item::Script),
Some(RunCommand::PythonRemote(script, _)) => {
Pep723Metadata::read(&script).await?.map(Pep723Item::Remote)
}
Some(RunCommand::PythonStdin(contents)) => { Some(RunCommand::PythonStdin(contents)) => {
Pep723Stdin::parse(contents)?.map(Pep723Item::Stdin) Pep723Metadata::parse(contents)?.map(Pep723Item::Stdin)
} }
_ => None, _ => None,
} }
@ -277,15 +226,6 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
Printer::Default Printer::Default
}; };
// We only have a tempfile if we downloaded a remote script.
if let Some(temp_file) = maybe_tempfile.as_ref() {
writeln!(
printer.stderr(),
"Downloaded remote script to: {}",
temp_file.path().display().cyan(),
)?;
}
// Configure the `warn!` macros, which control user-facing warnings in the CLI. // Configure the `warn!` macros, which control user-facing warnings in the CLI.
if globals.quiet { if globals.quiet {
uv_warnings::disable(); uv_warnings::disable();
@ -1226,7 +1166,6 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
.await .await
.expect("tokio threadpool exited unexpectedly"), .expect("tokio threadpool exited unexpectedly"),
}; };
drop(maybe_tempfile);
result result
} }
@ -1471,6 +1410,7 @@ async fn run_project(
let script = script.map(|script| match script { let script = script.map(|script| match script {
Pep723Item::Script(script) => script, Pep723Item::Script(script) => script,
Pep723Item::Stdin(_) => unreachable!("`uv remove` does not support stdin"), Pep723Item::Stdin(_) => unreachable!("`uv remove` does not support stdin"),
Pep723Item::Remote(_) => unreachable!("`uv remove` does not support remote files"),
}); });
commands::remove( commands::remove(

View file

@ -307,7 +307,7 @@ fn run_pep723_script() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]
@ -321,7 +321,7 @@ fn run_pep723_script() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
"###); "###);
@ -391,7 +391,7 @@ fn run_pep723_script() -> Result<()> {
Hello, world! Hello, world!
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
"###); "###);
// Running a script with `--locked` should warn. // Running a script with `--locked` should warn.
@ -402,7 +402,7 @@ fn run_pep723_script() -> Result<()> {
Hello, world! Hello, world!
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
warning: `--locked` is a no-op for Python scripts with inline metadata, which always run in isolation warning: `--locked` is a no-op for Python scripts with inline metadata, which always run in isolation
"###); "###);
@ -424,7 +424,7 @@ fn run_pep723_script() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
× No solution found when resolving script dependencies: × No solution found when resolving script dependencies:
Because there are no versions of add and you require add, we can conclude that your requirements are unsatisfiable. Because there are no versions of add and you require add, we can conclude that your requirements are unsatisfiable.
"###); "###);
@ -487,7 +487,7 @@ fn run_pep723_script_requires_python() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
warning: The Python request from `.python-version` resolved to Python 3.8.[X], which is incompatible with the script's Python requirement: `>=3.11` warning: The Python request from `.python-version` resolved to Python 3.8.[X], which is incompatible with the script's Python requirement: `>=3.11`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
@ -509,7 +509,7 @@ fn run_pep723_script_requires_python() -> Result<()> {
hello hello
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]
+ iniconfig==2.0.0 + iniconfig==2.0.0
@ -591,7 +591,7 @@ fn run_pep723_script_metadata() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]
@ -622,7 +622,7 @@ fn run_pep723_script_metadata() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: main.py Reading inline script metadata from `main.py`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]
@ -2095,7 +2095,7 @@ fn run_compiled_python_file() -> Result<()> {
----- stdout ----- ----- stdout -----
----- stderr ----- ----- stderr -----
Reading inline script metadata from: script.py Reading inline script metadata from `script.py`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]
@ -2258,7 +2258,7 @@ fn run_script_explicit() -> Result<()> {
Hello, world! Hello, world!
----- stderr ----- ----- stderr -----
Reading inline script metadata from: script Reading inline script metadata from `script`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]
@ -2319,8 +2319,7 @@ fn run_remote_pep723_script() {
Hello CI, from uv! Hello CI, from uv!
----- stderr ----- ----- stderr -----
Downloaded remote script to: [TEMP_PATH].py Reading inline script metadata from remote URL
Reading inline script metadata from: [TEMP_PATH].py
Resolved 4 packages in [TIME] Resolved 4 packages in [TIME]
Prepared 4 packages in [TIME] Prepared 4 packages in [TIME]
Installed 4 packages in [TIME] Installed 4 packages in [TIME]
@ -2386,7 +2385,7 @@ fn run_stdin_with_pep723() -> Result<()> {
Hello, world! Hello, world!
----- stderr ----- ----- stderr -----
Reading inline script metadata from: `stdin` Reading inline script metadata from `stdin`
Resolved 1 package in [TIME] Resolved 1 package in [TIME]
Prepared 1 package in [TIME] Prepared 1 package in [TIME]
Installed 1 package in [TIME] Installed 1 package in [TIME]