Fix cross-platform build: replace symlinks with copy_files#4085
Fix cross-platform build: replace symlinks with copy_files#4085elefthei wants to merge 33 commits intoFStarLang:masterfrom
Conversation
mk/stage.mk
Outdated
| @# Seems to need one final build? | ||
| cd dune && $(DUNE) build $(FSTAR_DUNE_BUILD_OPTIONS) | ||
| cd dune && $(DUNE) install $(FSTAR_DUNE_OPTIONS) --prefix=$(PREFIX) | ||
| cd dune && $(DUNE) install $(FSTAR_DUNE_OPTIONS) --prefix=$(call cygpath,$(PREFIX)) |
There was a problem hiding this comment.
Should this also be platform_path? What if cygpath is not present?
There was a problem hiding this comment.
Also is cygpath really needed here and below? Do relative paths not work?
| open Ast_helper | ||
| open Ast | ||
| open Ppxlib.Ast_builder.Default | ||
| open Longident |
There was a problem hiding this comment.
Are the changes in this file fixing the build with 5.4.0? That's great! Could it be put into a standalone PR?
stage1/dune/fstar-guts/dune
Outdated
| (copy_files ../../../stage0/dune/fstar-guts/fstarc.ml/*.ml) | ||
| (copy_files ../../../stage0/dune/fstar-guts/fstarc.ml/*.mli) | ||
| (copy_files ../../../stage0/dune/fstar-guts/app/*.ml) | ||
| (copy_files ../../../stage0/dune/fstar-guts/app/*.mli) |
There was a problem hiding this comment.
There won't be any *.mli files generated, so those lines could be removed. We could also just use * I think.
Also, the first two lines are wrong I think. The fstarc.ml we should use is the one in stage1.
There was a problem hiding this comment.
I think this change would also mean that, when a file is deleted from F* sources, the .ml file will be retained in this directory. There is logic in the makefile to delete these spurious files from the fstarc.ml directory, but the copy here would not take that into account. Is that right? It's not terrible, but we should maybe write a small note.
stage1/dune/fstar-guts/dune
Outdated
| (rule | ||
| (target FStar_UInt16.ml) | ||
| (deps (:script mk_int_file.sh) (:body FStar_Ints.ml.body)) | ||
| (action (with-stdout-to %{target} (run bash ./mk_int_file.sh U 16)))) |
There was a problem hiding this comment.
This logic seems to be repeated around 9 times now. Can it be simplified?
stage1/dune/fstar-plugins/dune
Outdated
| (target FStar_UInt16.ml) | ||
| (deps mk_int_file.sh FStar_Ints.ml.body) | ||
| (action (with-stdout-to %{target} | ||
| (bash "echo 'module M = Stdint.Uint16'; echo 'type uint16 = M.t'; echo 'let n = Prims.of_int 16'; echo ''; echo 'let uint_to_t x = M.of_string (Z.to_string x)'; echo 'let __uint_to_t = uint_to_t'; cat FStar_Ints.ml.body")))) |
There was a problem hiding this comment.
Shouldn't this just call the mk_int_file.sh script?
| @@ -162,7 +162,7 @@ let call_subtac g (t : unit -> unit __tac) u ty = | |||
| let call_subtac_tm = from_tac_4 "B.call_subtac_tm" B.call_subtac_tm | |||
|
|
|||
| let stats_record (s : string) (f : unit -> 'c __tac) : 'c __tac = | |||
| from_tac_2 "B.stats_record" (B.stats_record () ()) s (to_tac_0 (f ())) | |||
| TM.mk_tac (fun ps -> FStarC_Stats.record s (fun _ -> TM.run (f ()) ps)) | |||
There was a problem hiding this comment.
This is for 5.4.0 compatibility, maybe this should be a separate change
|
Thanks Lef, I left some comments. In general I'm slightly concerned about duplication, but if that's what it takes to get a native Windows build then so be it : ). |
220503d to
66b0fb3
Compare
9f83193 to
72f2e93
Compare
- Windows CI now uses ocaml/setup-ocaml@v3 with bash (Cygwin) instead of winget+PowerShell - this automatically handles zarith/GMP dependencies - Makefile now detects Cygwin/MSYS environments via uname and uses Unix commands when running in those environments - USE_UNIX_CMDS variable controls whether to use Unix or PowerShell commands - Package target handles CYGWIN/MINGW in OS name for correct package naming
The kernel.org mirror used by ocaml/setup-ocaml@v3's built-in Cygwin setup has signature verification issues. Following the upstream pattern, we use cygwin/cygwin-install-action@master which handles mirror fallback better and installs the required GMP/libffi packages for zarith.
The kernel.org Cygwin mirror is having signature verification issues. Using a known reliable mirror from the official Cygwin mirror list.
…m\.last-env\env-af1c662855dbfe768f60cb2a65c64014-0' $env:OPAM_SWITCH_PREFIX = 'C:\Users\eioannidis\AppData\Local\opam\default' $env:OCAMLTOP_INCLUDE_PATH = 'C:\Users\eioannidis\AppData\Local\opam\default\lib\toplevel' $env:CAML_LD_LIBRARY_PATH = 'C:\Users\eioannidis\AppData\Local\opam\default\lib\stublibs;C:\Users\eioannidis\AppData\Local\opam\default\lib\ocaml\stublibs;C:\Users\eioannidis\AppData\Local\opam\default\lib\ocaml' $env:OCAML_TOPLEVEL_PATH = 'C:\Users\eioannidis\AppData\Local\opam\default\lib\toplevel' $env:MANPATH = ':/cygdrive/c/Users/eioannidis/AppData/Local/opam/default/man' $env:Path = 'C:\Users\eioannidis\AppData\Local\opam\default\bin;C:\Users\eioannidis\AppData\Local\opam\.cygwin\root\usr\x86_64-w64-mingw32\sys-root\mingw\bin;C:\Program Files\PowerShell\7;C:\Program Files\glzr.io\GlazeWM\\cli;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Program Files\Azure Dev CLI;C:\Program Files\GitHub CLI\;C:\Users\vmadmin\AppData\Local\Microsoft\WindowsApps;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn\;C:\Program Files\Microsoft SQL Server\150\Tools\Binn\;C:\.tools\dotnet;C:\.tools\.npm-global;C:\Windows\system32\config\systemprofile\AppData\Local\Microsoft\WindowsApps;C:\Program Files\Microsoft Dev Box Agent\Scripts;C:\Program Files\PowerShell\7\;C:\Program Files\VFS for Git;C:\Program Files\dotnet\;C:\Program Files\nodejs\;C:\Program Files\Graphviz\bin;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;C:\Program Files\Microsoft SQL Server\170\Tools\Binn\;C:\ProgramData\chocolatey\bin;C:\Program Files\CMake\bin;C:\Program Files\Git\cmd;C:\Program Files\Go\bin;C:\Program Files\Docker\Docker\resources\bin;C:\Program Files\PowerShell\7;C:\Program Files\glzr.io\GlazeWM\\cli;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Program Files\Azure Dev CLI;C:\Program Files\GitHub CLI\;C:\Users\vmadmin\AppData\Local\Microsoft\WindowsApps;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn\;C:\Program Files\Microsoft SQL Server\150\Tools\Binn\;C:\.tools\dotnet;C:\.tools\.npm-global;C:\Windows\system32\config\systemprofile\AppData\Local\Microsoft\WindowsApps;C:\Program Files\Microsoft Dev Box Agent\Scripts;C:\Program Files\PowerShell\7\;C:\Program Files\VFS for Git;C:\Program Files\dotnet\;C:\Program Files\nodejs\;C:\Program Files\Graphviz\bin;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;C:\Program Files\Microsoft SQL Server\170\Tools\Binn\;C:\ProgramData\chocolatey\bin;C:\Program Files\CMake\bin;C:\Program Files\Docker\Docker\resources\bin;C:\Program Files\Git\cmd;C:\Users\eioannidis\.local\bin;C:\Users\eioannidis\AppData\Roaming\agency\CurrentVersion;C:\Users\eioannidis\.cargo\bin;C:\Users\eioannidis\AppData\Local\Microsoft\WindowsApps;C:\Users\eioannidis\.dotnet\tools;C:\Users\eioannidis\AppData\Roaming\npm;C:\Users\eioannidis\AppData\Local\Programs\Microsoft VS Code Insiders\bin;C:\tools\neovim\nvim-win64\bin;C:\Users\eioannidis\AppData\Local\PowerToys\DSCModules\;C:\Users\eioannidis\AppData\Local\Microsoft\WinGet\Links;C:\Users\eioannidis\AppData\Local\Pandoc\;C:\Users\eioannidis\AppData\Local\Programs\MiKTeX\miktex\bin\x64\;C:\Users\eioannidis\AppData\Local\Programs\Microsoft VS Code Insiders\bin;C:\Program Files\Git\mingw64\bin;C:\Users\eioannidis\AppData\Local\opam\default\bin;C:\Users\eioannidis\.bun\bin;C:\Users\eioannidis\go\bin;C:\Users\eioannidis\.dotnet\tools' before make commands The dune binary is installed in the opam switch, so we need to source the opam environment before running make commands.
Dune is a Windows binary that doesn't understand Cygwin paths like /cygdrive/d/... Use cygpath -w to convert to Windows paths.
setup-ocaml@v3 has a hardcoded Cygwin mirror (kernel.org) that frequently fails with signature verification errors. Pre-installing all packages it needs via cygwin-install-action with a working mirror (dotsrc.org) so setup-ocaml will find them already installed.
The previous cache contained references to D:\cygwin but we now install Cygwin to C:\cygwin to match setup-ocaml's default location. Invalidate the cache to fix opam path conflicts.
cygpath -w produces backslash paths (D:\\a\\...) which are interpreted as escape sequences by /bin/sh. Use cygpath -m for mixed-mode paths (D:/a/...) with forward slashes that work in both shell commands and Windows-native tools like dune.
The stage2 target only builds the executable in _build/default/, it doesn't copy it to out/bin/. Add make setlink-2 to create the out/bin/fstar symlink that the verify step expects.
Tested on Windows 11 pwsh and WSL (Arch). Replace symlinks with dune copy_files directives to fix build on Windows where symlinks are not well supported. Remove all symlinks from git and use copy_files to copy source files from ulib/ml to build directories.
Changes:
This ensures the build works on both Windows and Linux without requiring symlink support.