search

zip-rs / zip-old

Zip implementation in Rust
MIT License
731 stars 204 forks source link

Support zstd in Wasm by disabling default features #367

Closed nickbabcock closed 5 months ago

nickbabcock commented 1 year ago

Use case: creating zstd zips in Wasm

Currently it is not possible to create zstd zips in Wasm due to the default zdict_builder needing additional code that will cause compilation to fail for wasm32-unknown-unknown: https://github.com/gyscos/zstd-rs/issues/210

Since the zip crate's zstd implementation does not use anything from the zdict_builder feature, I've disabled it.

I've gone a step further and disabled the other features as well (arrays and legacy), as they are either unused (arrays) or a legacy format from 7 years ago that predates when the zip format supported zstd.

So far this patch has been working well for the Wasm application, and figured I should upstream it.

Plecra commented 1 year ago

Thanks, good recommendation :) Sadly it's a breaking change so I'll just get through the other PRs so that we can put this in the next update

lolpro11 commented 8 months ago

Hi, this is due to dynamic libs being used during compile. I tried using pure Rust impls of bzip2 and zstd for static linking, and it compiles. Check #429

nickbabcock commented 8 months ago

Hi, this is due to dynamic libs being used during compile. I tried using pure Rust impls of bzip2 and zstd for static linking, and it compiles. Check #429

Hmm, I don't see the need to swap them. I've been running the zstd crate (without default features) fine for the last 10 months in static executables and Wasm

And I don't see how zstud-sys is a Rust impl of zstd if it's just a bindgen wrapper.

lolpro11 commented 8 months ago

Hmm, I don't see the need to swap them. I've been running the zstd crate (without default features) fine for the last 10 months in static executables and Wasm

And I don't see how zstud-sys is a Rust impl of zstd if it's just a bindgen wrapper.

I agree, but zstud-sys doesn't depend on a system lib during runtime. Maybe disable default features for the zstd crate?

nickbabcock commented 8 months ago

I agree, but zstud-sys doesn't depend on a system lib during runtime. Maybe disable default features for the zstd crate?

Yup, that's what this PR does. Since features are additive, I can't remove remove the default features that zip-rs adds for zstd.

That's why I used a forked version of zip-rs with zstd without default features so that it can be used in Wasm.

Ideally, I'd still like to see this PR merged (and zstd updated). I don't see the benefit to zstud-sys

lolpro11 commented 8 months ago

Yup, that's what this PR does. Since features are additive, I can't remove remove the default features that zip-rs adds for zstd.

That's why I used a forked version of zip-rs with zstd without default features so that it can be used in Wasm.

Ideally, I'd still like to see this PR merged (and zstd updated). I don't see the benefit to zstud-sys

What about bzip2-sys?

Pr0methean commented 5 months ago

This repo is no longer maintained. This PR has already been replaced with https://github.com/zip-rs/zip2/pull/8, which has been merged.