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.
This commit is contained in:
konsti 2024-02-20 16:41:05 +01:00 committed by GitHub
parent 469ccf826f
commit 9a720a87c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 57 additions and 16 deletions

View file

@ -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?;
}
}
}

View file

@ -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>>()

View file

@ -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(())
}