thierry-martinez / stdcompat

Stdcompat: compatibility module for OCaml standard library
BSD 2-Clause "Simplified" License
31 stars 13 forks source link

stdcomp.19 failed on Windows #23

Closed kkirstein closed 2 years ago

kkirstein commented 2 years ago

It looks like building stdcomp.19 fails to build on Windows (Cygwin-based Ocaml-for-Windows environment):

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] The compilation of stdcompat failed at
        "C:\\Opt\\OCaml64\\home\\kayuwe.kirstein\\.opam\\4.14.0+mingw64\\bin\\dune.exe build -p
        stdcompat -j 11".

#=== ERROR while compiling stdcompat.19 =======================================#
# context     2.0.10 | win32/x86_64 | ocaml-variants.4.14.0+mingw64 | pinned(file://C:/Users/kayuwe.kirstein/Workspaces/OCaml/stdcompat)
# path        ~/.opam/4.14.0+mingw64/.opam-switch/build/stdcompat.19
# command     C:\Opt\OCaml64\home\kayuwe.kirstein\.opam\4.14.0+mingw64\bin\dune.exe build -p stdcompat -j 11
# exit-code   1
# env-file    ~/.opam/log/stdcompat-44852-6d07ae.env
# output-file ~/.opam/log/stdcompat-44852-6d07ae.out
### output ###
# Error: CreateProcess(): Exec format error
# -> required by _build/default/stdcompat.h
# -> required by _build/install/default/lib/stdcompat/stdcompat.h
# -> required by _build/default/stdcompat.install
# -> required by alias install

<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
┌─ The following actions failed
│ λ build stdcompat 19
└─
╶─ No changes have been performed

I already tried to apply the patches from opam-repository-mingw (automatic configure of shared lib extension) without any changes on the error message.

It looks like it, fails to properly start configure, but I can't figure out any root causes by the changes from 18 to 19.

thierry-martinez commented 2 years ago

Thank you for your report! I was not able to reproduce the problem because I don't have enough Windows-foo to setup a mingw-based opam environment (you can see my failed attempts here: https://github.com/thierry-martinez/stdcompat/actions?query=branch%3Afix.23.windows-build ), and the problem does not appear to exist with Visual C++ port. I suspect that the problem is that ./configure is an executable for CreateProcess, and the command should begin explicitly by sh as executable. I proposed a fix in #24: if you can confirm that it solves the problem, I will merge. Thank you again!

kkirstein commented 2 years ago

Yes, #24 fixes the build on Windows. Thanks for your fast response. With respect to the build environment, there are Github actions for Windows provided by the https://github.com/ocaml/setup-ocaml project (haven't used that by myself yet).

For a make-based install of stdcompat the patch from opam-repository-mingw would be needed to properly handle the shared libraries with either .so or .dll file extension (dune handles this automatically). If the make-based approach is still required, I can provide a PR for that.

thierry-martinez commented 2 years ago

Yes, #24 fixes the build on Windows.

Thank you. The fix is now merged.

With respect to the build environment, there are Github actions for Windows provided by the https://github.com/ocaml/setup-ocaml project (haven't used that by myself yet).

Yes, I use them already in stdcompat CI (for instance, here: https://github.com/thierry-martinez/stdcompat/actions/runs/2845475339 ) but Windows port is MSVC++ based, and, somewhat surprisingly, there is no problem with executing directly ./configure with this port.

For a make-based install of stdcompat the patch from opam-repository-mingw would be needed to properly handle the shared libraries with either .so or .dll file extension (dune handles this automatically). If the make-based approach is still required, I can provide a PR for that.

Again, it seems that MSVC++ port of OCaml does not need this patch. However it would be nice if the Makefile works with mingw too: https://github.com/thierry-martinez/stdcompat/pull/25 should fix this. Could you confirm (I still don't have a properly working mingw port)? Thank you very much!

kkirstein commented 2 years ago

Yes I can confirm the patch is working (basically the same I got). I observed just one minor caveat: make uninstall does not remove the stublib (dllstdcompat__stubs.dll). But this was probably also not handled by the original mingw-patch.

I also don't know, why there should be a difference between MSVC and Mingw-based OCaml versions, as both use a Cygwin shell. But I also could not explain, why building fails after upgrading from version 18 to 19. Maybe there is another issue with my shell setup.

Anyway, I think the patch with explicitly calling sh makes the whole build process more robust, so it is a good idea.