refactor(uv-trampoline,uv-trampoline-builder): address feedback

crates/uv-distribution/src/metadata/requires_dist.rs

@@ -619,11 +619,11 @@ mod test {
         "#};
 
         assert_snapshot!(format_err(input).await, @r#"
-        error: TOML parse error at line 8, column 16
+        error: TOML parse error at line 8, column 28
           |
         8 | tqdm = { url = invalid url to tqdm-4.66.0-py3-none-any.whl" }
           |                            ^
-        missing opening quote, expected `"`
+        missing comma between key-value pairs, expected `,`
         "#);
     }
 

crates/uv-trampoline-builder/src/lib.rs

@@ -1,11 +1,8 @@
-use std::io::{self, Cursor, Write};
+use std::io;
 use std::path::{Path, PathBuf};
 use std::str::Utf8Error;
 
 use thiserror::Error;
-use uv_fs::Simplified;
-use zip::ZipWriter;
-use zip::write::SimpleFileOptions;
 
 #[cfg(all(windows, target_arch = "x86"))]
 const LAUNCHER_I686_GUI: &[u8] =
@@ -36,12 +33,15 @@ const LAUNCHER_AARCH64_CONSOLE: &[u8] =
 const RT_RCDATA: u16 = 10;
 
 // Resource IDs matching uv-trampoline
-const RESOURCE_TRAMPOLINE_KIND: &str = "UV_TRAMPOLINE_KIND\0";
-const RESOURCE_PYTHON_PATH: &str = "UV_PYTHON_PATH\0";
+#[cfg(windows)]
+const RESOURCE_TRAMPOLINE_KIND: windows::core::PCWSTR = windows::core::w!("UV_TRAMPOLINE_KIND");
+#[cfg(windows)]
+const RESOURCE_PYTHON_PATH: windows::core::PCWSTR = windows::core::w!("UV_PYTHON_PATH");
 // Note: This does not need to be looked up as a resource, as we rely on `zipimport`
 // to do the loading work. Still, keeping the content under a resource means that it
 // sits nicely under the PE format.
-const RESOURCE_SCRIPT_DATA: &str = "UV_SCRIPT_DATA\0";
+#[cfg(windows)]
+const RESOURCE_SCRIPT_DATA: windows::core::PCWSTR = windows::core::w!("UV_SCRIPT_DATA");
 
 #[derive(Debug)]
 pub struct Launcher {
@@ -67,8 +67,11 @@ impl Launcher {
             use windows::Win32::System::LibraryLoader::LOAD_LIBRARY_AS_DATAFILE;
             use windows::Win32::System::LibraryLoader::LoadLibraryExW;
 
-            let mut path_str = path.as_os_str().encode_wide().collect::<Vec<_>>();
-            path_str.push(0);
+            let path_str = path
+                .as_os_str()
+                .encode_wide()
+                .chain(std::iter::once(0))
+                .collect::<Vec<_>>();
 
             // SAFETY: winapi call; null-terminated strings
             #[allow(unsafe_code)]
@@ -119,7 +122,15 @@ impl Launcher {
         }
     }
 
+    #[allow(unused_variables)]
     pub fn write_to_file(self, file_path: &Path, is_gui: bool) -> Result<(), Error> {
+        #[cfg(not(windows))]
+        {
+            Err(Error::NotWindows)
+        }
+        #[cfg(windows)]
+        {
+            use uv_fs::Simplified;
             let python_path = self.python_path.simplified_display().to_string();
 
             // Write the launcher binary
@@ -143,6 +154,7 @@ impl Launcher {
 
             Ok(())
         }
+    }
 
     #[must_use]
     pub fn with_python_path(self, path: PathBuf) -> Self {
@@ -166,6 +178,7 @@ pub enum LauncherKind {
 }
 
 impl LauncherKind {
+    #[cfg(windows)]
     fn to_resource_value(self) -> u8 {
         match self {
             Self::Script => 1,
@@ -203,6 +216,7 @@ pub enum Error {
 }
 
 #[allow(clippy::unnecessary_wraps, unused_variables)]
+#[cfg(windows)]
 fn get_launcher_bin(gui: bool) -> Result<&'static [u8], Error> {
     Ok(match std::env::consts::ARCH {
         #[cfg(all(windows, target_arch = "x86"))]
@@ -233,20 +247,13 @@ fn get_launcher_bin(gui: bool) -> Result<&'static [u8], Error> {
         arch => {
             return Err(Error::UnsupportedWindowsArch(arch));
         }
-        #[cfg(not(windows))]
-        _ => &[],
     })
 }
 
 /// Helper to write Windows PE resources
 #[allow(unused_variables)]
-fn write_resources(path: &Path, resources: &[(&str, &[u8])]) -> Result<(), Error> {
-    #[cfg(not(windows))]
-    {
-        Err(Error::NotWindows)
-    }
-    #[cfg(windows)]
-    {
+#[cfg(windows)]
+fn write_resources(path: &Path, resources: &[(windows::core::PCWSTR, &[u8])]) -> Result<(), Error> {
     // SAFETY: winapi calls; null-terminated strings
     #[allow(unsafe_code)]
     unsafe {
@@ -255,18 +262,19 @@ fn write_resources(path: &Path, resources: &[(&str, &[u8])]) -> Result<(), Error
             BeginUpdateResourceW, EndUpdateResourceW, UpdateResourceW,
         };
 
-            let mut path_str = path.as_os_str().encode_wide().collect::<Vec<_>>();
-            path_str.push(0);
+        let path_str = path
+            .as_os_str()
+            .encode_wide()
+            .chain(std::iter::once(0))
+            .collect::<Vec<_>>();
         let handle = BeginUpdateResourceW(windows::core::PCWSTR(path_str.as_ptr()), false)
             .map_err(|err| Error::Io(io::Error::from_raw_os_error(err.code().0)))?;
 
         for (name, data) in resources {
-                let mut name_null_term = name.encode_utf16().collect::<Vec<_>>();
-                name_null_term.push(0);
             UpdateResourceW(
                 handle,
                 windows::core::PCWSTR(RT_RCDATA as *const _),
-                    windows::core::PCWSTR(name_null_term.as_ptr()),
+                *name,
                 0,
                 Some(data.as_ptr().cast()),
                 u32::try_from(data.len()).map_err(|_| Error::ResourceTooLarge)?,
@@ -279,12 +287,14 @@ fn write_resources(path: &Path, resources: &[(&str, &[u8])]) -> Result<(), Error
     }
 
     Ok(())
-    }
 }
 
 #[cfg(windows)]
 /// Safely reads a resource from a PE file
-fn read_resource(handle: windows::Win32::Foundation::HMODULE, name: &str) -> Option<Vec<u8>> {
+fn read_resource(
+    handle: windows::Win32::Foundation::HMODULE,
+    name: windows::core::PCWSTR,
+) -> Option<Vec<u8>> {
     // SAFETY: winapi calls; null-terminated strings; all pointers are checked.
     #[allow(unsafe_code)]
     unsafe {
@@ -294,7 +304,7 @@ fn read_resource(handle: windows::Win32::Foundation::HMODULE, name: &str) -> Opt
         // Find the resource
         let resource = FindResourceW(
             Some(handle),
-            windows::core::PCWSTR(name.encode_utf16().collect::<Vec<_>>().as_ptr()),
+            name,
             windows::core::PCWSTR(RT_RCDATA as *const _),
         );
         if resource.is_invalid() {
@@ -303,6 +313,9 @@ fn read_resource(handle: windows::Win32::Foundation::HMODULE, name: &str) -> Opt
 
         // Get resource size and data
         let size = SizeofResource(Some(handle), resource);
+        if size == 0 {
+            return None;
+        }
         let data = LoadResource(Some(handle), resource).ok()?;
         let ptr = LockResource(data) as *const u8;
         if ptr.is_null() {
@@ -324,11 +337,20 @@ pub fn windows_script_launcher(
     is_gui: bool,
     python_executable: impl AsRef<Path>,
 ) -> Result<Vec<u8>, Error> {
-    // This method should only be called on Windows, but we avoid `#[cfg(windows)]` to retain
-    // compilation on all platforms.
-    if cfg!(not(windows)) {
-        return Err(Error::NotWindows);
+    // 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 std::io::{Cursor, Write};
+
+        use zip::ZipWriter;
+        use zip::write::SimpleFileOptions;
+
+        use uv_fs::Simplified;
 
         let launcher_bin: &[u8] = get_launcher_bin(is_gui)?;
 
@@ -353,7 +375,7 @@ pub fn windows_script_launcher(
 
         // Start with base launcher binary
         // Create temporary file for the launcher
-    let temp_dir = tempfile::tempdir()?;
+        let temp_dir = tempfile::TempDir::new()?;
         let temp_file = temp_dir
             .path()
             .join(format!("uv-trampoline-{}.exe", std::process::id()));
@@ -375,6 +397,7 @@ pub fn windows_script_launcher(
         fs_err::remove_file(temp_file)?;
 
         Ok(launcher)
+    }
 }
 
 /// A minimal .exe launcher binary for Python.
@@ -385,11 +408,15 @@ pub fn windows_python_launcher(
     python_executable: impl AsRef<Path>,
     is_gui: bool,
 ) -> Result<Vec<u8>, Error> {
-    // This method should only be called on Windows, but we avoid `#[cfg(windows)]` to retain
-    // compilation on all platforms.
-    if cfg!(not(windows)) {
-        return Err(Error::NotWindows);
+    // 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;
 
         let launcher_bin: &[u8] = get_launcher_bin(is_gui)?;
 
@@ -397,7 +424,7 @@ pub fn windows_python_launcher(
         let python_path = python.simplified_display().to_string();
 
         // Create temporary file for the launcher
-    let temp_dir = tempfile::tempdir()?;
+        let temp_dir = tempfile::TempDir::new()?;
         let temp_file = temp_dir
             .path()
             .join(format!("uv-trampoline-{}.exe", std::process::id()));
@@ -418,6 +445,7 @@ pub fn windows_python_launcher(
         fs_err::remove_file(temp_file)?;
 
         Ok(launcher)
+    }
 }
 
 #[cfg(all(test, windows))]
@@ -569,7 +597,7 @@ if __name__ == "__main__":
 
     /// Signs the given binary using `PowerShell`'s `Set-AuthenticodeSignature` with a temporary certificate.
     fn sign_authenticode(bin_path: impl AsRef<Path>) {
-        let temp_dir = tempfile::tempdir().expect("Failed to create temporary directory");
+        let temp_dir = tempfile::TempDir::new().expect("Failed to create temporary directory");
         let temp_pfx =
             create_temp_certificate(&temp_dir).expect("Failed to create self-signed certificate");
 

crates/uv-trampoline/src/bounce.rs

@@ -36,8 +36,8 @@ use crate::{error, format, warn};
 const RT_RCDATA: u16 = 10;
 
 /// Resource IDs for the trampoline metadata
-const RESOURCE_TRAMPOLINE_KIND: &str = "UV_TRAMPOLINE_KIND\0";
-const RESOURCE_PYTHON_PATH: &str = "UV_PYTHON_PATH\0";
+const RESOURCE_TRAMPOLINE_KIND: windows::core::PCWSTR = windows::core::w!("UV_TRAMPOLINE_KIND");
+const RESOURCE_PYTHON_PATH: windows::core::PCWSTR = windows::core::w!("UV_PYTHON_PATH");
 
 /// The kind of trampoline.
 enum TrampolineKind {
@@ -58,15 +58,13 @@ impl TrampolineKind {
 }
 
 /// Safely loads a resource from the current module
-fn load_resource(resource_id: &str) -> Option<Vec<u8>> {
+fn load_resource(resource_id: windows::core::PCWSTR) -> Option<Vec<u8>> {
     // SAFETY: winapi calls; null-terminated strings; all pointers are checked.
     unsafe {
-        let mut resource_id_null_term = resource_id.encode_utf16().collect::<Vec<_>>();
-        resource_id_null_term.push(0);
         // Find the resource
         let resource = FindResourceW(
             None,
-            windows::core::PCWSTR(resource_id_null_term.as_ptr()),
+            resource_id,
             windows::core::PCWSTR(RT_RCDATA as *const _),
         );
         if resource.is_invalid() {