sail-riscv icon indicating copy to clipboard operation
sail-riscv copied to clipboard

Incremental OCaml builds don't work

Open Timmmm opened this issue 1 year ago • 1 comments

This is quite annoying. If you run this command

ARCH=RV64 make ocaml_emulator/riscv_ocaml_sim_RV64

and you have some error in your OCaml code it will do the expected thing, compile the Sail, print your OCaml error. If you immediately run it again you'll get this error:

mkdir -p ocaml_emulator/_sbuild
cp ocaml_emulator/_tags ocaml_emulator/platform.ml ocaml_emulator/platform_impl.ml ocaml_emulator/softfloat.ml ocaml_emulator/riscv_ocaml_sim.ml generated_definitions/ocaml/RV64/*.ml ocaml_emulator/_sbuild
cp: cannot create regular file 'ocaml_emulator/_sbuild/elf_loader.ml': Permission denied
cp: cannot create regular file 'ocaml_emulator/_sbuild/sail_lib.ml': Permission denied
cp: cannot create regular file 'ocaml_emulator/_sbuild/util.ml': Permission denied
make: *** [Makefile:222: ocaml_emulator/_sbuild/riscv_ocaml_sim.native] Error 1

Looking at those files for some reason they are made read-only:

❯ ls -l ocaml_emulator/_sbuild/
total 4776
drwxr-xr-x 2 timothy.hutt domain users    4096 Feb 12 15:11 _build
-r--r--r-- 1 timothy.hutt domain users   11857 Feb 12 15:11 elf_loader.ml
-rw-r--r-- 1 timothy.hutt domain users    6753 Feb 12 15:20 platform_impl.ml
-rw-r--r-- 1 timothy.hutt domain users    5874 Feb 12 15:20 platform.ml
-rw-r--r-- 1 timothy.hutt domain users 4771832 Feb 12 15:20 riscv.ml
-rw-r--r-- 1 timothy.hutt domain users    7339 Feb 12 15:20 riscv_ocaml_sim.ml
-r--r--r-- 1 timothy.hutt domain users   41396 Feb 12 15:11 sail_lib.ml
-rw-r--r-- 1 timothy.hutt domain users    1914 Feb 12 15:20 softfloat.ml
-rw-r--r-- 1 timothy.hutt domain users     163 Feb 12 15:20 _tags
-r--r--r-- 1 timothy.hutt domain users   22051 Feb 12 15:11 util.ml

They look like they are from the Sail compiler so I guess it has something to do with how I installed that, which is just via make install. From the Sail source directory:

❯ ls -l `find . -name 'sail_lib.ml'`
-r--r--r-- 1 timothy.hutt domain users 41396 Dec 12 15:59 ./_build/default/src/lib/sail_lib.ml
lrwxrwxrwx 1 timothy.hutt domain users    39 Dec 12 15:59 ./_build/install/default/lib/libsail/sail_lib.ml -> ../../../../default/src/lib/sail_lib.ml
lrwxrwxrwx 1 timothy.hutt domain users    45 Dec 12 15:59 ./_build/install/default/share/sail/src/lib/sail_lib.ml -> ../../../../../../default/src/lib/sail_lib.ml
-rw-r--r-- 1 timothy.hutt domain users 41396 Feb  9 15:58 ./src/lib/sail_lib.ml

This seems to be something that Dune deliberately does so I have no idea what's going on here or how it was supposed to work.

Timmmm avatar Feb 12 '24 15:02 Timmmm

Not sure it has anything to do with dune. Sail currently sets up the OCaml output build as an ocamlbuild project (which I do need to change to dune at some point, as ocamlbuild is now effectively deprecated by the OCaml community).

Alasdair avatar Feb 12 '24 15:02 Alasdair

I looked into this a bit more. The issue is this code in the Sail OCaml backend:

  let _ = Unix.system ("cp -r " ^ sail_dir ^ "/src/lib/elf_loader.ml .") in
  let _ = Unix.system ("cp -r " ^ sail_dir ^ "/src/lib/sail_lib.ml .") in
  let _ = Unix.system ("cp -r " ^ sail_dir ^ "/src/lib/util.ml .") in

(I don't know why there's an -r?)

Those files are installed read-only when you do make install:

❯ ls -l $(sail -dir)/src/lib/elf_loader.ml
-r--r--r-- 1 .../sail/_build/default/src/lib/elf_loader.ml

(I don't know the exact conditions that result in them being read-only.)

So the Sail compiler copies those files to the RISC-V model in generated_definitions/ocaml/$ARCH with this command:

	$(SAIL) $(SAIL_FLAGS) -ocaml -ocaml-nobuild -ocaml_build_dir generated_definitions/ocaml/$(ARCH) -o riscv $(SAIL_SRCS)

And later it copies them into _sbuild with this command:

	cp ocaml_emulator/_tags $(PLATFORM_OCAML_SRCS) generated_definitions/ocaml/$(ARCH)/*.ml ocaml_emulator/_sbuild

If you already have a build, then the second cp fails. I believe the first one in the Sail compiler would fail too but errors are ignored, and the files are already there so it doesn't matter.

Hacky fix: I will add -f to the second cp in the Makefile.

Proper fix: probably the Sail compiler shouldn't be shelling out to cp, but copying the file natively like this (OCaml doesn't seem to have a built in function to copy files but that's not much code to copy/paste).

I'll make a PR for the -f hack.

Timmmm avatar Jun 14 '24 13:06 Timmmm