From 9a720a87c842a070ecc58f5309c7ecc6a334da67 Mon Sep 17 00:00:00 2001
From: konsti <konstin@mailbox.org>
Date: Tue, 20 Feb 2024 16:41:05 +0100
Subject: [PATCH] Only preserve the executable bit (#1743)

A file in a zip can set arbitrary unix permissions, but we, like pip,
want to preserve only the executable bit and otherwise use the OS
defaults.

This should be faster for wheels with many files since we now avoid the
blocking fs call to set the permissions in most cases.

Fixes #1740.
---
 crates/uv-extract/src/stream.rs | 23 ++++++++++++++++++-----
 crates/uv-extract/src/sync.rs   | 27 ++++++++++++++++-----------
 crates/uv/tests/pip_sync.rs     | 23 +++++++++++++++++++++++
 3 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/crates/uv-extract/src/stream.rs b/crates/uv-extract/src/stream.rs
index b7b6b21f9..7186c8563 100644
--- a/crates/uv-extract/src/stream.rs
+++ b/crates/uv-extract/src/stream.rs
@@ -37,6 +37,7 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
                 }
             }
 
+            // We don't know the file permissions here, because we haven't seen the central directory yet.
             let file = fs_err::tokio::File::create(path).await?;
             let mut writer =
                 if let Ok(size) = usize::try_from(entry.reader().entry().uncompressed_size()) {
@@ -70,12 +71,24 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
                 continue;
             }
 
-            // Construct the (expected) path to the file on-disk.
-            let path = entry.filename().as_str()?;
-            let path = target.as_ref().join(path);
+            let Some(mode) = entry.unix_permissions() else {
+                continue;
+            };
 
-            if let Some(mode) = entry.unix_permissions() {
-                fs_err::set_permissions(&path, Permissions::from_mode(mode))?;
+            // The executable bit is the only permission we preserve, otherwise we use the OS defaults.
+            // https://github.com/pypa/pip/blob/3898741e29b7279e7bffe044ecfbe20f6a438b1e/src/pip/_internal/utils/unpacking.py#L88-L100
+            let has_any_executable_bit = mode & 0o111;
+            if has_any_executable_bit != 0 {
+                // Construct the (expected) path to the file on-disk.
+                let path = entry.filename().as_str()?;
+                let path = target.as_ref().join(path);
+
+                let permissions = fs_err::tokio::metadata(&path).await?.permissions();
+                fs_err::tokio::set_permissions(
+                    &path,
+                    Permissions::from_mode(permissions.mode() | 0o111),
+                )
+                .await?;
             }
         }
     }
diff --git a/crates/uv-extract/src/sync.rs b/crates/uv-extract/src/sync.rs
index 7c5addd8f..0489cf063 100644
--- a/crates/uv-extract/src/sync.rs
+++ b/crates/uv-extract/src/sync.rs
@@ -1,4 +1,3 @@
-use std::fs::OpenOptions;
 use std::path::{Path, PathBuf};
 use std::sync::Mutex;
 
@@ -45,24 +44,30 @@ pub fn unzip<R: Send + std::io::Read + std::io::Seek + HasLength>(
                 }
             }
 
-            // Create the file, with the correct permissions (on Unix).
-            let mut options = OpenOptions::new();
-            options.write(true);
-            options.create_new(true);
+            // Copy the file contents.
+            let mut outfile = fs_err::File::create(&path)?;
+            std::io::copy(&mut file, &mut outfile)?;
 
+            // See `uv_extract::stream::unzip`. For simplicity, this is identical with the code there except for being
+            // sync.
             #[cfg(unix)]
             {
-                use std::os::unix::fs::OpenOptionsExt;
+                use std::fs::Permissions;
+                use std::os::unix::fs::PermissionsExt;
 
                 if let Some(mode) = file.unix_mode() {
-                    options.mode(mode);
+                    // https://github.com/pypa/pip/blob/3898741e29b7279e7bffe044ecfbe20f6a438b1e/src/pip/_internal/utils/unpacking.py#L88-L100
+                    let has_any_executable_bit = mode & 0o111;
+                    if has_any_executable_bit != 0 {
+                        let permissions = fs_err::metadata(&path)?.permissions();
+                        fs_err::set_permissions(
+                            &path,
+                            Permissions::from_mode(permissions.mode() | 0o111),
+                        )?;
+                    }
                 }
             }
 
-            // Copy the file contents.
-            let mut outfile = options.open(&path)?;
-            std::io::copy(&mut file, &mut outfile)?;
-
             Ok(())
         })
         .collect::<Result<_, Error>>()
diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs
index 8c752c951..cb12e347c 100644
--- a/crates/uv/tests/pip_sync.rs
+++ b/crates/uv/tests/pip_sync.rs
@@ -2734,3 +2734,26 @@ fn tar_dont_preserve_mtime() -> Result<()> {
 
     Ok(())
 }
+
+/// Avoid creating a file with 000 permissions
+#[test]
+fn set_read_permissions() -> Result<()> {
+    let context = TestContext::new("3.12");
+    let requirements_in = context.temp_dir.child("requirements.in");
+    requirements_in.write_str("databricks==0.2")?;
+
+    uv_snapshot!(command(&context)
+        .arg("requirements.in"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 1 package in [TIME]
+    Downloaded 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + databricks==0.2
+    "###);
+
+    Ok(())
+}