Skip to content
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

Fix extracted files permissions #609

Closed
wants to merge 2 commits into from

Conversation

gilfree
Copy link

@gilfree gilfree commented Apr 3, 2024

Fix #608: This commit fixes the extracted file permissions when using .extract/.extract_files in addition
to when using the unpack cli command.

The fix is by overriding the _extract_member method of zip file, and adding permissions fix there.

The permissions fix was moved from the unpack to the new method.

This commit fixes the extracted file permissions
when using .extract/.extract_files in addition
to when using the unpack cli command.

The fix is by overriding the _extract_member
method of zip file, and adding permissions fix
there.

The permissions fix was moved from the unpack to
the new method.
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.23%. Comparing base (1e00742) to head (f3f09e5).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #609      +/-   ##
==========================================
+ Coverage   71.03%   71.23%   +0.19%     
==========================================
  Files          13       13              
  Lines        1084     1088       +4     
==========================================
+ Hits          770      775       +5     
+ Misses        314      313       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gilfree gilfree changed the title fix: Fix extracted files permissions Fix extracted files permissions Apr 3, 2024
@gilfree gilfree marked this pull request as ready for review April 3, 2024 10:48
@gilfree
Copy link
Author

gilfree commented Jul 29, 2024

Any plans to merge this? Anything I can do?

@agronholm
Copy link
Contributor

In what situation would this change ever be used? The WheelFile class in this library is not part of any public API.

@gilfree
Copy link
Author

gilfree commented Jul 30, 2024

In what situation would this change ever be used? The WheelFile class in this library is not part of any public API.

Ok, then. Your choice, I just thought you wanted this change, will close this PR.

Thanks for your work :)

@gilfree gilfree closed this Jul 30, 2024
@agronholm
Copy link
Contributor

Yeah, sorry, this was a brain fart of mine. First I OK'd this PR, then I turned it down. I guess I was just too busy with everything else and got confused, especially given how much time had passed. I don't know why I initially thought there would be a point in making the change to WheelFile itself at this point. If this change is to be made to a wheel API, it should be made to a public API, like the one I've been working on here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WheelFile extract does not preserve permissions
2 participants