search

WasmEdge / wasmedge-rust-sdk

Embed WasmEdge functions in a Rust host app
Apache License 2.0
30 stars 15 forks source link

Fix static build to link agains zstd #91

Closed jprendes closed 1 year ago

jprendes commented 1 year ago

It looks like upstream WasmEdge has zstd as a new dependency. This is not currently captured in the build script, which results in a linking error when using the static feature. This PR also changes wasmedge-sys minimum requirement from 1.7 to 1.7.4 since wasmedge-sdk is not compatible with wasmedge-sys 1.7.2 (due to the u64 -> Duration change in here)

The missing dependency should have been captured by the rust-static-lib CI workflow, but it seems that workflow has never been triggered: https://github.com/WasmEdge/wasmedge-rust-sdk/actions/workflows/rust-static-lib.yml This PR makes no attempt at resolving that.

juntao commented 1 year ago

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.

Commit 034e9187da10f063ad6a2308ffbafde342482401

Key changes in the patch:

  1. The version of wasmedge-sys in Cargo.toml has been changed from 0.17 to 0.17.4.
  2. The "zstd" library has been added to the list of dependencies in build.rs.

Potential problems:

  1. It is difficult to determine if the version change in wasmedge-sys is necessary without more context. It would be helpful to understand why the version was specifically changed to 0.17.4.
  2. Adding the "zstd" library as a dependency could introduce new issues. It is important to test the static build thoroughly to ensure that all dependencies are linked correctly and that there are no conflicts or compatibility issues with other dependencies.
  3. Without the rest of the codebase, it is not possible to determine if there are any other potential problems or conflicts with these changes. It is important to review the entire codebase and consider the implications of these changes on the overall project.
jprendes commented 1 year ago

@Mossaka, this should fix the issues in https://github.com/containerd/runwasi/pull/395

hydai commented 1 year ago

@apepkuss Let's tag a new version to ship this on crates.io

apepkuss commented 1 year ago

@apepkuss Let's tag a new version to ship this on crates.io

Yeah, I'll release a new version. Before that, I'll check the rust-static-lib workflow first as @jprendes mentioned it's not triggered. Thanks!

apepkuss commented 1 year ago

@jprendes wasmedge-sdk v0.13.2 and wasmedge-sys v0.17.5 have been released. Please check them out. Thanks!