Wasm_of_ocaml support #8278

vouillon · 2023-07-26T10:55:50Z

No description provided.

src/dune_rules/jsoo/js_of_ocaml.ml

hhugo · 2023-07-31T09:51:51Z

src/dune_rules/jsoo/js_of_ocaml.ml

         })
    else
      fields
        (let+ flags = Flags.decode
         and+ javascript_files =
           field "javascript_files" (repeat string) ~default:[]
-         in
-         { flags; javascript_files })
+         and+ wasm_files = field "wasm_files" (repeat string) ~default:[] in


We probably need to enable this only for newer dune lang versions. see how it's done above if syntax_version < (3, 0) then

hhugo · 2023-07-31T09:59:21Z

src/dune_rules/jsoo/jsoo_rules.ml

@@ -137,6 +137,8 @@ end

 let install_jsoo_hint = "opam install js_of_ocaml-compiler"

+let install_wasoo_hint = "opam install wasm_of_ocaml-compiler"


has this been released on opam already ?

Not yet. I think we should wait for thinks (wasm_of_ocaml, the WebAssembly Garbage Collection proposal, and related tools) to stabilize a bit.

The hint is a bit misleading then. Should it say something like
opam pin add wasm_of_ocaml-compiler https://github.com/ocaml-wasm/wasm_of_ocaml.git
@rgrinberg, any opinion ?

If there is no good hint to give, then I would just not give any. You can always add a hint in the future when the situation is more clear.

hhugo · 2023-07-31T10:23:33Z

src/dune_rules/jsoo/jsoo_rules.ml

+             ((match ctarget with
+              | JS -> []
+              | Wasm -> wasm_runtime_files libs)
+             @ jsoo_runtime_files libs))


does the wasm target depend on the jsoo_runtime_files ?

The JavaScript runtime files are parsed to get purity information about primitives. We want also to include JavaScript files with no annotation, like lib/virtualdom.compiled.js in package virtual_dom. And I have just added a way to link JavaScript code referenced by the Wasm runtime code.

@hhugo can we run the wasm compilation with sandboxing? Or are there still issues with source maps?

@vouillon, do you plan to support sourcemap for the wasm target ? What's the content of the js file when target is wasm ?

It would be nice to support them eventually. Probably not a high priority, though.
What are the issues with source maps?
The contents of the js file is currently a mix of js code generated by the js_of_ocaml linker and of javascript code directly copied from a file. I should certainly improve on this, and then one could add a sourcemap to this file.

The issue with source map is that it might need to access sources to embed them. We do not currently track this kind of dependencies. Preventing us from using sandboxing.

hhugo · 2023-07-31T10:26:02Z

src/dune_rules/jsoo/jsoo_rules.ml

+    exe_rule ~ctarget cc ~javascript_files ~wasm_files:[] ~src ~target ~flags
+    >>= Super_context.add_rule sctx ~loc ~dir ~mode
+  | _, Wasm ->
+    exe_rule ~ctarget cc ~javascript_files ~wasm_files ~src ~target ~flags


Shouldn't it be an error if Separate_compilation, Wasm ? instead of silently ignoring cmode.

The idea is that you can switch to producing Wasm code just by setting the target, without having to specify a compilation mode as well.
Do you think it would be better to change Super_context.js_of_ocaml_compilation_mode to not default to Separate_compilation when the target is wasm?

Have you considered making wasm part of compilation_mode ?
I'm also happy to keep this as is and revisit later if needed.

Actually, this is what I did at first. But this is orthogonal. I plan to implement separate compilation in the middle term.

Leave it as is then.

Maybe add a comment in the code saying that separate compilation is coming later.

hhugo · 2023-08-01T14:53:49Z

src/dune_rules/jsoo/js_of_ocaml.ml

@@ -161,24 +184,31 @@ module Env = struct
    fields
    @@ let+ compilation_mode =
         field_o "compilation_mode" Compilation_mode.decode
+       and+ target = field_o "target" Target.decode


This should probably be guarded based on dune lang version as well

rgrinberg · 2023-08-02T15:46:38Z

src/dune_rules/super_context.mli

@@ -38,6 +38,8 @@ val js_of_ocaml_runtest_alias : t -> dir:Path.Build.t -> Alias.Name.t Memo.t
 val js_of_ocaml_compilation_mode :
  t -> dir:Path.Build.t -> Js_of_ocaml.Compilation_mode.t Memo.t

+val js_of_ocaml_target : t -> dir:Path.Build.t -> Js_of_ocaml.Target.t Memo.t


Can you inline this function in the one place where it's being used?

rgrinberg · 2023-08-02T15:48:28Z

In general the code looks really good to me. Awaiting for approval from Hugo. Also don't forget to:

Add some tests
Update the CHANGES entry
Add something to the manual

hhugo · 2023-08-03T07:12:28Z

The dune lang constraint will need to be updated to 3.11.0 (dune.3.10) as been released alrady)

rgrinberg · 2023-09-07T09:42:11Z

@hhugo do you want this to be ready for 3.11?

vouillon · 2023-09-07T11:35:39Z

I think this can wait.

I'm wondering whether this should work likemodes. It makes a lot of sense to be able to generate both JavaScript and Wasm code.

Alizter · 2023-11-24T18:38:56Z

otherlibs/dune-rpc/private/types.ml

@@ -35,7 +35,7 @@ end
 module Version = struct
  type t = int * int

-  let latest = (3, 10)


If you rebase you will find we are on 3.13 now.

… Wasm. Signed-off-by: Jérôme Vouillon <jerome.vouillon@gmail.com>

Signed-off-by: Jérôme Vouillon <jerome.vouillon@gmail.com>

hhugo · 2023-11-28T20:16:39Z

src/dune_rules/test_rules.ml

+            | Some targets -> List.length (Js_of_ocaml.Target.Set.to_list targets) > 1
+            | None -> false
+          in
+          if multiple_targets


I don't really understand the logic here, why do we have Js_of_ocaml.Ext.exe when targets is wasm only ?
This kind of pattern seems fragile/ error prone if with ever add a new target

I would like to be able to simply switch between producing a JavaScript file or a Wasm binary by using a dune profile. Since we are using the JavaScript file names in dune files or in HTML files, it seems natural to use the same JavaScript file name in both cases.

Since Wasm is not widely available, one also needs to be able to generate both a Javascript file and a Wasm binary, so that a web server can send one or the other based on the browser. In this case, we need two different file names.

Do you see a better way to deal with both of these use cases?

hhugo · 2023-11-28T20:20:06Z

src/dune_rules/jsoo/js_of_ocaml.ml

@@ -91,6 +93,7 @@ module In_buildable = struct
  type t =
    { flags : Ordered_set_lang.Unexpanded.t Flags.t
    ; javascript_files : string list
+    ; wasm_files : string list


I think it would be nice to have a comment explaining why wasm_of_ocaml reuses the (js_of_ocaml ..) field instead of being its own thing

OlivierNicole · 2024-02-21T15:21:53Z

I found out that adding (targets wasm) in the (js_of_ocaml ...) stanza of an executable doesn’t work, only modifying environments seems possible. Wouldn’t it be good to be able to specify the wasm target for executables too?

vouillon marked this pull request as ready for review July 26, 2023 10:56

rgrinberg requested a review from hhugo July 31, 2023 09:42

hhugo reviewed Jul 31, 2023

View reviewed changes

src/dune_rules/jsoo/js_of_ocaml.ml Outdated Show resolved Hide resolved

hhugo reviewed Jul 31, 2023

View reviewed changes

vouillon force-pushed the wasm branch 2 times, most recently from a9a82a2 to 7b05a53 Compare August 1, 2023 14:29

hhugo reviewed Aug 1, 2023

View reviewed changes

vouillon force-pushed the wasm branch from 7b05a53 to ec0a80a Compare August 1, 2023 15:36

rgrinberg reviewed Aug 2, 2023

View reviewed changes

rgrinberg added the jsoo label Aug 2, 2023

vouillon force-pushed the wasm branch from ec0a80a to 92c6d48 Compare August 3, 2023 21:14

vouillon mentioned this pull request Aug 29, 2023

Runtime implementation status ocaml-wasm/wasm_of_ocaml#5

Open

72 tasks

hhugo mentioned this pull request Nov 19, 2023

[BUG] Build fails with Error: Unknown field target ocaml-wasm/wasm_of_ocaml#8

Closed

vouillon force-pushed the wasm branch from 33ac360 to f0d3a2b Compare November 24, 2023 18:23

Alizter reviewed Nov 24, 2023

View reviewed changes

vouillon added 3 commits November 28, 2023 16:49

Add a switch to js_of_ocaml options to select between targeting JS or…

1dcc5a2

… Wasm. Signed-off-by: Jérôme Vouillon <jerome.vouillon@gmail.com>

Add wasm_runtime and wasm_files options

6f17be5

Signed-off-by: Jérôme Vouillon <jerome.vouillon@gmail.com>

Possibility to generate both JavaScript and Wasm code

ac04ab0

Signed-off-by: Jérôme Vouillon <jerome.vouillon@gmail.com>

vouillon force-pushed the wasm branch from f0d3a2b to ac04ab0 Compare November 28, 2023 19:47

hhugo reviewed Nov 28, 2023

View reviewed changes

rickyvetter mentioned this pull request Apr 23, 2024

Separate compilation ocaml-wasm/wasm_of_ocaml#36

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wasm_of_ocaml support #8278

Wasm_of_ocaml support #8278

vouillon commented Jul 26, 2023

hhugo Jul 31, 2023

hhugo Jul 31, 2023

vouillon Aug 1, 2023

hhugo Aug 1, 2023 •

edited

Alizter Aug 1, 2023

hhugo Jul 31, 2023

vouillon Jul 31, 2023

rgrinberg Aug 2, 2023

hhugo Aug 3, 2023

vouillon Aug 3, 2023

hhugo Aug 3, 2023

hhugo Jul 31, 2023

vouillon Aug 1, 2023

hhugo Aug 1, 2023

vouillon Aug 1, 2023

hhugo Aug 1, 2023

hhugo Aug 1, 2023

hhugo Aug 1, 2023

rgrinberg Aug 2, 2023

rgrinberg commented Aug 2, 2023

hhugo commented Aug 3, 2023

rgrinberg commented Sep 7, 2023

vouillon commented Sep 7, 2023

Alizter Nov 24, 2023

hhugo Nov 28, 2023

vouillon Nov 28, 2023

hhugo Nov 28, 2023

OlivierNicole commented Feb 21, 2024

		@@ -137,6 +137,8 @@ end

		let install_jsoo_hint = "opam install js_of_ocaml-compiler"

		let install_wasoo_hint = "opam install wasm_of_ocaml-compiler"

Wasm_of_ocaml support #8278

Are you sure you want to change the base?

Wasm_of_ocaml support #8278

Conversation

vouillon commented Jul 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhugo Aug 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgrinberg commented Aug 2, 2023

hhugo commented Aug 3, 2023

rgrinberg commented Sep 7, 2023

vouillon commented Sep 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlivierNicole commented Feb 21, 2024

hhugo Aug 1, 2023 •

edited