search

aantron / luv

Cross-platform asynchronous I/O and system calls
https://aantron.github.io/luv
MIT License
275 stars 26 forks source link

Support MSVC compiler #126

Closed jonahbeckford closed 2 years ago

jonahbeckford commented 3 years ago

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:

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:

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 commented 2 years ago

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:

https://github.com/aantron/luv/blob/e5b109676c0d37b2a1ec5161a5bcc04efea9c9ba/.github/workflows/ci.yml#L32

and get a log.

jonahbeckford commented 2 years ago

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 commented 2 years ago

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:

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

aantron commented 2 years ago
  • 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.

jonahbeckford commented 2 years ago
  • 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 commented 2 years ago

Thanks! Would you like a speedy release of this?

aantron commented 2 years ago

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 commented 2 years ago

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!