From f2b69d292c11fa74a39808ef4e6517fc308ab313 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 28 Aug 2025 09:41:17 -0500 Subject: [PATCH] Review --- crates/uv-install-wheel/Cargo.toml | 3 +- crates/uv-install-wheel/src/lib.rs | 1 - crates/uv-install-wheel/src/wheel.rs | 9 +- crates/uv-python/Cargo.toml | 3 +- crates/uv-python/src/managed.rs | 27 +- crates/uv-trampoline-builder/src/lib.rs | 368 ++++++++++++----------- crates/uv/Cargo.toml | 10 +- crates/uv/src/commands/python/install.rs | 29 +- 8 files changed, 219 insertions(+), 231 deletions(-) diff --git a/crates/uv-install-wheel/Cargo.toml b/crates/uv-install-wheel/Cargo.toml index 3eac625f9..06e72af39 100644 --- a/crates/uv-install-wheel/Cargo.toml +++ b/crates/uv-install-wheel/Cargo.toml @@ -29,6 +29,7 @@ uv-normalize = { workspace = true } uv-pep440 = { workspace = true } uv-pypi-types = { workspace = true } uv-shell = { workspace = true } +uv-trampoline-builder = { workspace = true } uv-warnings = { workspace = true } clap = { workspace = true, optional = true, features = ["derive"] } @@ -51,8 +52,6 @@ tracing = { workspace = true } walkdir = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] -uv-trampoline-builder = { workspace = true } - same-file = { workspace = true } self-replace = { workspace = true } diff --git a/crates/uv-install-wheel/src/lib.rs b/crates/uv-install-wheel/src/lib.rs index ea835d7be..f36a699f1 100644 --- a/crates/uv-install-wheel/src/lib.rs +++ b/crates/uv-install-wheel/src/lib.rs @@ -80,7 +80,6 @@ pub enum Error { MismatchedVersion(Version, Version), #[error("Invalid egg-link")] InvalidEggLink(PathBuf), - #[cfg(windows)] #[error(transparent)] LauncherError(#[from] uv_trampoline_builder::Error), #[error("Scripts must not use the reserved name {0}")] diff --git a/crates/uv-install-wheel/src/wheel.rs b/crates/uv-install-wheel/src/wheel.rs index 62d80e291..78ebab994 100644 --- a/crates/uv-install-wheel/src/wheel.rs +++ b/crates/uv-install-wheel/src/wheel.rs @@ -17,6 +17,7 @@ use uv_fs::{Simplified, persist_with_retry_sync, relative_to}; use uv_normalize::PackageName; use uv_pypi_types::DirectUrl; use uv_shell::escape_posix_for_single_quotes; +use uv_trampoline_builder::windows_script_launcher; use uv_warnings::warn_user_once; use crate::record::RecordEntry; @@ -225,18 +226,14 @@ pub(crate) fn write_script_entrypoints( ); // If necessary, wrap the launcher script in a Windows launcher binary. - #[cfg(windows)] - { - use uv_trampoline_builder::windows_script_launcher; + if cfg!(windows) { write_file_recorded( site_packages, &entrypoint_relative, &windows_script_launcher(&launcher_python_script, is_gui, &launcher_executable)?, record, )?; - } - #[cfg(not(windows))] - { + } else { write_file_recorded( site_packages, &entrypoint_relative, diff --git a/crates/uv-python/Cargo.toml b/crates/uv-python/Cargo.toml index cccf911c0..1c6f09b15 100644 --- a/crates/uv-python/Cargo.toml +++ b/crates/uv-python/Cargo.toml @@ -34,6 +34,7 @@ uv-pypi-types = { workspace = true } uv-redacted = { workspace = true } uv-state = { workspace = true } uv-static = { workspace = true } +uv-trampoline-builder = { workspace = true } uv-warnings = { workspace = true } anyhow = { workspace = true } @@ -68,8 +69,6 @@ which = { workspace = true } once_cell = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] -uv-trampoline-builder = { workspace = true } - windows-registry = { workspace = true } windows-result = { workspace = true } windows-sys = { workspace = true } diff --git a/crates/uv-python/src/managed.rs b/crates/uv-python/src/managed.rs index 581cb448d..09f51e085 100644 --- a/crates/uv-python/src/managed.rs +++ b/crates/uv-python/src/managed.rs @@ -20,6 +20,7 @@ use uv_platform::{Error as PlatformError, Os}; use uv_platform::{LibcDetectionError, Platform}; use uv_state::{StateBucket, StateStore}; use uv_static::EnvVars; +use uv_trampoline_builder::{Launcher, LauncherKind}; use crate::downloads::{Error as DownloadError, ManagedPythonDownload}; use crate::implementation::{ @@ -92,7 +93,6 @@ pub enum Error { }, #[error("Failed to find a directory to install executables into")] NoExecutableDirectory, - #[cfg(windows)] #[error(transparent)] LauncherError(#[from] uv_trampoline_builder::Error), #[error("Failed to read managed Python directory name: {0}")] @@ -619,13 +619,9 @@ impl ManagedPythonInstallation { /// Returns `true` if the path is a link to this installation's binary, e.g., as created by /// [`create_bin_link`]. pub fn is_bin_link(&self, path: &Path) -> bool { - #[cfg(unix)] - { + if cfg!(unix) { same_file::is_same_file(path, self.executable(false)).unwrap_or_default() - } - #[cfg(windows)] - { - use uv_trampoline_builder::{Launcher, LauncherKind}; + } else if cfg!(windows) { let Some(launcher) = Launcher::try_from_path(path).unwrap_or_default() else { return false; }; @@ -637,9 +633,7 @@ impl ManagedPythonInstallation { // directly. dunce::canonicalize(&launcher.python_path).unwrap_or(launcher.python_path) == self.executable(false) - } - #[cfg(not(any(unix, windows)))] - { + } else { unreachable!("Only Windows and Unix are supported") } } @@ -882,8 +876,7 @@ pub fn create_link_to_executable(link: &Path, executable: &Path) -> Result<(), E err, })?; - #[cfg(unix)] - { + if cfg!(unix) { // Note this will never copy on Unix — we use it here to allow compilation on Windows match symlink_or_copy_file(executable, link) { Ok(()) => Ok(()), @@ -896,9 +889,7 @@ pub fn create_link_to_executable(link: &Path, executable: &Path) -> Result<(), E err, }), } - } - #[cfg(windows)] - { + } else if cfg!(windows) { use uv_trampoline_builder::windows_python_launcher; // TODO(zanieb): Install GUI launchers as well @@ -916,10 +907,8 @@ pub fn create_link_to_executable(link: &Path, executable: &Path) -> Result<(), E err, }) } - } - #[cfg(not(any(unix, windows)))] - { - unimplemented!("Only Windows and Unix systems are supported.") + } else { + unimplemented!("Only Windows and Unix are supported.") } } diff --git a/crates/uv-trampoline-builder/src/lib.rs b/crates/uv-trampoline-builder/src/lib.rs index d89c17eb9..f44314f1a 100644 --- a/crates/uv-trampoline-builder/src/lib.rs +++ b/crates/uv-trampoline-builder/src/lib.rs @@ -51,109 +51,113 @@ pub struct Launcher { } impl Launcher { + /// Attempt to read [`Launcher`] metadata from a trampoline executable file. + /// + /// On Unix, this always returns [`None`]. Trampolines are a Windows-specific feature and cannot + /// be read on other platforms. + #[cfg(not(windows))] + pub fn try_from_path(_path: &Path) -> Result, Error> { + Ok(None) + } + /// Read [`Launcher`] metadata from a trampoline executable file. /// /// Returns `Ok(None)` if the file is not a trampoline executable. /// Returns `Err` if the file looks like a trampoline executable but is formatted incorrectly. - #[allow(unused_variables)] + #[cfg(windows)] pub fn try_from_path(path: &Path) -> Result, Error> { - #[cfg(not(windows))] - { - Err(Error::NotWindows) - } - #[cfg(windows)] - { - use std::os::windows::ffi::OsStrExt; - use windows::Win32::System::LibraryLoader::LOAD_LIBRARY_AS_DATAFILE; - use windows::Win32::System::LibraryLoader::LoadLibraryExW; + use std::os::windows::ffi::OsStrExt; + use windows::Win32::System::LibraryLoader::LOAD_LIBRARY_AS_DATAFILE; + use windows::Win32::System::LibraryLoader::LoadLibraryExW; - let path_str = path - .as_os_str() - .encode_wide() - .chain(std::iter::once(0)) - .collect::>(); + let path_str = path + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); - // SAFETY: winapi call; null-terminated strings - #[allow(unsafe_code)] - let Some(module) = (unsafe { - LoadLibraryExW( - windows::core::PCWSTR(path_str.as_ptr()), - None, - LOAD_LIBRARY_AS_DATAFILE, - ) - .ok() - }) else { + // SAFETY: winapi call; null-terminated strings + #[allow(unsafe_code)] + let Some(module) = (unsafe { + LoadLibraryExW( + windows::core::PCWSTR(path_str.as_ptr()), + None, + LOAD_LIBRARY_AS_DATAFILE, + ) + .ok() + }) else { + return Ok(None); + }; + + let result = (|| { + let Some(kind_data) = read_resource(module, RESOURCE_TRAMPOLINE_KIND) else { return Ok(None); }; - - let result = (|| { - let Some(kind_data) = read_resource(module, RESOURCE_TRAMPOLINE_KIND) else { - return Ok(None); - }; - let Some(kind) = LauncherKind::from_resource_value(kind_data[0]) else { - return Err(Error::UnprocessableMetadata); - }; - - let Some(path_data) = read_resource(module, RESOURCE_PYTHON_PATH) else { - return Ok(None); - }; - let python_path = PathBuf::from( - String::from_utf8(path_data) - .map_err(|err| Error::InvalidPath(err.utf8_error()))?, - ); - - let script_data = read_resource(module, RESOURCE_SCRIPT_DATA); - - Ok(Some(Self { - kind, - python_path, - script_data, - })) - })(); - - // SAFETY: winapi call; handle is known to be valid. - #[allow(unsafe_code)] - unsafe { - windows::Win32::Foundation::FreeLibrary(module) - .map_err(|err| Error::Io(io::Error::from_raw_os_error(err.code().0)))?; + let Some(kind) = LauncherKind::from_resource_value(kind_data[0]) else { + return Err(Error::UnprocessableMetadata); }; - result - } + let Some(path_data) = read_resource(module, RESOURCE_PYTHON_PATH) else { + return Ok(None); + }; + let python_path = PathBuf::from( + String::from_utf8(path_data).map_err(|err| Error::InvalidPath(err.utf8_error()))?, + ); + + let script_data = read_resource(module, RESOURCE_SCRIPT_DATA); + + Ok(Some(Self { + kind, + python_path, + script_data, + })) + })(); + + // SAFETY: winapi call; handle is known to be valid. + #[allow(unsafe_code)] + unsafe { + windows::Win32::Foundation::FreeLibrary(module) + .map_err(|err| Error::Io(io::Error::from_raw_os_error(err.code().0)))?; + }; + + result } - #[allow(unused_variables)] + /// Write this trampoline launcher to a file. + /// + /// On Unix, this always returns [`Error::NotWindows`]. Trampolines are a Windows-specific + /// feature and cannot be written on other platforms. + #[cfg(not(windows))] + pub fn write_to_file(self, _file_path: &Path, _is_gui: bool) -> Result<(), Error> { + Err(Error::NotWindows) + } + + /// Write this trampoline launcher to a file. + #[cfg(windows)] pub fn write_to_file(self, file_path: &Path, is_gui: bool) -> Result<(), Error> { - #[cfg(not(windows))] - { - Err(Error::NotWindows) + use uv_fs::Simplified; + let python_path = self.python_path.simplified_display().to_string(); + + // Write the launcher binary + fs_err::write(file_path, get_launcher_bin(is_gui)?)?; + + // Write resources + let resources = &[ + ( + RESOURCE_TRAMPOLINE_KIND, + &[self.kind.to_resource_value()][..], + ), + (RESOURCE_PYTHON_PATH, python_path.as_bytes()), + ]; + if let Some(script_data) = self.script_data { + let mut all_resources = resources.to_vec(); + all_resources.push((RESOURCE_SCRIPT_DATA, &script_data)); + write_resources(file_path, &all_resources)?; + } else { + write_resources(file_path, resources)?; } - #[cfg(windows)] - { - use uv_fs::Simplified; - let python_path = self.python_path.simplified_display().to_string(); - // Write the launcher binary - fs_err::write(file_path, get_launcher_bin(is_gui)?)?; - - // Write resources - let resources = &[ - ( - RESOURCE_TRAMPOLINE_KIND, - &[self.kind.to_resource_value()][..], - ), - (RESOURCE_PYTHON_PATH, python_path.as_bytes()), - ]; - if let Some(script_data) = self.script_data { - let mut all_resources = resources.to_vec(); - all_resources.push((RESOURCE_SCRIPT_DATA, &script_data)); - write_resources(file_path, &all_resources)?; - } else { - write_resources(file_path, resources)?; - } - - Ok(()) - } + Ok(()) } #[must_use] @@ -251,7 +255,6 @@ fn get_launcher_bin(gui: bool) -> Result<&'static [u8], Error> { } /// Helper to write Windows PE resources -#[allow(unused_variables)] #[cfg(windows)] fn write_resources(path: &Path, resources: &[(windows::core::PCWSTR, &[u8])]) -> Result<(), Error> { // SAFETY: winapi calls; null-terminated strings @@ -289,8 +292,8 @@ fn write_resources(path: &Path, resources: &[(windows::core::PCWSTR, &[u8])]) -> Ok(()) } -#[cfg(windows)] /// Safely reads a resource from a PE file +#[cfg(windows)] fn read_resource( handle: windows::Win32::Foundation::HMODULE, name: windows::core::PCWSTR, @@ -327,125 +330,136 @@ fn read_resource( } } +/// Construct a Windows script launcher. +/// +/// On Unix, this always returns [`Error::NotWindows`]. Trampolines are a Windows-specific feature +/// and cannot be created on other platforms. +#[cfg(not(windows))] +pub fn windows_script_launcher( + _launcher_python_script: &str, + _is_gui: bool, + _python_executable: impl AsRef, +) -> Result, Error> { + Err(Error::NotWindows) +} + +/// Construct a Windows script launcher. +/// /// A Windows script is a minimal .exe launcher binary with the python entrypoint script appended as /// stored zip file. /// /// -#[allow(unused_variables)] +#[cfg(windows)] pub fn windows_script_launcher( launcher_python_script: &str, is_gui: bool, python_executable: impl AsRef, ) -> Result, Error> { - // This method should only be called on Windows, but we avoid function-scope - // `#[cfg(windows)]` to retain compilation on all platforms. - #[cfg(not(windows))] + use std::io::{Cursor, Write}; + + use zip::ZipWriter; + use zip::write::SimpleFileOptions; + + use uv_fs::Simplified; + + let launcher_bin: &[u8] = get_launcher_bin(is_gui)?; + + let mut payload: Vec = Vec::new(); { - Err(Error::NotWindows) + // We're using the zip writer, but with stored compression + // https://github.com/njsmith/posy/blob/04927e657ca97a5e35bb2252d168125de9a3a025/src/trampolines/mod.rs#L75-L82 + // https://github.com/pypa/distlib/blob/8ed03aab48add854f377ce392efffb79bb4d6091/PC/launcher.c#L259-L271 + let stored = + SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + let mut archive = ZipWriter::new(Cursor::new(&mut payload)); + let error_msg = "Writing to Vec should never fail"; + archive.start_file("__main__.py", stored).expect(error_msg); + archive + .write_all(launcher_python_script.as_bytes()) + .expect(error_msg); + archive.finish().expect(error_msg); } - #[cfg(windows)] - { - use std::io::{Cursor, Write}; - use zip::ZipWriter; - use zip::write::SimpleFileOptions; + let python = python_executable.as_ref(); + let python_path = python.simplified_display().to_string(); - use uv_fs::Simplified; + // Start with base launcher binary + // Create temporary file for the launcher + let temp_dir = tempfile::TempDir::new()?; + let temp_file = temp_dir + .path() + .join(format!("uv-trampoline-{}.exe", std::process::id())); + fs_err::write(&temp_file, launcher_bin)?; - let launcher_bin: &[u8] = get_launcher_bin(is_gui)?; + // Write resources + let resources = &[ + ( + RESOURCE_TRAMPOLINE_KIND, + &[LauncherKind::Script.to_resource_value()][..], + ), + (RESOURCE_PYTHON_PATH, python_path.as_bytes()), + (RESOURCE_SCRIPT_DATA, &payload), + ]; + write_resources(&temp_file, resources)?; - let mut payload: Vec = Vec::new(); - { - // We're using the zip writer, but with stored compression - // https://github.com/njsmith/posy/blob/04927e657ca97a5e35bb2252d168125de9a3a025/src/trampolines/mod.rs#L75-L82 - // https://github.com/pypa/distlib/blob/8ed03aab48add854f377ce392efffb79bb4d6091/PC/launcher.c#L259-L271 - let stored = - SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); - let mut archive = ZipWriter::new(Cursor::new(&mut payload)); - let error_msg = "Writing to Vec should never fail"; - archive.start_file("__main__.py", stored).expect(error_msg); - archive - .write_all(launcher_python_script.as_bytes()) - .expect(error_msg); - archive.finish().expect(error_msg); - } + // Read back the complete file + let launcher = fs_err::read(&temp_file)?; + fs_err::remove_file(temp_file)?; - let python = python_executable.as_ref(); - let python_path = python.simplified_display().to_string(); - - // Start with base launcher binary - // Create temporary file for the launcher - let temp_dir = tempfile::TempDir::new()?; - let temp_file = temp_dir - .path() - .join(format!("uv-trampoline-{}.exe", std::process::id())); - fs_err::write(&temp_file, launcher_bin)?; - - // Write resources - let resources = &[ - ( - RESOURCE_TRAMPOLINE_KIND, - &[LauncherKind::Script.to_resource_value()][..], - ), - (RESOURCE_PYTHON_PATH, python_path.as_bytes()), - (RESOURCE_SCRIPT_DATA, &payload), - ]; - write_resources(&temp_file, resources)?; - - // Read back the complete file - let launcher = fs_err::read(&temp_file)?; - fs_err::remove_file(temp_file)?; - - Ok(launcher) - } + Ok(launcher) } +/// Construct a Windows Python launcher. +/// +/// On Unix, this always returns [`Error::NotWindows`]. Trampolines are a Windows-specific feature +/// and cannot be created on other platforms. +#[cfg(not(windows))] +pub fn windows_python_launcher( + _python_executable: impl AsRef, + _is_gui: bool, +) -> Result, Error> { + Err(Error::NotWindows) +} + +/// Construct a Windows Python launcher. +/// /// A minimal .exe launcher binary for Python. /// /// Sort of equivalent to a `python` symlink on Unix. -#[allow(unused_variables)] +#[cfg(windows)] pub fn windows_python_launcher( python_executable: impl AsRef, is_gui: bool, ) -> Result, Error> { - // This method should only be called on Windows, but we avoid function-scope - // `#[cfg(windows)]` to retain compilation on all platforms. - #[cfg(not(windows))] - { - Err(Error::NotWindows) - } - #[cfg(windows)] - { - use uv_fs::Simplified; + use uv_fs::Simplified; - let launcher_bin: &[u8] = get_launcher_bin(is_gui)?; + let launcher_bin: &[u8] = get_launcher_bin(is_gui)?; - let python = python_executable.as_ref(); - let python_path = python.simplified_display().to_string(); + let python = python_executable.as_ref(); + let python_path = python.simplified_display().to_string(); - // Create temporary file for the launcher - let temp_dir = tempfile::TempDir::new()?; - let temp_file = temp_dir - .path() - .join(format!("uv-trampoline-{}.exe", std::process::id())); - fs_err::write(&temp_file, launcher_bin)?; + // Create temporary file for the launcher + let temp_dir = tempfile::TempDir::new()?; + let temp_file = temp_dir + .path() + .join(format!("uv-trampoline-{}.exe", std::process::id())); + fs_err::write(&temp_file, launcher_bin)?; - // Write resources - let resources = &[ - ( - RESOURCE_TRAMPOLINE_KIND, - &[LauncherKind::Python.to_resource_value()][..], - ), - (RESOURCE_PYTHON_PATH, python_path.as_bytes()), - ]; - write_resources(&temp_file, resources)?; + // Write resources + let resources = &[ + ( + RESOURCE_TRAMPOLINE_KIND, + &[LauncherKind::Python.to_resource_value()][..], + ), + (RESOURCE_PYTHON_PATH, python_path.as_bytes()), + ]; + write_resources(&temp_file, resources)?; - // Read back the complete file - let launcher = fs_err::read(&temp_file)?; - fs_err::remove_file(temp_file)?; + // Read back the complete file + let launcher = fs_err::read(&temp_file)?; + fs_err::remove_file(temp_file)?; - Ok(launcher) - } + Ok(launcher) } #[cfg(all(test, windows))] diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index a053fe3cb..49444f947 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -53,6 +53,7 @@ uv-shell = { workspace = true } uv-static = { workspace = true } uv-tool = { workspace = true } uv-torch = { workspace = true } +uv-trampoline-builder = { workspace = true } uv-types = { workspace = true } uv-version = { workspace = true } uv-virtualenv = { workspace = true } @@ -107,13 +108,8 @@ walkdir = { workspace = true } which = { workspace = true } zip = { workspace = true } -[target.'cfg(target_os = "windows")'.dependencies] -uv-trampoline-builder = { workspace = true } - arrayvec = { workspace = true } self-replace = { workspace = true } -windows = { workspace = true } -windows-result = { workspace = true } [dev-dependencies] assert_cmd = { workspace = true } @@ -135,6 +131,10 @@ whoami = { workspace = true } wiremock = { workspace = true } zip = { workspace = true } +[target.'cfg(target_os = "windows")'.dependencies] +windows = { workspace = true } +windows-result = { workspace = true } + [target.'cfg(unix)'.dependencies] nix = { workspace = true } diff --git a/crates/uv/src/commands/python/install.rs b/crates/uv/src/commands/python/install.rs index 2e09a4f7d..edb3d1b84 100644 --- a/crates/uv/src/commands/python/install.rs +++ b/crates/uv/src/commands/python/install.rs @@ -29,6 +29,7 @@ use uv_python::{ PythonVersionFile, VersionFileDiscoveryOptions, VersionFilePreference, VersionRequest, }; use uv_shell::Shell; +use uv_trampoline_builder::{Launcher, LauncherKind}; use uv_warnings::{warn_user, write_error_chain}; use crate::commands::python::{ChangeEvent, ChangeEventKind}; @@ -1050,32 +1051,22 @@ fn find_matching_bin_link<'a>( mut installations: impl Iterator, path: &Path, ) -> Option<&'a ManagedPythonInstallation> { - #[cfg(not(any(unix, windows)))] - { - unreachable!("Only Windows and Unix are supported"); - } - - #[cfg(unix)] - { + if cfg!(unix) { if !path.is_symlink() { return None; } let target = fs_err::canonicalize(path).ok()?; installations.find(|installation| installation.executable(false) == target) - } - #[cfg(windows)] - { - let target = { - use uv_trampoline_builder::{Launcher, LauncherKind}; - - let launcher: Launcher = Launcher::try_from_path(path).ok()??; - if !matches!(launcher.kind, LauncherKind::Python) { - return None; - } - dunce::canonicalize(launcher.python_path).ok()? - }; + } else if cfg!(windows) { + let launcher = Launcher::try_from_path(path).ok()??; + if !matches!(launcher.kind, LauncherKind::Python) { + return None; + } + let target = dunce::canonicalize(launcher.python_path).ok()?; installations.find(|installation| installation.executable(false) == target) + } else { + unreachable!("Only Unix and Windows are supported") } }