zshipko / ocaml-rs

OCaml extensions in Rust
https://docs.rs/ocaml
ISC License
259 stars 31 forks source link

Example dune needs some changes for reliable/incremental builds #25

Closed mbacarella closed 4 years ago

mbacarella commented 4 years ago

Hi there. Cool project!

Some rough spots to iron out in the example.

w.r.t. test/src/dune, I had to say not dot, but two dots (source_tree ..) for the launched cargo command to be able to find its .toml file after dune copies everything to the _build directory.

Additionally, to get incremental rebuilds to work, I had to change the ../rust path to something absolute, like /var/tmp/rust, so that dune wouldn't blow it away from the _build dir on subsequent runs. Without this, it would rebuild from scratch every time (a 1-2 minute long operation).

Does your dune do this differently? Curious if it can be told to leave the rust build artifacts alone

zshipko commented 4 years ago

The build works for me, but there are issues around incremental builds like you mentioned.

These sound like great suggestions - I will update the dune file sometime today

zshipko commented 4 years ago

Just out of curiosity, which dune version are you on? Also, could you post your modified dune file?

mbacarella commented 4 years ago

Ya sure thing. dune --version says 2.5.1

Contents of my src/dune file:

(include_subdirs unqualified)

(rule
  (deps (source_tree ..))
  (targets librustlib.a dllrustlib.so)
  (action
    (progn
      ; Build the Rust code.  The public example uses ../rust, but dune
      ;  blows away anything in _build it doesn't recognize on each run,
      ;  which ruins incremental builds.  So, let's just stick the work
      ;  files in /var/tmp instead!
      (run cargo build --target-dir /var/tmp/rustlib --release)
      ; This is needed to support Linux and MacOS shared libraries
      (run sh -c
        "cp -f /var/tmp/rustlib/release/librustlib.so ./dllrustlib.so || cp -f /var/tmp/rustlib/release/librustlib.dylib ./dllocamlrs_test_stubs.so")
      ; Copy over the static library too
      (run cp -f /var/tmp/rustlib/release/librustlib.a librustlib.a)
      )))

(library
  (name rustlib)
  (modules mixer)
  ;(inline_tests)
  ;(preprocess (pps ppx_inline_test))
  (libraries unix)
  (foreign_archives rustlib)
  (c_library_flags
  (-lpthread -lc -lm)))
zshipko commented 4 years ago

Thanks!

zshipko commented 4 years ago

It turns out the test project is a little abnormal due to how tightly coupled it is to the actual ocaml crate. This makes it a pretty bad example of how to get started!

I've added https://github.com/zshipko/ocaml-rust-starter and updated the documentation: https://github.com/zshipko/ocaml-rs/tree/starter to mention this instead of the test dune file.

Let me know if that clears things up, or you have any more suggestions!

zshipko commented 4 years ago

After some further investigation, it looks like /var/tmp is mounted as read-only in the opam sandbox. I've switched the path to %{project_root}/target instead.

mbacarella commented 4 years ago

Sweet. FWIW, this project was re-plugged recently here https://discuss.ocaml.org/t/is-there-a-way-to-turn-off-garbage-collection-inside-of-one-function/5626/16 so polishing up the starter project is good timing.

zshipko commented 4 years ago

Ah, cool. Thanks for the heads up!

I am in the middle of a big rewrite working up to a 1.0 release. So I'm really hoping to iron out these kinds of issues before then.

Let me know if you run into any other issues, based on that post it looks like you have a pretty specific use-case, which I haven't spent much time exploring myself.