ocaml / ocamlbuild

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

Windows/Cygwin: hygienic check fails for some packages #316

Closed kkirstein closed 3 months ago

kkirstein commented 2 years ago

It looks like an old issue has popped up again in ocamlbuild 0.14.1. For some packages on Windows/Cygwin (Mingw toolchain), the hygienic check fails, e.g., for mtime 1.3.0:

mkdir 'C:\Opt\OCaml64\home\kayuwe.kirstein\.opam\4.13.1+mingw64\.opam-switch\build\mtime.1.3.0\_build'
Das System kann den angegebenen Pfad nicht finden.
''ocamlfind ocamlopt unix.cmxa -I 'C:/Opt/OCaml64/home/kayuwe.kirstein/.opam/4.13.1+mingw64/lib\ocamlbuild' 'C:/Opt/OCaml64/home/kayuwe.kirstein/.opam/4.13.1+mingw64/lib\ocamlbuild/ocamlbuildlib.cmxa' -linkpkg myocamlbuild.ml 'C:/Opt/OCaml64/home/kayuwe.kirstein/.opam/4.13.1+mingw64/lib\ocamlbuild/ocamlbuild.cmx' -o myocamlbuild.exe
SANITIZE: a total of 3 files that should probably not be in your source tree
  has been found. A script shell file
  "C:\\Opt\\OCaml64\\home\\kayuwe.kirstein\\.opam\\4.13.1+mingw64\\.opam-switch\\build\\mtime.1.3.0\\_build/sanitize.sh"
  is being created. Check this script and run it to remove unwanted files or
  use other options (such as defining hygiene exceptions or using the
  -no-hygiene option).
IMPORTANT: I cannot work with leftover compiled files.
ERROR: Leftover object files:
  File myocamlbuild.o in _build has suffix .o
ERROR: Leftover OCaml compilation files:
  File myocamlbuild.cmi in _build has suffix .cmi
  File myocamlbuild.cmx in _build has suffix .cmx
Exiting due to hygiene violations.
pkg.ml: [ERROR] cmd ["ocamlbuild" "-use-ocamlfind" "-classic-display" "-j" "4" "-tag" "debug"
     "-build-dir" "_build" "opam" "pkg/META" "CHANGES.md" "LICENSE.md"
     "README.md" "src/mtime.a" "src/mtime.cmxs" "src/mtime.cmxa"
     "src/mtime.cma" "src/mtime.cmx" "src/mtime.cmi" "src/mtime.mli"
     "src/mtime_top.a" "src/mtime_top.cmxs" "src/mtime_top.cmxa"
     "src/mtime_top.cma" "src/mtime_top.cmx" "src/mtime_top_init.ml"
     "doc/index.mld" "src-os/mtime_clock.a" "src-os/mtime_clock.cmxs"
     "src-os/mtime_clock.cmxa" "src-os/mtime_clock.cma"
     "src-os/mtime_clock.cmx" "src-os/mtime_clock.cmi"
     "src-os/mtime_clock.mli" "src-os/dllmtime_clock_stubs.dll"
     "src-os/libmtime_clock_stubs.a" "test-os/min_os.ml"]: exited with 1

I am not sure whether the first error message on not finding the build folder is a serious failure. Executing sanitize.sh does not help, I guess opam install mtime regenerates the object files every time.

Observed on Windows 10, Cygwin/Mingw (Ocaml for Windows) for OCaml 4.13.1+mingw64. My current workaround is downgrading ocamlbuild to 0.14.0.

kit-ty-kate commented 2 years ago

opam-repository-mingw has a patch for ocamlbuild apparently. I have no idea what it’s supposed to fix but could you give it try to see if it fixes your issue?

https://github.com/fdopen/opam-repository-mingw/blob/df5006bfe42608c42d02ad1f23a44f2451f35e3a/packages/ocamlbuild/ocamlbuild.0.14.0/files/ocamlbuild-0.14.0.patch

kkirstein commented 2 years ago

@kit-ty-kate You pointed to the right location. Actually I had a mix-up of different versions. I have both repos upstream opam-repository from ocaml.org & opam-repository-mingw from github/fdopen in my opam setup (This is the suggested configuration, as the fdopen repo is deprecated for ordinary packages). I thought opam always takes the packages from the first repo, if available, but it seems to give priority to the highest patchlevel.

Anyway, here is the (corrected) list of ocamlbuild version & observed issue:

So, it looks like the patch from opam-repository-mingw is needed for Windows/Cygwin. It would be nice to have that merged into the ocamlbuild sources. Are there any reasons not to do this? If yes, I would suggest to add a constraint to ocamlbuild.opam like os != "win32" & os-distribution != "cygwinports", so one can not accidently pick up an unpatched version.

gasche commented 2 years ago

Are there any reasons not to do this?

To my knowledge, no one has done the work of submitting the patch upstream (here) yet, and (hopefully) explaining what it does.

gasche commented 2 years ago

@kkirstein if you were interested in getting some of this patch merged, I think that would be very nice! There are some parts that were not intended for upstreaming (they are redundant with other code, etc., not written with maintenability in mind), some parts that could be simplified, some parts whose purpose/effect is unclear, etc. (And of course no tests :-) I think someone would need to split it into separate commits that make sense individually (instead of mixing unrelated things together), and submit PRs. I'm happy to help by reviewing the PRs then, but given that I don't have a Windows machine I cannot help with the testing.

gasche commented 2 years ago

Note: another route for you would be of course to submit an ocamlbuild.14.1 package in opam-repository-mingw, with the same patch as before (I would guess that the rebase would be very easy). That would not help for future versions of reduce the maintenance burden, but it's certainly a good short-term step you could take, hopefully in parallel to my suggestion above.

(For the record, the OCaml Foundation offered funding to fdopen, the author and maintainer of opam-repository-mingw, to keep maintaining the patches and work on upstreaming them in their respective projects, but fdopen's opinion at the time (in addition to a lack of time available to work on this) was that it's less necessary nowadays because more recent packages, in particular those using dune, work fine under Windows, so the repository is getting less and less relevant. That sounds like a reasonable position to me, but I'm not familiar with the Windows situation.)

kkirstein commented 2 years ago

@gasche I would agree with fdopen that we should keep the number of packages in the mingw repo with special patches small and concentrate of merging Windows-related patches to the upstream repos. I had a (very rough) look at the patch and indeed there seems to quite some stuff in it, where some of it probably is not needed anymore or unrelated to the observed issue. But (at least for me) it is hard to sort that out. In addition, we have to be careful not to break the other target systems, like "pure" Cygwin and "pure" Windows.

Anyway, I will have a closer look at fixing the hygienic/path issue, but I am not sure I can provide something in the near future.

kkirstein commented 2 years ago

A more general question: What is the roadmap for ocamlbuild anyway? dune is getting more and more adoption and has advantage with respect to platform independence. But I also remember some posts on discuss.ocaml.org of people who want to stick with ocamlbuild.

gasche commented 2 years ago

Right now ocamlbuild is in maintenance-mode only, with no new developments planned, and the idea is to encourage people to migrate to dune. We keep the lights on and write the easy fixes and merge PRs when they look good, but not much more.

dbuenzli commented 2 years ago

So far I still use it in my packages to build my distributions via topkg. Thanks for keeping the light on !