Preserve zip files permissions #371
Conversation
|
I've tested it locally with the |
os/src/ZipOps.scala
Outdated
| val relPath = path.subRelativeTo(dest).toString | ||
| if (os.zip.shouldInclude(relPath, excludePatterns, includePatterns)) { | ||
| val entry = zipFS.getPath(relPath) | ||
| val permissions = Files.getPosixFilePermissions(entry) |
There was a problem hiding this comment.
I think you should use the more low-level Files.getAttribute(entry, "zip:permissions") here.
In case the zip entry does not have permission information, Files.getAttribute(entry, "zip:permissions") would be empty and we could skip os.perms.set.
If using Files.getPosixFilePermissions, according to the doc,
The "permissions" attribute is not optional in the "posix" view so a default set of permissions are used for entries that do not have access permissions stored in the Zip file. The default set of permissions are
OWNER_READ
OWNER_WRITE
GROUP_READ
There was a problem hiding this comment.
But the unzipped file would get some default permissions even if we dont set them, right?
With this approach we at least can set it with the "defaultPermissions" property.
Currently I set it to empty
There was a problem hiding this comment.
When there's no permission information stored with the zip entry, we want it to have the default permissions set by the user's os/file system/umask when unzipped right, not the zipfs "defaultPermissions"? Also the zipfs "defaultPermissions" is never empty. When it's null/unset it's set to [OWNER_READ, OWNER_WRITE, GROUP_READ]. So we should check with Files.getAttribute(entry, "zip:permissions") instead, and skip the os.perms.set part if it's empty.
There was a problem hiding this comment.
Yeah, you are right, I'm not super familiar with *nix permissions.
Fixed now, looks good?
There was a problem hiding this comment.
So I didn't have a correct understanding of umask either. There're no default permissions in Unix. Applications set permissions to files they create. Umask will then mask some of the permissions. What you did before was totally fine. https://askubuntu.com/questions/44542/what-is-umask-and-how-does-it-work
|
Closing based on discussion in #356 (comment) |
…d zip source code from Apache Ant (#374) ### Vendored code Follow the discussion in #356, this adds support for permissions and symlinks in `os.zip`/`unzip` with vendored zip source code from Apache Ant. The vendored source code is generated by the `os.zip.apacheAntZipSource` task and put in `os/zip`. It's shaded with the package renamed from `org.apache.tools.zip` to `os.shaded_org_apache_tools_zip`. `scala-steward.conf` was added and configured to run `os.zip.apacheAntZipSource` on `org.apache.ant:ant` updates. ### Features This brings support for permissions and symlinks to `zip` (for creating new zips, not modifying existing ones), `zip.stream` and `unzip`. As for modifying existing zips, we would still have to rely on `jdk.zipfs` which does not support symlinks. | | file permissions | symlinks | | --- | --- | --- | | `os.zip.open` | if Java Runtime Version >= 14 | | | `os.zip` (create new) | ✅ | ✅ | | `os.zip` (modify existing) | if Java Runtime Version >= 14 | | | `os.zip.stream` | ✅ | ✅ | | `os.unzip` | ✅ | ✅ | | `os.unzip.stream` | | | ### TODO - [ ] **(Advice needed)** make sure we comply with Apache Ant's license to include the code here. Would appreciate opinions on this as I'm not an expert. - [ ] **(Advice needed)** make `ZipOps` JVM only - [x] tests - [x] make sure things don't break on Windows - [x] add permission support to modifying existing zips with `jdk.zipfs` like what @sake92 did in #371
2 participants
Seems like we can open the ZIP after it is created, and then add the permissions to the ZIP entries.
See the docs, it needs to be done through java's zip filesystem https://docs.oracle.com/en/java/javase/17/docs/api/jdk.zipfs/module-summary.html
Same with unzip, we extract the files and then dig into the ZIP via zip filesystem to fetch the perms.
Seems like it works. ¯\(ツ)/¯