Skip to content

Switch to libz-rs-sys for zlib implementation #5562

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

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

coolreader18
Copy link
Member

This is a rust-only reimplementation of the libz-sys api, which I discovered from this blog post. It can build without any C toolchain, which makes it a good pick for us when we're targeting windows and wasm, and the performance isn't anything to scoff at (see blog post). Also, the zlib feature now enables the entire zlib module, which I feel like just makes more sense.

@youknowone
Copy link
Member

if it is a pure rust implementation, do we still need a zlib feature?

@coolreader18
Copy link
Member Author

It's a reasonably heavy dependency (~500KB), so I figured it might be nice to still have the option to turn it off.

@youknowone
Copy link
Member

We don't suggest feature for every library which can be a redundant dependencies. If the binary size matters, turning off the entire stdlib feature and consist their own well-selected (or we can suggest a few pre-selected) set of libraries will make more sense.

@coolreader18
Copy link
Member Author

Fair enough.

@coolreader18 coolreader18 force-pushed the zlib-rs branch 2 times, most recently from 6e740d9 to 3887f7c Compare February 26, 2025 02:55
@youknowone
Copy link
Member

youknowone commented Feb 26, 2025

Probably we can remove zlib feature from rustpython crate but add a lot of features for each library in rustpython-stdlib as a user interface to support easy library on/off. (not a scope of this pr)

@coolreader18 coolreader18 merged commit 4468dcb into RustPython:main Feb 26, 2025
11 checks passed
@coolreader18 coolreader18 deleted the zlib-rs branch February 26, 2025 05:19
@arihant2math arihant2math mentioned this pull request Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants