Draft: Use InflateLib to add Deflate64 support to ZIP decoding - #2806
Draft: Use InflateLib to add Deflate64 support to ZIP decoding#2806dunhor wants to merge 19 commits into
Conversation
Yep. I was looking at simplifying the Windows build, at least, by removing the precompiled dependency archives and moving them to vcpkg. I don't know y'all's appetite for that, but it would make dependency management a little bit more straightforward/discoverable. Specifically only for CI, FWIW. |
DHowett
left a comment
There was a problem hiding this comment.
Notes from our chat: Should we add an inflatelib version to archive_version? Is the only thing keeping it in draft consensus? I'm sure we can get Tobias or Tim to have a look or make a pass over it 🙂
Right... yes... thanks for the reminder 😅 |
Okay, added. This raises a question: should we add an |
It certainly would be aligned with the other decompression suites! I'm in support of it. Probably easy to make it a constant string with a macro or something too :) |
We already have that - that's what I'm using in the implementation here. Looking at the other implementations got me thinking that it's probably better to have a function in the event that the header version != the version that is loaded at runtime. |
|
Adding |
This is a follow-up to #2325.
A while back we wrote and released microsoft/inflatelib, which allows decompression ("inflation") of Deflate and Deflate64 encoded data with its main purpose being to add Deflate64 support to libarchive. The API of the library is modeled after zlib (specifically after
inflate, notinflateBack) and its implementation is based off the implementation in dotnet'sSystem.IO.Compressionnamespace (though it slowly diverged a little for performance reasons, mainly how allocations are handled).Since InflateLib is still considered "prerelease" (we'll probably finalize the API once the implementation here is settled on), I'm issuing this as a draft PR for now, though if there's no contention, there's nothing that should prevent it from getting merged. I've also set the various "enable" switches in the
CMakeListsandautoconffiles to be off by default for now for this same reason. I've also added a vcpkg manifest to the root of the repo to make my - and hopefully others - lives easier for dependency management. E.g. pass-DCMAKE_TOOLCHAIN_FILE=%VCPKG_ROOT%during CMake initialization. It's my understanding that these files don't need to be in the root of the repo - you just need to pass more arguments to CMake to get things to work correctly - so I can move them if desired. This can also be used to help with dependency management in CI (I know @DHowett had plans for this in the past).The implementation itself is virtually identical to the zlib path, since the two APIs are virtually identical. As such, I've also factored out the encryption handling logic since it's the same for both & added a few comments and extra variables for my own benefit of understanding the operations better.