Incorporate @compnerd's patch for Windows support

timsneath commented 2 weeks ago

It would be great if we could incorporate https://github.com/marmelroy/Zip/pull/246/files, which would enable this to work on Windows. Today there's no well-maintained ZIP implementation that includes Windows support, and you're achingly close!

fpseverino commented 2 weeks ago

I've implemented the code from the original PR in #10, but I'm having some trouble with the CI, as I am not able to pass a build of zlib. Maybe @compnerd could give us a hand?

timsneath commented 1 week ago

I think (from the actions log), the problem is that tar only supports the Zip format in Windows 11 builds and later, and the GitHub Actions windows-latest image is Windows Server 2022, which is based on Windows 10 21H2.

It looks like the runner image does include 7Zip, so you can probably replace the tar line with something like:

7z x archive.7z -opath/to/dir

fpseverino commented 1 week ago

Ok, now zilb is correctly downloaded and linked.

All tests succeed except for a important one. I'll do what I can in the coming days.

We are almost there! 😃

fpseverino commented 1 week ago

That test I previously mentioned fails because of two reasons:

The zip file used in the test has colons in some of the dir/file names it contains, which are not valid characters in Windows file names.
- This zip file was zipped with vapor-community/Zip itself. Changing it to an identical file zipped with Finder seems to fix the issue.
- I'm unable to test if a file (containing colons in its name) zipped with vapor-community/Zip on Windows can be unzipped (or even if it can be zipped). If it's zipped in Linux/macOS it can't be unzipped on Windows.
Permissions are very different in Windows. Copilot helped me converting the POSIX ones, but there's a single file to which it fails to set the correct permissions.

Right now the solution to the colons problem is avoiding it, and the one for the permissions problem is an ugly hack that doesn't even work all the time. Of course more work is needed. If you know more on Windows permissions I would appreciate your help!

fpseverino commented 1 week ago

I've managed to remove colons in file names and I've removed any permissions setting specific to Windows, now all the test succeed.

I've marked the PR as ready to review and requested some reviews. When it gets merged I'll do a new patch release, but I think that more real world testing will be needed.

vapor-community / Zip

Incorporate @compnerd's patch for Windows support #9