Skip to content

Conversation

harshithsaiv
Copy link

This PR adds support for Zlib(level) gated by the optional zlib Cargo feature.

Changes

  • CompressionType

    • New variant: Zlib(u8).
    • Updated Encode/Decode:
      • Levels are clamped to 0..=9 on encode/decode to match flate2 expectations.
  • Block I/O

    • Zlib branches added in write_into, from_reader, and from_file.
    • Compression via flate2::write::ZlibEncoder.
    • Decompression via flate2::read::ZlibDecoder.
  • Blob path

    • blob_tree/compression.rs: MyCompressor added support for zlib
    • blob_tree/mod.rs: BlobTree::open() wires CompressionType::{None, Lz4, Zlib(_)} .
  • Cargo.toml

    • Add optional dependency: flate2 with zlib-rs backend (default-features = false).
    • Introduce zlib feature that enables the dependency.
  • No breaking changes when i ran cargo build --features zlib and cargo test --features zlib

One open point: I clamped levels ≥10 to 9 during encode. Would you prefer rejecting invalid levels instead?

@harshithsaiv
Copy link
Author

Hi @marvin-j97 ,
Is there anything else that I should work on for this issue ? , I see that the build is failing because of files that are going to be removed as you mentioned.

@marvin-j97
Copy link
Contributor

marvin-j97 commented Sep 23, 2025

Hi @marvin-j97 , Is there anything else that I should work on for this issue ? , I see that the build is failing because of files that are going to be removed as you mentioned.

See the last remaining comment about the InvalidCompression variant

@harshithsaiv
Copy link
Author

Hi @marvin-j97 Marvin,
I’m having trouble viewing your latest comment in the PR GitHub isn’t showing it to me. Could you please repost or clarify your feedback so I can address it? Thank you!

@marvin-j97
Copy link
Contributor

Oops the review was not sent

Copy link
Contributor

@marvin-j97 marvin-j97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just rustfmt missing I think

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