search

ocaml / ocamlbuild

The legacy OCamlbuild build manager
Other
121 stars 81 forks source link

Import fdopen patch for windows #330

Closed hhugo closed 3 months ago

hhugo commented 3 months ago

This is a straight import of the patch used to fix ocamlbuild on windows inside opam-repository.

Opened as a Draft for now.

I've imported it on top of #329 so that we can have a CI. I've added a commit to make the testsuite pass again on my machine.

hhugo commented 2 months ago
  1. handle long command on windows (above 8191 bytes), by moving the command to a separate shell script.
  2. use a leading space instead of '' for bash commands (it looks nicer)
  3. handle Unix.EPIPE gracefully when Sys.win32 in ocamlbuild_executor
  4. normalize path with forward slash
  5. set stdout to binary mode to avoid CRLF

We might want to import 1 and 2, I don't know about 3. We can probably ignore 4 and 5

hhugo commented 2 months ago

3 was fixed in OCaml 4.05

  • 7342, #797: fix Unix.read on pipes with no data left on Windows

    it previously raised an EPIPE error, it now returns 0 like other OSes (Jonathan Protzenko, review by Andreas Hauptmann and Damien Doligez)

hhugo commented 2 months ago

2 -> https://github.com/ocaml/ocamlbuild/pull/343

hhugo commented 2 months ago
  1. handle long command on windows (above 8191 bytes), by moving the command to a separate shell script.

@dra27, now that we no longer rely on cmd.exe do we still have such a low limit ? A quick search online says that createProcess has a limit of 32767.

hhugo commented 2 months ago
  1. is implemented in #344