fix(jsoo): don't ignore linkall

Signed-off-by: Hugo Heuzard <[email protected]>

CHANGES.md

@@ -81,6 +81,8 @@ Unreleased
 - Fix *js_of_ocaml* separate compilation rules when `--enable=effects`
   or `--enable=use-js-string` is used. (#6714, #6828, @hhugo)
 
+- Fix *js_of_ocaml* separate compilation in presence of linkall (#6832, @hhugo)
+
 - Remove spurious build dir created when running `dune init proj ...` (#6707,
   fixes #5429, @gridbugs)
 

src/dune_rules/command.mli

@@ -108,3 +108,9 @@ end
     command in directory [dir]. *)
 val expand :
   dir:Path.t -> 'a Args.t -> string list Action_builder.With_targets.t
+
+(** [expand_no_targets ~dir args] interprets the command line arguments [args]
+    to produce corresponding strings, assuming they will be used as arguments to
+    run a command in directory [dir]. *)
+val expand_no_targets :
+  dir:Path.t -> Args.without_targets Args.t -> string list Action_builder.t

src/dune_rules/dune_rules.ml

@@ -17,6 +17,7 @@ module Modules = Modules
 module Module_compilation = Module_compilation
 module Exe_rules = Exe_rules
 module Lib_rules = Lib_rules
+module Jsoo_rules = Jsoo_rules
 module Obj_dir = Obj_dir
 module Merlin_ident = Merlin_ident
 module Merlin = Merlin

src/dune_rules/exe.ml

@@ -209,8 +209,12 @@ let link_js ~name ~loc ~obj_dir ~top_sorted_modules ~link_args ~promote
   in
   let src = exe_path_from_name cctx ~name ~linkage:Linkage.byte_for_jsoo in
   let linkall =
-    ignore link_args;
-    Action_builder.return false
+    Action_builder.bind link_args ~f:(fun cmd ->
+        let open Action_builder.O in
+        let+ l =
+          Command.expand_no_targets ~dir:(Path.build (CC.dir cctx)) cmd
+        in
+        List.exists l ~f:(String.equal "--linkall"))
   in
   Jsoo_rules.build_exe cctx ~loc ~obj_dir ~in_context ~src ~top_sorted_modules
     ~promote ~link_time_code_gen ~linkall

src/dune_rules/jsoo/jsoo_rules.ml

@@ -72,6 +72,49 @@ end = struct
       | name, false -> [ "--disable"; name ])
 end
 
+module Version = struct
+  type t = int * int
+
+  let of_string s : t option =
+    let s =
+      match
+        String.findi s ~f:(function
+          | '+' | '-' | '~' -> true
+          | _ -> false)
+      with
+      | None -> s
+      | Some i -> String.take s i
+    in
+    try
+      match String.split s ~on:'.' with
+      | [] -> None
+      | [ major ] -> Some (int_of_string major, 0)
+      | major :: minor :: _ -> Some (int_of_string major, int_of_string minor)
+    with _ -> None
+
+  let compare (ma1, mi1) (ma2, mi2) =
+    match Int.compare ma1 ma2 with
+    | Eq -> Int.compare mi1 mi2
+    | n -> n
+
+  let impl_version bin =
+    let open Memo.O in
+    let* _ = Build_system.build_file bin in
+    Memo.of_reproducible_fiber
+    @@ Process.run_capture_line Process.Strict bin [ "--version" ]
+    |> Memo.map ~f:of_string
+
+  let version_memo =
+    Memo.create "jsoo-version" ~input:(module Path) impl_version
+
+  let jsoo_version path =
+    let open Memo.O in
+    let* jsoo = path in
+    match jsoo with
+    | Ok jsoo_path -> Memo.exec version_memo jsoo_path
+    | Error e -> Action.Prog.Not_found.raise e
+end
+
 let install_jsoo_hint = "opam install js_of_ocaml-compiler"
 
 let in_build_dir ~sctx ~config args =
@@ -219,10 +262,13 @@ let link_rule cc ~runtime ~target ~obj_dir cm ~flags ~linkall
            (Super_context.js_of_ocaml_flags sctx ~dir flags))
       |> Action_builder.map ~f:Config.of_flags
     and+ cm = cm
+    and+ linkall = linkall
     and+ libs = Resolve.Memo.read (Compilation_context.requires_link cc)
     and+ { Link_time_code_gen_type.to_link; force_linkall } =
       Resolve.read link_time_code_gen
-    and+ force_linkall2 = linkall in
+    and+ jsoo_version =
+      Action_builder.of_memo (Version.jsoo_version (jsoo ~dir sctx))
+    in
     (* Special case for the stdlib because it is not referenced in the
        META *)
     let stdlib =
@@ -247,8 +293,7 @@ let link_rule cc ~runtime ~target ~obj_dir cm ~flags ~linkall
         (in_build_dir ~sctx ~config
            [ "stdlib"; "std_exit" ^ Js_of_ocaml.Ext.cmo ])
     in
-    let linkall = force_linkall || force_linkall2 in
-    ignore linkall;
+    let linkall = force_linkall || linkall in
     Command.Args.S
       [ Deps
           (List.concat
@@ -258,6 +303,13 @@ let link_rule cc ~runtime ~target ~obj_dir cm ~flags ~linkall
              ; all_other_modules
              ; [ std_exit ]
              ])
+      ; As
+          (match (jsoo_version, linkall) with
+          | Some version, true -> (
+            match Version.compare version (5, 1) with
+            | Lt -> []
+            | Gt | Eq -> [ "--linkall" ])
+          | None, _ | _, false -> [])
       ]
   in
   let spec = Command.Args.S [ Dep (Path.build runtime); Dyn get_all ] in

src/dune_rules/jsoo/jsoo_rules.mli

@@ -8,6 +8,14 @@ module Config : sig
   val all : t list
 end
 
+module Version : sig
+  type t = int * int
+
+  val of_string : string -> t option
+
+  val compare : t -> t -> Ordering.t
+end
+
 val build_cm :
      Super_context.t
   -> dir:Path.Build.t

test/blackbox-tests/test-cases/jsoo/inline-tests.t/run.t

@@ -7,10 +7,10 @@ Run inline tests using node js
   $ dune runtest
   inline tests (Byte)
   inline tests (Byte)
-  inline tests (JS)
-  inline tests (JS)
   inline tests (Native)
   inline tests (Native)
+  inline tests (JS)
+  inline tests (JS)
 
   $ dune runtest --profile release
   inline tests (JS)

test/blackbox-tests/test-cases/jsoo/jsoo-config.t/run.t

@@ -2,13 +2,13 @@ tests js_of_ocaml conigs
 
   $ dune build bin/bin1.bc.js bin/bin2.bc.js bin/bin3.bc.js --display short
    js_of_ocaml bin/.bin1.eobjs/jsoo/bin1.bc.runtime.js
+   js_of_ocaml bin/.bin2.eobjs/jsoo/bin2.bc.runtime.js
+   js_of_ocaml bin/.bin3.eobjs/jsoo/bin3.bc.runtime.js
    js_of_ocaml .js/use-js-string/stdlib/std_exit.cmo.js
    js_of_ocaml .js/use-js-string/stdlib/stdlib.cma.js
         ocamlc lib/.library1.objs/byte/library1.{cmi,cmo,cmt}
-   js_of_ocaml bin/.bin2.eobjs/jsoo/bin2.bc.runtime.js
    js_of_ocaml .js/!use-js-string/stdlib/std_exit.cmo.js
    js_of_ocaml .js/!use-js-string/stdlib/stdlib.cma.js
-   js_of_ocaml bin/.bin3.eobjs/jsoo/bin3.bc.runtime.js
    js_of_ocaml .js/default/stdlib/std_exit.cmo.js
    js_of_ocaml .js/default/stdlib/stdlib.cma.js
         ocamlc bin/.bin1.eobjs/byte/dune__exe__Bin1.{cmi,cmti}

test/blackbox-tests/test-cases/jsoo/no-check-prim.t/run.t

@@ -61,13 +61,13 @@ Compilation using jsoo
       ocamlopt lib/.x.objs/native/x__.{cmx,o}
         ocamlc lib/.x.objs/byte/x.{cmi,cmo,cmt}
       ocamlopt lib/.x.objs/native/x__Y.{cmx,o}
-        ocamlc bin/.technologic.eobjs/byte/z.{cmi,cmo,cmt}
       ocamlopt lib/.x.objs/native/x.{cmx,o}
+        ocamlc bin/.technologic.eobjs/byte/z.{cmi,cmo,cmt}
         ocamlc lib/x.cma
-        ocamlc bin/.technologic.eobjs/byte/technologic.{cmi,cmo,cmt}
       ocamlopt lib/x.{a,cmxa}
-   js_of_ocaml bin/technologic.bc.js
+        ocamlc bin/.technologic.eobjs/byte/technologic.{cmi,cmo,cmt}
       ocamlopt lib/x.cmxs
+   js_of_ocaml bin/technologic.bc.js
   $ dune build --display short bin/technologic.bc.js @install --profile release
         ocamlc lib/.x.objs/byte/x__.{cmi,cmo,cmt}
         ocamlc lib/.x.objs/byte/x__Y.{cmi,cmo,cmt}

test/blackbox-tests/test-cases/jsoo/public-libs.t/run.t

@@ -3,11 +3,11 @@ Compilation of libraries with pulic-names
   $ dune build --display short
         ocamlc a/.a.objs/byte/a.{cmi,cmo,cmt}
    js_of_ocaml b/.main.eobjs/jsoo/main.bc.runtime.js
-   js_of_ocaml .js/default/stdlib/std_exit.cmo.js
-   js_of_ocaml .js/default/stdlib/stdlib.cma.js
       ocamlopt a/.a.objs/native/a.{cmx,o}
         ocamlc b/.main.eobjs/byte/dune__exe__Main.{cmi,cmti}
         ocamlc a/a.cma
+   js_of_ocaml .js/default/stdlib/std_exit.cmo.js
+   js_of_ocaml .js/default/stdlib/stdlib.cma.js
       ocamlopt a/a.{a,cmxa}
         ocamlc b/.main.eobjs/byte/dune__exe__Main.{cmo,cmt}
    js_of_ocaml a/.a.objs/jsoo/default/a.cma.js

test/expect-tests/jsoo_tests.ml

@@ -0,0 +1,48 @@
+open Stdune
+module Jsoo_rules = Dune_rules.Jsoo_rules
+
+let%expect_test _ =
+  let test s l =
+    match Jsoo_rules.Version.of_string s with
+    | None -> print_endline "Could not parse version"
+    | Some version ->
+      let c = Jsoo_rules.Version.compare version l in
+      let r =
+        match c with
+        | Eq -> "="
+        | Lt -> "<"
+        | Gt -> ">"
+      in
+      print_endline r
+  in
+  (* equal *)
+  test "5.0.1" (5, 0);
+  [%expect {| = |}];
+  test "5.0.0" (5, 0);
+  [%expect {| = |}];
+  test "5.0" (5, 0);
+  [%expect {| = |}];
+  test "5" (5, 0);
+  [%expect {| = |}];
+  test "5.0+1" (5, 0);
+  [%expect {| = |}];
+  test "5.0~1" (5, 0);
+  [%expect {| = |}];
+  test "5.0+1" (5, 0);
+  [%expect {| = |}];
+  test "5.0.1+git-5.0.1-14-g904cf100b0" (5, 0);
+  [%expect {| = |}];
+
+  test "5.0.1" (5, 1);
+  [%expect {| < |}];
+  test "5.0" (5, 1);
+  [%expect {| < |}];
+  test "5.1.1" (5, 0);
+  [%expect {| > |}];
+  test "5.1" (5, 0);
+  [%expect {| > |}];
+  test "4.0.1" (5, 0);
+  [%expect {| < |}];
+  test "5.0.1" (4, 0);
+  [%expect {| > |}];
+  ()