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

Zip permissions fixes #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

chrissawer
Copy link

Fix a couple of permissions issues with zip files, spotted when using the arduino CLI to install the LiquidCrystal library "arduino-cli lib install LiquidCrystal" which results in inappropriate permissions of 744 for the LiquidCrystal directory and all subdirectories, despite the zip file specifying permissions 755.

Tests with fixes as follows:

  • Add test TestUnixPermissions along with zip and tar test files to check a variety of common permissions in both files and directories. Note that the tar test file passes with current master, but the zip file doesn't. This is due to adding the additional permission decimal 100 in extractor.go, fix is to change this to octal 0100 (or 0o100).
  • Add test TestZipDirectoryPermissions to download the Arduino LiquidCrystal library and check the permissions are correct when it is extracted. In this zip file, the files are listed ahead of their containing directories, so the extract library is creating the directories before it sees what the permissions should be. Fix is to ensure that the code updates the permissions of such directories when they are encountered in the zip file.

These new tests pass on both Linux (Ubuntu 22.04) and Windows 11, go version 1.21.6 in both cases. Note that on Windows there are unrelated test failures which also occur on master.

@chrissawer
Copy link
Author

Looks like the CI system is using Go 1.17.13 and I have used an API (CutPrefix) which was introduced in a later version. I will take a look

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.

None yet

1 participant