Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add nil backend #1794

Merged
merged 3 commits into from
Apr 28, 2022
Merged

feat: add nil backend #1794

merged 3 commits into from
Apr 28, 2022

Conversation

haraldh
Copy link
Member

@haraldh haraldh commented Apr 27, 2022

This backend calls exec-wasmtime directly via a library call, without using any elf binary and shim.

❯ enarx run --backend nil hello_wasi_snapshot1.wasm
Hello, world!

@enarxbot enarxbot enabled auto-merge (rebase) April 27, 2022 11:39
@github-actions github-actions bot added backend infrastructure General issues around project infrastructure wasm Issues related to WebAssembly labels Apr 27, 2022
@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #1794 (25c8a9f) into main (ea6d853) will increase coverage by 2.33%.
The diff coverage is 70.76%.

❗ Current head 25c8a9f differs from pull request most recent head bfe0286. Consider uploading reports for the commit bfe0286 to get more accurate results

@@            Coverage Diff             @@
##             main    #1794      +/-   ##
==========================================
+ Coverage   54.68%   57.01%   +2.33%     
==========================================
  Files         151      152       +1     
  Lines       10316    10352      +36     
==========================================
+ Hits         5641     5902     +261     
+ Misses       4675     4450     -225     
Impacted Files Coverage Δ
src/backend/direct.rs 65.45% <65.45%> (ø)
crates/exec-wasmtime/src/lib.rs 79.62% <100.00%> (ø)
src/backend/mod.rs 54.54% <100.00%> (+4.54%) ⬆️
src/main.rs 95.00% <100.00%> (+2.14%) ⬆️
src/workldr/exec_wasmtime/mod.rs 78.57% <100.00%> (ø)
src/backend/sgx/hasher.rs 0.00% <0.00%> (ø)
src/backend/sgx/builder.rs 91.97% <0.00%> (ø)
...es/exec-wasmtime/src/loader/configured/platform.rs 84.12% <0.00%> (+6.34%) ⬆️
crates/exec-wasmtime/src/loader/mod.rs 87.80% <0.00%> (+7.31%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea6d853...bfe0286. Read the comment docs.

@npmccallum
Copy link
Member

@platten Note that this makes the "static binary" problem much more acute since the dependency graphs are now overlapping.

@bstrie
Copy link
Contributor

bstrie commented Apr 27, 2022

@npmccallum Can you elaborate on how this makes the static binary problem more acute?

Copy link
Contributor

@bstrie bstrie left a comment

Choose a reason for hiding this comment

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

So AIUI the reason to split out exec-wasmtime into lib and bin crates is so that we can make the lib optional and avoid building wasmtime twice, once for the host and once for musl, since wasmtime takes a while to build. @npmccallum This isn't a new bug in bindeps, because it's expected behavior; bindeps allows you to have both an artifact and a normal dependency via the lib = true key, but there's no facility to make lib = true optional, and it's unclear how you'd even expose that. So this is bindeps working as intended, and is not relevant to the known optional = true bug ( rust-lang/cargo#10526 ).

Instead, the ultimate fix for this would be multidep ( rust-lang/cargo#10061 ), which would allow us to have a single exec-wasmtime crate and rely on it twice from within Cargo.toml, once as an optional direct dependency and once as a bindep. Please leave a FIXME somewhere mentioning that we can undo this split once multidep arrives.

Cargo.toml Outdated Show resolved Hide resolved
src/backend/direct.rs Outdated Show resolved Hide resolved
src/workldr/exec_wasmtime/mod.rs Show resolved Hide resolved
@haraldh
Copy link
Member Author

haraldh commented Apr 27, 2022

Shouldn't we open an issue which tracks the external multidep and a second one depending on this to fix our monorepo?

@bstrie
Copy link
Contributor

bstrie commented Apr 28, 2022

@haraldh #1798 and #1799. Can you add the first as a FIXME comment?

Signed-off-by: Harald Hoyer <harald@profian.com>
This backend calls `exec-wasmtime` directly via a library call, without
using any elf binary.

```
❯ cargo run -- run --backend nil hello_wasi_snapshot1.wasm
Hello, world!
```

Signed-off-by: Harald Hoyer <harald@profian.com>
Signed-off-by: Harald Hoyer <harald@profian.com>
@haraldh
Copy link
Member Author

haraldh commented Apr 28, 2022

@haraldh #1798 and #1799. Can you add the first as a FIXME comment?

@bstrie done

@haraldh haraldh requested a review from bstrie April 28, 2022 15:44
@haraldh haraldh changed the title feat: add direct backend feat: add nil backend Apr 28, 2022
@enarxbot enarxbot merged commit c26aaac into enarx:main Apr 28, 2022
@haraldh haraldh deleted the direct branch July 4, 2022 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend infrastructure General issues around project infrastructure wasm Issues related to WebAssembly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants