Frommi / miniz_oxide

Rust replacement for miniz
MIT License
174 stars 49 forks source link

Use Rust API directly from flate2 #16

Closed matklad closed 5 years ago

matklad commented 6 years ago

An interesting problem was reported at https://gitter.im/rust-lang/rust?at=5a57cc7183152df26d5d5cfc.

Basically, miniz-sys and miniz_oxide_c_api expose the same C-API, so you can't link them both into the same process. So it might make sense to bypass C-API in flate2 and avoid polluting linker's namespace.

oyvindln commented 6 years ago

Yeah, that was the goal, the current way it's used was to make it easy to switch between the back-ends without . Though a fix for now, there is a no_c_export feature in miniz_oxide_c_api, which among other things makes the exported functions mangled (there is a macro that adds the no_mangle attribute to the functions if that feature is disabled), so they should not conflict. Maybe this feature should be enabled by default, or added in the dependencies section of flate2, I will add a PR there now. Ideally that macro ought to disable "extern C" as well, but I had some trouble putting "extern C" in a macro, maybe someone else knows whether that's possible.

Frommi commented 6 years ago

There seems to be an easy way to choose features of dependencies now that if I remember correctly wasn't there a few months ago. So it may be good idea to advertise no_c_export on main readme and add that to flate2.

oyvindln commented 6 years ago

Added a PR to to flate2 to use the no_c_export feature. I wasn't able to trigger the error myself, so I can't guarantee that it will fix the problem, but hopefully it will.

oyvindln commented 5 years ago

Fixed in flate2 now https://github.com/alexcrichton/flate2-rs/pull/202