Skip to content

Conversation

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Oct 20, 2024

So far it's failing:

--- a/_build/.sandbox/6975479b7ea39eedbfcff9056c4b3b15/default/test/blackbox-tests/test-cases/wasmoo/inline-tests.t/run.t
+++ b/_build/.sandbox/6975479b7ea39eedbfcff9056c4b3b15/default/test/blackbox-tests/test-cases/wasmoo/inline-tests.t/run.t.corrected
@@ -15,8 +15,17 @@ Run inline tests using Node.js
   $ dune runtest --profile release
   inline tests (Native)
   inline tests (Native)
-  inline tests (Wasm)
-  inline tests (Wasm)
+  File "wasm/dune", lines 1-6, characters 0-142:
+  1 | (library
+  2 |  (name inline_tests_wasm)
+  3 |  (js_of_ocaml (wasm_files wasm.wat) (submodes wasm))
+  4 |  (inline_tests
+  5 |   (modes js)
+  6 |   (backend fake_backend)))
+  wasm_of_ocaml: unknown option '--linkall'.
m  Usage: wasm_of_ocaml [COMMAND] …
+  Try 'wasm_of_ocaml --help' for more information.
+  [1]

vouillon and others added 14 commits October 20, 2024 16:25
Signed-off-by: Jérôme Vouillon <[email protected]>
Signed-off-by: Jérôme Vouillon <[email protected]>
Signed-off-by: Jérôme Vouillon <[email protected]>
Signed-off-by: Jérôme Vouillon <[email protected]>
Signed-off-by: Jérôme Vouillon <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
_
Signed-off-by: Rudi Grinberg <[email protected]>
@vouillon
Copy link
Member

@rgrinberg The CI should pass now (I have updated wasm_of_ocaml to accept the --linkall flag.


Without modifying any dune files, I would like the flexibility to either:

  • Always compile to JavaScript, with the option to also compile to Wasm.
  • Or choose between JavaScript and Wasm as the target.

This could potentially be managed through a profile, and also possibly by detecting whether wasm_of_ocaml is installed. However, I'm unsure how to achieve this using dune's regular modes, which seem fairly static. Do you have any suggestions?

Currently, we use different profiles to determine whether to run the js_of_ocaml test suite with js_of_ocaml or wasm_of_ocaml. Given that programs built with wasm_of_ocaml don’t work everywhere (e.g., Safari doesn’t yet support Wasm GC), and considering that wasm_of_ocaml introduces additional dependencies (it requires the installation of Binaryen), it seems important to me to offer flexibility to the end user.

@rgrinberg
Copy link
Member Author

Did you push or release your changes? I still see:

--- a/_build/.sandbox/bc7fe4ecdd5c35f4e5f942fc63925c8a/default/test/blackbox-tests/test-cases/wasmoo/inline-tests.t/run.t
+++ b/_build/.sandbox/bc7fe4ecdd5c35f4e5f942fc63925c8a/default/test/blackbox-tests/test-cases/wasmoo/inline-tests.t/run.t.corrected
@@ -15,8 +15,17 @@ Run inline tests using Node.js
   $ dune runtest --profile release
   inline tests (Native)
   inline tests (Native)
-  inline tests (Wasm)
-  inline tests (Wasm)
+  File "wasm/dune", lines 1-6, characters 0-142:
+  1 | (library
+  2 |  (name inline_tests_wasm)
+  3 |  (js_of_ocaml (wasm_files wasm.wat) (submodes wasm))
+  4 |  (inline_tests
+  5 |   (modes js)
+  6 |   (backend fake_backend)))
+  wasm_of_ocaml: unknown option '--linkall'.
m  Usage: wasm_of_ocaml [COMMAND] …
+  Try 'wasm_of_ocaml --help' for more information.
+  [1]

@hhugo
Copy link
Collaborator

hhugo commented Nov 6, 2024

This can be closed

@hhugo hhugo closed this Nov 6, 2024
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.

4 participants