-
Notifications
You must be signed in to change notification settings - Fork 803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only preserve the executable bit #1743
Conversation
crates/uv-extract/src/stream.rs
Outdated
fs_err::set_permissions(&path, Permissions::from_mode(mode))?; | ||
// Create a file with read and write permissions. | ||
// Otherwise, databricks-0.2.dist-info/RECORD in databricks-0.2-py2.py3-none-any.whl has 000 permissions. | ||
let permissions = Permissions::from_mode(mode | 0o660); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Permission seems to lack an api to set read and write without manual bitflags math.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should take umask
into account, see https://github.com/pypa/pip/blob/main/src/pip/_internal/operations/install/wheel.py#L652
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pip reference!
I rewrote it so that we're now never setting the permissions explicitly ourselves, but instead first create the file with the default options and then attach the executable bit if required.
8820655
to
9bddac7
Compare
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.