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

Preserve permissions when untarring and unzipping #32

Merged
merged 3 commits into from
Oct 5, 2021
Merged

Conversation

iffy
Copy link
Contributor

@iffy iffy commented Oct 5, 2021

  • I've added a test and watched it fail, then pass. The test ensures that zippy does the same thing as tar and unzip regarding permissions.
  • I have not tested on Windows or Linux, just macOS
  • The test data file could be made more complex. Right now it only has a single script.sh file in it with u+x permission.

@guzba
Copy link
Owner

guzba commented Oct 5, 2021

Thanks for PR! Looks good. I will confirm Windows locally too and also look into default permissions when composing a new zip / tar programmatically.

@guzba guzba merged commit cb24656 into guzba:master Oct 5, 2021
@guzba
Copy link
Owner

guzba commented Oct 5, 2021

Currently, all files extracted from some of my test zips on Windows end up as read only (setPermissions with no fpUserWrite). Zip file permissions also generally appear to be absent or ignored by Windows even if present (only work on Unix). This complicates the behavior needed by Zippy when extracting a .zip from Windows on Mac or Linux for since it will currently appear to have no permissions.

@guzba
Copy link
Owner

guzba commented Oct 5, 2021

On the tarball front, my Nim-1.4.2.tar.gz which I've used for testing only has {fpGroupWrite, fpGroupRead, fpOthersWrite, fpOthersRead} on many (all?) files, whcih does not include the fpUserWrite permission. This results in Windows making all the files read only.

On Windows, only the readonly flag is changed, depending on fpUserWrite permission. https://nim-lang.org/docs/os.html#setFilePermissions%2Cstring%2Cset%5BFilePermission%5D

@guzba
Copy link
Owner

guzba commented Oct 5, 2021

I think 4d1fb10 resolves the .zip side.

@guzba
Copy link
Owner

guzba commented Oct 5, 2021

I think #head now has zip and tar ok on Windows.

@guzba
Copy link
Owner

guzba commented Oct 5, 2021

Changes released in zippy 0.7.1

@iffy
Copy link
Contributor Author

iffy commented Oct 5, 2021

Thanks for the quick turnaround!

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.

2 participants