Support MSVC compiler

[jonahbeckford]

Context: I'm distributing a MSVC-compiled distribution of OCaml for Windows. I am using an unreleased port of ctypes for MSVC which passes ctypes own (large) internal test suite. I'd like to use libuv a) to test ctypes before releasing it and b) to include/recommend for the Windows distribution.

This PR gets the luv code to compile with:

• libuv 1.42 compiled with CMake from source (/tmp/libuv ), with all 400+ libuv tests passing except watcher_cross_stop
• LUV_USE_SYSTEM_LIBUV=yes INCLUDE="$(cygpath -aw /tmp/libuv/include);$INCLUDE" LIB="$(cygpath -aw /tmp/libuv/build/Debug);$LIB" make build

The luv tests also build (LUV_USE_SYSTEM_LIBUV=yes INCLUDE="$(cygpath -aw /tmp/libuv/include);$INCLUDE" LIB="$(cygpath -aw /tmp/libuv/build/Debug);$LIB" PATH="/tmp/libuv/build/Debug:$PATH" make test) but there are failures during test execution:

• many TCP tests hung
• 22 failures in file
• 1 failure in fs_event (change)
• 1 failure in dns (getnameinfo)

My immediate problem is I can't get the ordinary (GCC-compiled) Windows version of luv to build using the "official" OCaml CI image ... which means I don't have a baseline to resolve any test failures:

PS1> docker run -it ocaml/opam:windows-mingw-20H2
C:\cygwin64\home\opam>git clone https://github.com/aantron/luv.git
C:\cygwin64\home\opam>cd luv
C:\cygwin64\home\opam\luv>git switch -d 0.5.10
C:\cygwin64\home\opam\luv>git submodule update --init --recursive
C:\cygwin64\home\opam\luv>opam install . --with-test --deps-only
C:\cygwin64\home\opam\luv>make build
C:\cygwin64\home\opam\luv>make test
dune runtest --no-buffer --force
Done: 853/995 (jobs: 2)luv_unix.c:9:10: fatal error: uv.h: No such file or directory
    9 | #include <uv.h>
      |          ^~~~~~
compilation terminated.

So ... how did you test the Windows version? Did all of your tests pass when you did?

Thanks! Jonah

[aantron]

So ... how did you test the Windows version?

I originally tested the Windows version on a local installation of the MinGW-based OCaml for Windows, and then I set up building but not testing of it in GitHub Actions.

Did all of your tests pass when you did?

As I recall, many tests failed, mainly because the tests themselves assumed a more Unix-like system and features that libuv itself does not implement on Windows.

Are you able to install the MinGW OCaml for Windows to run the tests and get a report? Alternatively, you should be able to modify the CI workflow by adding

- run: opam exec -- dune runtest

or similar, after this line:

luv/.github/workflows/ci.yml

Line 32 in e5b1096

- run: opam exec -- dune build -p luv

and get a log.

[jonahbeckford]

Okay; thanks! I should be able to get a baseline from what you mentioned. I'll post it to this PR; probably in a week or so.

[jonahbeckford]

Ok. Completed the baselining and rebased.

Results

What	MinGW	MSVC	Interpretation
Test Results	GitHub Actions	bottom of test-win32-msvc.sh
Tests Run	275	277	More tests stall in MinGW
Tests Succeed	271 (275-4)	272 (277-5)	connect, synchronous error leak passes in MSVC
Tests Stalled	test-win32-mingw.sh	test-win32-msvc.sh	hundreds of stalled tests

There are some failures in both MinGW and MSVC that shouldn't be there:

• open, read, close: async and open, read, close: sync don't seem to consider that Windows has carriage returns
• connect, handle lifetime gives ECONNREFUSED on Windows rather than ECANCELED (not sure that is a bug)

And one failure on MSVC (getnameinfo) seems like it is working since it is trying to resolve kubernetes.docker.internal which won't be present in Docker Windows containers.

Conclusions

• There are no regressions in this PR
• This PR seems to give a very slight improvement (less stalling; more successes) over MinGW
• Windows + luv, regardless of MinGW or MSVC, is not healthy. The TCP, file and thread APIs are pretty critical and almost entirely stall or fail.

[aantron]

• Windows + luv, regardless of MinGW or MSVC, is not healthy. The TCP, file and thread APIs are pretty critical and almost entirely stall or fail.

Have you verified this, or is this based on the tests? Many of the tests themselves currently make assumptions about running on a Unix-like platform, and I did not port them to Windows or to being cross-platform. If this statement is based on the tests, it would require looking in detail at each test and API instead.

[aantron]

As far as I can tell at first glance, only this change and the change to src/c/windows_version.h are necessary for Luv itself. Is that right?

[jonahbeckford]

Correct. The rest is regression test apparatus for Windows. If you want, I can split the PR when I get back in next week.

[aantron]

Yes, please. In the long term (not in this PR), the test suite should be made compatible by porting more of it to Windows, and then it can be run directly from the CI without intervening scripts. So I prefer not to merge the scripts, but keep them in a branch for reference.

[jonahbeckford]

1. Branch with test scripts is https://github.com/jonahbeckford/luv/tree/0.5.11-msvctesting
2. Did a force push that removes test scripts.

[jonahbeckford]

• Windows + luv, regardless of MinGW or MSVC, is not healthy. The TCP, file and thread APIs are pretty critical and almost entirely stall or fail.

Have you verified this, or is this based on the tests? Many of the tests themselves currently make assumptions about running on a Unix-like platform, and I did not port them to Windows or to being cross-platform. If this statement is based on the tests, it would require looking in detail at each test and API instead.

Sorry for the delay (still OOO). I have not seen any failures in the Luv APIs I am using internally, so I understand the point you are making. So please consider that "problem" closed.

FYI: If you can point to the lines of code you think is not cross-platform, I may come back to it later and submit a PR to fix it. But as you implied that is low priority.

[aantron]

Thanks! Would you like a speedy release of this?

[aantron]

FYI: If you can point to the lines of code you think is not cross-platform, I may come back to it later and submit a PR to fix it. But as you implied that is low priority.

Thanks. I don't recall and can't immediately get time to find these lines. But they concern such issues as I have the test suite deliberately assume that a Unix.file_descr is an int (it is, on Unix) and use Obj.magic on it in order to convert between Luv code and other code the tests use as reference behavior, but on Windows it is a union of a HANDLE and a SOCKET, with some other fields as well, so this "cast" obviously can only lead to incorrect behavior. Tests that do things like this need to be re-thought somehow. I should probably myself start a hobby project to gradually review all the tests and Windows-ify them :)

[jonahbeckford]

Thanks! Would you like a speedy release of this?

It would be nice to have a release so I can remove the patches from the DKML Windows Opam repository, but it is not urgent since the patches are available already. So please do a release whenever it is most convenient for you. Thanks!