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

Yordan/my wit component #6

Open
wants to merge 44 commits into
base: blocksense
Choose a base branch
from

Conversation

yordanmadzhunkov
Copy link
Collaborator

WIP

Yordan Madzhunkov and others added 30 commits July 1, 2024 16:57
cd ~/code/repos/spin/crates/core &&
cd ./tests/core-wasi-test/ &&
cargo build &&
ls -l target/wasm32-wasi/debug/core-wasi-test.wasm &&
cp target/wasm32-wasi/debug/core-wasi-test.wasm /home/yordan/code/repos/spin/crates/core/../../target/test-programs/core-wasi-test.wasm &&
cd ../.. && cargo test
@smanilov
Copy link
Collaborator

Pesho asked me to review this PR, focusing on the server/host side of things. The following is the account of what followed.

I spent an hour and I managed to build only few packages of the ones affected by this PR. I couldn't run any tests. I'm attaching my review notes, but perhaps I'm doing something wrong. Have I missed some crucial setup step that would allow me to build this and run the tests?

Notes:


My eyes hit this PR. They met no README or any sort of instruction of how to proceed.

I built the code. It fails to build. (Actually, realized on a later retry.)

I tried to run the tests. They fail to build. The error message is the following:

  --- stderr
     Compiling proc-macro2 v1.0.86
     Compiling indexmap v2.2.6
     Compiling image v0.24.9
  error[E0658]: use of unstable library feature 'proc_macro_byte_character'
     --> /home/stan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proc-macro2-1.0.86/src/wrapper.rs:871:21
      |
  871 |                     proc_macro::Literal::byte_character(byte)
      |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = note: see issue #115268 <https://github.com/rust-lang/rust/issues/115268> for more information
      = help: add `#![feature(proc_macro_byte_character)]` to the crate attributes to enable
      = note: this compiler was built on 2024-03-17; consider upgrading it if it is out of date

  error[E0658]: use of unstable library feature 'proc_macro_c_str_literals'
     --> /home/stan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/proc-macro2-1.0.86/src/wrapper.rs:898:21
      |
  898 |                     proc_macro::Literal::c_string(string)
      |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = note: see issue #119750 <https://github.com/rust-lang/rust/issues/119750> for more information
      = help: add `#![feature(proc_macro_c_str_literals)]` to the crate attributes to enable
      = note: this compiler was built on 2024-03-17; consider upgrading it if it is out of date

I started looking for new tests introduced by this PR. I found crates/core/tests/test_host_components/empty_ml.rs.

I get "file not included in crate hierarchy" for it, which after investigation seems to be related to the build configuration. It defines an MLHostComponent which is used in crates/trigger/src/lib.rs. Due to the configuration, however, actually, a different MLHostComponent is used there, the one from ml/src/host_component.rs.

So the purpose of empty_ml.rs remains a mystery to me at this point. Is it used in testing, to mock the type?


Skipping through the test_host_components files, I reach crates/ml/Cargo.toml. I try to build just that package with cargo build -p spin-ml. It fails to build with the following message:

error[E0432]: unresolved imports `openvino::Precision`, `openvino::TensorDesc`
  --> crates/ml/src/backend/openvino.rs:10:30
   |
10 | use openvino::{Core, Layout, Precision, TensorDesc};
   |                              ^^^^^^^^^  ^^^^^^^^^^ no `TensorDesc` in the root
   |                              |
   |                              no `Precision` in the root

This is followed with more errors, 14 in total.

So I give up on the spin-ml package and move on.


There are what seem to be drive-by changes until I reach crates/trigger/Cargo.toml. It includes spin-ml as a dependency, so I have no hope that it builds. Still, I run direnv reload, in case I have messed something up and run cargo build -p spin-trigger. I get the same error as before.


I discover crates/world/wit/ml.wit. Comments! How refreshing! crates/world/wit/world.wit includes it, it's not clear to me how to build it, though, so I skip forwards.


I reach examples/http-rust-imagenet/.cargo/config.toml and go there to build that package. This build succeeds! I see that there are also tests in it, but running cargo test fails. The error message is the following:

     Running unittests src/lib.rs (target/wasm32-wasi/debug/deps/http_rust_imagenet-84fec25580a30f29.wasm)
error: test failed, to rerun pass `--lib`

Caused by:
  could not execute process `/home/stan/code/repos/spin/examples/http-rust-imagenet/target/wasm32-wasi/debug/deps/http_rust_imagenet-84fec25580a30f29.wasm` (never executed)

Caused by:
  Exec format error (os error 8)

The next example I discover is examples/spin-timer/Cargo.lock. Strangely, there are changes only to the lock file, but I try to build it anyway. Building fails with 14 jobs. I am not looking into the error message, but I guess the problem is as before.


There are no more files. The review is done!

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.

3 participants