Skip to content

Conversation

@SersemPeca
Copy link
Contributor

This PR rewrites our compression logic to use ruzstd instead of zeekstd, since ruzstd can be build for wasm targets

@SersemPeca SersemPeca force-pushed the feat/compress-with-ruzstd branch 2 times, most recently from e552279 to 968c722 Compare August 28, 2025 13:01
@SersemPeca SersemPeca changed the title feat: rewrite the compression logic to use ruzstd instead of zeekstd Support wasm32-wasip1 as a target Aug 28, 2025
@SersemPeca SersemPeca requested a review from Madman10K August 28, 2025 17:24
Copy link
Member

@alehander92 alehander92 left a comment

Choose a reason for hiding this comment

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

we don't need a wasm-compatible writer for now, only reader, but if it's easy to support as an option, that's good

@nickysn
Copy link
Collaborator

nickysn commented Aug 28, 2025

What about other targets, that are neither x86_64, nor wasm32? E.g. aarch64, etc.

@SersemPeca
Copy link
Contributor Author

A valid comment. I'll change it so that we fall back to the rust rewrite of zstd only when we build explicitly for wasm32 targets

@SersemPeca SersemPeca force-pushed the feat/compress-with-ruzstd branch 3 times, most recently from 694c5df to 688a563 Compare August 29, 2025 10:35
Copy link
Member

@alehander92 alehander92 left a comment

Choose a reason for hiding this comment

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

ok, seems that now it should preserve the old behavior with no wasm

just need to update the vesion in Cargo.toml

@SersemPeca SersemPeca requested a review from alehander92 August 29, 2025 13:34
@SersemPeca SersemPeca force-pushed the feat/compress-with-ruzstd branch from 4c32397 to 1a16f67 Compare August 29, 2025 13:39
@alehander92 alehander92 merged commit 6e9f03a into master Sep 2, 2025
2 checks passed
@alehander92
Copy link
Member

merging: @nickysn reminded us correctly to test if zeekstd works with aarch64: TODO separately

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.

4 participants