-
Notifications
You must be signed in to change notification settings - Fork 185
Enable window_bits and gzip functions for zlib-rs backend #520
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
8f2cdbb to
b4993c4
Compare
977d847 to
d8a67e7
Compare
|
@folkertdev I'd need your help here 🙏. I think this PR shows that |
|
I would have expected this PR to break CI given that zlib-rs does not currently expose the API to implement those features? I'll need to make a quick release exposing some additional things. We have the functionality, just not the API. |
Actually the test doesn't check for set_dictionary and set_level for that reason, but everything else is exposed. Once we have access to these via the Rust API, the test can be adjusted to also validate them. In theory, it could then be copy-pasted for each of the other implementations. In any case, thanks for your help, it's much appreciated! |
|
Some fun kind of undocumented behavior: the native zlib backends count the dictionary length towards |
|
Maybe @jongiddy has more context, as this question is way over my head. Knowing that some models as built into the GH chat window have access to a lot of context and can do some thinking/planning, I thought maybe sharing their output here can be useful. Maybe not, I really can't even decide that 😅. GPT 4.1 MiniShort answer: probably a deliberate bookkeeping choice in flate2 rather than a spec bug in zlib. zlib's C implementation treats the preset dictionary as part of the stream history and increments its internal total_in/total_out counters to reflect that the stream now "contains" those bytes; flate2's counters reflect only the bytes that were actually passed to the compress()/decompress() calls by the caller (i.e., explicit input), and so do not add in the implicit dictionary length. Why that difference exists (summary)
Is it deliberate?
What I recommend
Suggested issue title/body (you can paste/edit this): Title: Body: Repro:
Observed:
Expected / question:
Workaround: If you’d like, I can help you draft a minimal reproducible Rust snippet to include in the issue (or a short test) showing the differing totals for the two backends. Which backend(s) and flate2 version are you testing? |
Several functions were unnecessarily restricted to C-based zlib backends despite working with pure Rust backends like
zlib-rs.Changes
Changed feature gate from
any_zlibtoany_implfor:Compress::new_with_window_bits()- Custom window size for compressionCompress::new_gzip()- Gzip-formatted compressionDecompress::new_with_window_bits()- Custom window size for decompressionDecompress::new_gzip()- Gzip-formatted decompressionThese functions work across all backends because:
make()methods already acceptwindow_bitsparameterwindow_bits + 16convention supported universallyDictionary functions (
set_dictionary,set_level) remainany_zlib-only as they require direct stream manipulation not exposed in zlib-rs stable API.Tests
Added validation for newly enabled functions:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.