Github actions: Windows

[cknitt]

Currently, the Github actions Windows build is disabled because

1. Building ninja from source fails because configure.py cannot find cl.exe

ninja.tar.gz not availble in CI mode
Traceback (most recent call last):
  File "D:\a\rescript-compiler\rescript-compiler\ninja\configure.py", line 322, in <module>
    if platform.msvc_needs_fs():
  File "D:\a\rescript-compiler\rescript-compiler\ninja\configure.py", line 85, in msvc_needs_fs
    popen = subprocess.Popen(['cl', '/nologo', '/?'],
  File "C:\hostedtoolcache\windows\Python\3.9.12\x64\lib\subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\hostedtoolcache\windows\Python\3.9.12\x64\lib\subprocess.py", line 1[42](https://github.com/rescript-lang/rescript-compiler/runs/6474262702?check_suite_focus=true#step:6:43)0, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2] The system cannot find the file specified

2. When disabling building ninja from source and using the prebuilt binary instead, the Windows build does not work in a deterministic way. It fails most of the time with

FAILED: stdlib-406/printexc.cmj
../win32/bsc.exe -bs-read-cmi -bs-cmi -bs-cmj -no-keep-locs -no-alias-deps -bs-no-version-header -bs-no-check-div-by-zero -nostdlib  -bs-cross-module-opt -make-runtime -w -9-3-106 -warn-error A -I others  -I stdlib-406  stdlib-406/printexc.ml
File "stdlib-406/printexc.ml", line 21, characters 15-16:
Error: Unbound module Obj

See also https://github.com/rescript-lang/rescript-compiler/pull/5376#issuecomment-1128493426.

3. In the case where the build succeeds, the tests fail with

.....................FFFFFF....................................................................................................................EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE...............................................................
==============================================================================
Error: ounit_tests_main.ml:17:ounit_ffi_error_debug_test.ml:4:File "ounit_ffi_error_debug_test.ml", line 64, characters 8-15

Invalid_argument("Unix.fork not implemented")
------------------------------------------------------------------------------
...
==============================================================================
Failure: ounit_tests_main.ml:2:ounit_path_tests.ml:5:File "ounit_path_tests.ml", line 103, characters 4-11

OUnit: expected: ./..\..\..\..\/ but got: ../../../..
------------------------------------------------------------------------------
...

[cknitt]

As for 1., I was able to solve this by passing --platform mingw to ninja's configure script.

MinGW is included out of the box in Github actions.

[cknitt]

As for 2., the error occurs in this line

   {j|File "$(s)", line $(linum), characters $(start)-$(finish): $(msg)|j}

@bobzhang @cristianoc Can it be that Obj is referenced indirectly here, e.g., via Obj.magic?

Like here? https://github.com/rescript-lang/rescript-compiler/blob/f12823dc8491ae87b7797483935365953c92872b/jscomp/frontend/ast_utf8_string_interp.ml#L360

[cristianoc]

As for 2., the error occurs in this line

   {j|File "$(s)", line $(linum), characters $(start)-$(finish): $(msg)|j}

@bobzhang @cristianoc Can it be that Obj is referenced indirectly here, e.g., via Obj.magic?

Like here?

https://github.com/rescript-lang/rescript-compiler/blob/f12823dc8491ae87b7797483935365953c92872b/jscomp/frontend/ast_utf8_string_interp.ml#L360

Looks like this is the case. The question is why module Obj is not found. Notice obj.ml should have been already compiled by the time we get there. Is the output verbose enough to check whether it has been compiled?

[cknitt]

Actually in the cases where the build fails, obj.ml is not compiled before printexc.ml.

Compare

• Failed Windows build: https://github.com/rescript-lang/rescript-compiler/runs/6475670846?check_suite_focus=true#step:6:1
• Successful macOS build: https://github.com/rescript-lang/rescript-compiler/runs/6465960470?check_suite_focus=true

So it seems that if no explicit dependency is specified, the build order is somewhat random / platform-dependent, and looking at release.ninja, printexc does not depend on obj (compare e.g. to parsing above which does):

o stdlib-406/parsing.cmj : cc_cmi stdlib-406/parsing.ml | stdlib-406/array.cmj stdlib-406/lexing.cmj stdlib-406/obj.cmj stdlib-406/parsing.cmi $bsc others
o stdlib-406/parsing.cmi : cc stdlib-406/parsing.mli | stdlib-406/lexing.cmi stdlib-406/obj.cmi stdlib-406/pervasives.cmj $bsc others
o stdlib-406/printexc.cmj : cc_cmi stdlib-406/printexc.ml | stdlib-406/printexc.cmi $bsc others
o stdlib-406/printexc.cmi : cc stdlib-406/printexc.mli | stdlib-406/pervasives.cmj $bsc others

[cristianoc]

Actually in the cases where the build fails, obj.ml is not compiled before printexc.ml.

Compare

• Failed Windows build: https://github.com/rescript-lang/rescript-compiler/runs/6475670846?check_suite_focus=true#step:6:1
• Successful macOS build: https://github.com/rescript-lang/rescript-compiler/runs/6465960470?check_suite_focus=true

So it seems that if no explicit dependency is specified, the build order is somewhat random / platform-dependent, and looking at release.ninja, printexc does not depend on obj (compare e.g. to parsing above which does):

o stdlib-406/parsing.cmj : cc_cmi stdlib-406/parsing.ml | stdlib-406/array.cmj stdlib-406/lexing.cmj stdlib-406/obj.cmj stdlib-406/parsing.cmi $bsc others
o stdlib-406/parsing.cmi : cc stdlib-406/parsing.mli | stdlib-406/lexing.cmi stdlib-406/obj.cmi stdlib-406/pervasives.cmj $bsc others
o stdlib-406/printexc.cmj : cc_cmi stdlib-406/printexc.ml | stdlib-406/printexc.cmi $bsc others
o stdlib-406/printexc.cmi : cc stdlib-406/printexc.mli | stdlib-406/pervasives.cmj $bsc others

How about adding an explicit dependency at the place where Obj is use indirectly. i.e. where Longident.Ldot (Lident "Obj", "magic") is used also add let _ = Obj.magic that should make the dependency work hopefully

[cristianoc]

When the build fails for analogous reason from time to time on my M1 Mac, it fails on curry.ml which is dependent on Caml_array:

let%private sub = Caml_array.sub

Perhaps whatever the %private tranformation does, is not picked up by the dependency analysis.

[cknitt]

How about adding an explicit dependency at the place where Obj is use indirectly.

printexc.ml is not the only one using template strings though, arg.ml is, too.

Alternatively we could

• Rewrite those places to use string concatenation with ^ instead of template strings.
• Generate the release.ninja differently. From what I can see, pervasives is added as a dependency for all sources in https://github.com/rescript-lang/rescript-compiler/blob/8337c997f0df6bc866491f00d937856e3b06d222/scripts/ninja.js#L1138 - maybe we could do the same for obj?

What do you think?

[cristianoc]

A general solution such as adding obj as dependency would be nice. Does it compile OK with it?

[cknitt]

@cristianoc I now just changed the template strings to string concatenation in printexc.ml because that was the easiest/quickest solution for me. Windows builds are now passing reliably. 🎉

[bobzhang]

Obj.magic is used a lot internally, I am thinking of add a builtin function __unsafe_cast so that we don't need any magic building stdlibs

[cknitt]

@bobzhang That sounds good, but could we still get #5388 merged in the meantime so that I can finish the Windows CI?

[cristianoc]

For context, the change amounts to adding one line to pervasives.ml and .mli:

external __unsafe_cast : 'a -> 'b = "%identity"

[cknitt]

@cristianoc Ok, plus changing

Ldot (Lident "Obj", "magic")

to

Ldot (Lident "Pervasives", "__unsafe_cast")

everywhere, right?

[cristianoc]

I think Pervasives is not needed, just Lident "__unsafe_cast". Which is good, as the name is not future-proof and might disappear one day.

[cknitt]

@cristianoc @bobzhang Did the __unsafe_cast thing in #5388 as requested. Had to commit the result of ./scripts/ninja.js build to get tests passing.

[cknitt]

The CI build for Windows seems good enough now (even though not all test can be run on Windows). Closing this issue.