-
Notifications
You must be signed in to change notification settings - Fork 187
Complete the zlib-rs support without the need for C-bindings #524
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR completes zlib-rs support by adding set_dictionary and set_level methods that call zlib-rs directly without requiring C-bindings. The implementation maintains separate counters to track total input/output bytes excluding dictionary data, consistent with the existing C backend approach.
Key Changes:
- Added native
set_dictionaryandset_levelsupport for zlib-rs backend - Implemented separate total_in/total_out tracking to exclude dictionary bytes from counts
- Updated zlib-rs dependency from version 0.5.3 to 0.5.4
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Cargo.toml | Updated zlib-rs dependency from 0.5.3 to 0.5.4 to support new API features |
| src/mem.rs | Added set_dictionary methods for zlib-rs backend and refactored set_level to support both any_zlib and zlib-rs; updated test configuration for error message test |
| src/ffi/zlib_rs.rs | Added total_in/total_out tracking fields, implemented set_dictionary and set_level methods, and updated error handling to use error_message() from inner state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9199797 to
fec67eb
Compare
joshtriplett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for refactoring and cleaning up.
|
Thanks a lot! I rebased #525 on top of main, acting as regression protection. During our previous attempt to release When that is available, I think a new release should be attempted. |
Because we no longer depend on it since bc2f339 (GitoxideLabs#2370). For context on the associated TODO this completes and removes, see: - rust-lang/flate2-rs#520 (superseded) - rust-lang/flate2-rs#524 - rust-lang/flate2-rs#525
This PR adds
set_dictionaryandset_levelto thezlib-rsbackend, which is now used directly from Rust, without a detour through C-bindings.Followed up by #525 .