| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
- Implement ZstdDict, ZstdCompressor, and ZstdDecompressor on top of the zstd-safe crate - Wire the module into stdlib_module_defs (gated off Android/wasm32) - Mark test_zstd_multithread_compress as expected failure when libzstd lacks multi-threading support
`PyMutex` is built on `RawCellMutex` on single-threaded targets (iOS, Android, wasm32) and is not `Sync`, so a `static PyMutex<...>` fails to compile. `PyTypeRef` is also not `Send` by default, which compounds the issue. Replace the static parameter-type registry with class-name comparison (`"CompressionParameter"` / `"DecompressionParameter"`). The two names originate from the `compression.zstd` module we ship, so identity vs. name comparison is equivalent in practice. `set_parameter_types` now just validates that its arguments are type objects. Verified locally: - `cargo build -p rustpython-stdlib` (default features): clean. - `cargo check -p rustpython-stdlib --no-default-features --features host_env` (simulates iOS by dropping the `threading` feature): clean. - `cargo run -- -m test test_zstd`: 119 tests pass, 1 expected failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RustPython's clippy lint set rejects `std::` imports for items that also
live in `core::` or `alloc::`. Switch every such reference in `zstd.rs`:
- `std::ffi::{c_int, CStr}` -> `core::ffi::{c_int, CStr}`
- `std::fmt::*` -> `core::fmt::*`
- `std::slice::from_raw_parts` -> `core::slice::from_raw_parts`
- `std::borrow::Cow` -> `alloc::borrow::Cow`
Also clear up other clippy findings under `-D warnings`:
- Drop the redundant `obj.clone()` on the last use of `obj` in
`parse_zstd_dict_arg`.
- Collapse `.map(|n| n.get()).unwrap_or(0)` to `.map_or(0, |n| n.get())`
in the two dict-id readers.
- Replace `&*work_data` with `&work_data` (auto-deref).
- Factor the `(Option<Digested>, Option<PyRef<ZstdDict>>)` return shape
into a `DictLoadResult<D>` type alias to satisfy `type_complexity`.
- Replace the remaining `std::mem::transmute<u32, ZSTD_cParameter>` /
`ZSTD_dParameter` in `get_param_bounds` with the existing safe
`c_param_enum` / `d_param_enum` helpers; surfaces a clear "invalid
parameter" `ValueError` instead of relying on UB-adjacent transmutes
for unknown ints.
Verified locally:
- `cargo clippy -p rustpython-stdlib --no-deps -- -Dwarnings`: clean
(only the pre-existing `socket.rs::sock_wait` warning remains).
- `cargo run -- -m test test_zstd`: 119 pass, 1 expected failure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous run's `Lint` job failed with a `401 Unauthorized` from reviewdog hitting the GitHub API (a transient CI/permissions issue, unrelated to this PR's diff). Pushing an empty commit to re-run CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
{"subject": "Format zstd module and extend spell-check ignores", "body": "- Apply rustfmt formatting to zstd.rs\n- Add additional spell-checker ignore entries for zstd identifiers"}
Co-authored-by: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com>
{"subject": "Remove rustdoc comments from zstd module", "body": "- Strip doc comments from internal items in _zstd module\n- Convert a few API-doc comments into regular implementation notes where appropriate"}
{"subject": "Mark zstd load_dict as unsafe and document invariant", "body": "- Add # Safety docs requiring PyRef<ZstdDict> to outlive the context\n- Wrap calls in load_compressor_dict and load_decompressor_dict with SAFETY comments"}
{
"subject": "refactor(zstd): release GIL during (de)compress and tighten dict-load safety",
"body": "- Wrap compress/decompress loops in vm.allow_threads to release the GIL\n- Replace load_*_dict helpers with build_*_state that assemble the full state, making the load_dict safety invariants structural\n- Switch constructor args from OptionalArg to OptionalOption and use try_to_value instead of the pyobj_to_i32/arg_or_none helpers\n- Add check_sample_sizes_match with overflow-safe summing for train_dict/finalize_dict"
}
- Have set_parameter_types stash the CompressionParameter/DecompressionParameter classes as private _zstd module attributes instead of validating and discarding them - check_wrong_param_kind now compares key classes by identity against the registered type, matching CPython's Py_TYPE check, and skips when unregistered - Error message names the actual key type as an attribute
- Raise TypeError (not RuntimeError), matching CPython, when both `level` and `options` are passed to ZstdCompressor - Parse set_pledged_input_size's argument before taking the state lock so a re-entrant __index__ can't deadlock, and fix the off-by-one in its bound message to match CPython Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the remaining `// ===...` section separators from zstd.rs per the project comment guidelines, drop the unused `zstd` crate dependency (only zstd-safe/zstd-sys are used) and declare zstd-safe's std feature explicitly instead of relying on the zstd crate's feature unification, and convert a doc comment on #[extend_class] to a regular comment.
Replace the C-library bindings (zstd-safe/zstd-sys) with Trifecta Tech Foundation's libzstd-rs-sys (pinned git rev), a c2rust translation of upstream libzstd. The module now calls its C-API-compatible functions through small owning RAII wrappers (CCtx/DCtx/CDict/DDict) with the same drop-order invariants as before. The new backend builds zstdmt_compress in, so nb_workers bounds are (0, 256) and test_zstd_multithread_compress passes for real; drop the now-obsolete expectedFailureIf marker.
The zero-cap probe iteration handed libzstd a 1-byte buffer and the post-loop truncation discarded whatever it emitted, while the input counter had already advanced past the bytes that produced it — every decompress(..., max_length=0) call with pending output silently dropped one byte. The suite never caught it because max_length=0 was only exercised against zero-output (skippable) frames. Match CPython's mechanism instead: probe with a zero-size output buffer, so libzstd consumes input without emitting and never reports frame completion with un-emitted content. Add a regression snippet covering lossless zero-cap probes (content, truncated, and skippable frames), verified byte-exact against CPython.
- Derive the compression/decompression parameter ids from libzstd's own `ZSTD_cParameter`/`ZSTD_dParameter` constants instead of restating them as literals. The crate declares both as `#[repr(transparent)]` newtypes over a private `u32` with no accessor, `From` impl or `Deref`, so the id is read back through that guaranteed layout. - Drop the redundant `name = ` on `#[pyattr(once)] fn zstd_version` and `fn zstd_version_number`; the function names already match. - Use `CStr::to_str` for the version string. libzstd returns a fixed ASCII `MAJOR.MINOR.RELEASE` literal, so there is nothing for a lossy conversion to repair. - Replace the `DICT_TYPE_*` int constants with a `DictType` enum carrying the pinned discriminants, decoded at the Python boundary by `DictType::from_marker`. - Explain the non-trivial assertions: why `level_bounds`' `expect` cannot fire, and why a NULL context allocation panics rather than raising `MemoryError`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`COMP_MODE_CONTINUE`/`FLUSH_BLOCK`/`FLUSH_FRAME` become a `CompressMode` enum with pinned discriminants, decoded at the Python boundary by `CompressMode::from_int` and mapped to libzstd by `CompressMode::end_directive`. `CompressorState::last_mode` now holds the enum, so the states `set_pledged_input_size` accepts are checked by the type system rather than by comparing ints. `flush()` keeps rejecting `CONTINUE` and out-of-range modes with its own message rather than the one `compress()` uses, so the Python-visible errors are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verified each claim the previous two commits added, against the pinned libzstd-rs-sys rev, CPython's `Modules/_zstd/` and `Lib/test/test_zstd.py`, and corrected the ones that did not hold: - The `DictType` discriminants come from CPython's `dictionary_type` enum, not a type named `DictType`, and `test_zstd` never hand-builds a valid `(zdict, marker)` tuple — it pins the accepted range from the outside with `(zd, -1)` and `(zd, 3)` rejection cases. - `#[repr(transparent)]` guarantees layout, not validity invariants. The transmute is sound because every bit pattern is a valid `u32`, which holds only in the enum-to-int direction; say so, and note that this is why `c_param_enum` decodes untrusted ids with an explicit match. - The id is unreachable through the crate's public API only in the sense that no accessor exists; the derived `Debug` does print it. - Undigested dictionary loading is not uniformly lazy: `ZSTD_DCtx_loadDictionary` digests eagerly and rejects corrupted content at construction time, while `ZSTD_CCtx_loadDictionary` accepts it. - `CompressMode`'s discriminants are libzstd's `ZSTD_e_continue`/ `ZSTD_e_flush`/`ZSTD_e_end` values. `CCtx::create`/`DCtx::create` now return `PyResult` and raise `MemoryError` like CPython's `_zstd` does, rather than panicking. The old justification for the panic claimed a `vm` reference would have to be threaded through every construction site; there are two, and both already had one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found by reviewing the module against CPython's `Modules/_zstd/` and the
pinned libzstd-rs-sys sources; each item below was reproduced before the
fix and re-checked after.
- `train_dict`/`finalize_dict` sized their output buffer with an
infallible `Vec::with_capacity`/`vec![]` from a caller-supplied
`dict_size`, so `train_dict([b'x'], 2**62)` aborted the interpreter
instead of raising. Reserve fallibly and raise `MemoryError`, as CPython
does.
- A digested dictionary was always built at `ZSTD_CLEVEL_DEFAULT`. Since
`ZSTD_CCtx_refCDict` takes its parameters from the CDict, that silently
discarded the requested level: `level=19` with `as_digested_dict`
compressed a test corpus to 39817 bytes instead of 31951. Build the
CDict at the compressor's effective level, tracked through `level=` and
the `options` dict like CPython's `self->compression_level`.
- A bare `ZstdDict` was loaded as digested in both directions; CPython
compresses with an undigested dictionary by default and only
decompresses with a digested one. Give `DictLoader` a per-direction
default.
- Neither compressor nor decompressor reset its session after an error,
leaving the object permanently broken — every later call failed with
"Operation not authorized at current processing stage" or re-reported
the original corruption on valid input. Reset as CPython's error paths
do, including restoring `last_mode` to `FLUSH_FRAME`.
- `apply_options` range-checked every parameter, rejecting values libzstd
and CPython accept: 0 ("use the default") for `window_log`, `strategy`,
`window_log_max` and friends, any non-zero value for the boolean flags,
and the clamped `nb_workers`/`job_size`/`overlap_log`. Only
`compression_level` needs the check — libzstd clamps that one silently,
which is why `test_compress_parameters` requires a `ValueError` — so
everything else now goes to libzstd, whose error code already maps to
the same message.
- `ZstdDict` accepted content shorter than 8 bytes; CPython rejects it up
front regardless of `is_raw`.
- `compress`/`decompress` refused `data` as a keyword and
`get_param_bounds` required `is_compress` as one, both contrary to
CPython's clinic signatures.
- The decompressor allocated a full 128 KiB scratch buffer even when
`max_length` capped output far below that.
`test_zstd` (119 tests) and the snippet test pass; clippy is clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The branch dropped the `skipIf(not SUPPORT_MULTITHREADING)` guard from `test_zstd_multithread_compress`. That guard can never fire against this backend: libzstd-rs-sys compiles multi-threaded compression in unconditionally — its Cargo.toml has no `zstdmt`-style feature — so `CompressionParameter.nb_workers.bounds()` is never `(0, 0)` and `SUPPORT_MULTITHREADING` is always true. Removing the decorator therefore changed nothing except to put a modification of a vendored CPython test in the diff, and it would turn into a hard failure rather than a skip if a future backend ever lacked multi-threading. Restore the file so the PR touches no CPython test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drop libzstd-rs-sys and the platform cfg that kept _zstd off Android and wasm. The module now calls rusty_zstd for streaming compression, decompression, frame headers, and dictionary training. Assisted-by: Grok:grok-4.7
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
Replace rusty_zstd 0.2.5 with libzstd-rs-sys rev 7afaf79f14f4. Streaming compression, decompression, frame queries, and dictionary training go through that crate. zstd_version is 1.5.8. Assisted-by: Grok:grok-4.7
| Back | FazBrowse Home | New Git URL |
One of checkbox below must be checked.
Summary
Add _zstd, the accelerator behind compression.zstd, using the pure-Rust crate rusty_zstd 0.2.5. The module is registered on every target, including Android and wasm.
ZstdCompressor, ZstdDecompressor, and ZstdDict cover streaming and one-shot compression, decompression with max_length, frame headers, pledged content size, dictionaries (digested, undigested, and prefix), and train_dict / finalize_dict.
Checked locally with a release rustpython:
The workspace clippy feature set and an i686 build were not run here. The 32-bit parameter bounds are selected with target_pointer_width.
c5962fbd98 was written with Grok 4.7 (Assisted-by: Grok:grok-4.7). Earlier commits keep their original trailers.
— commented by Grok