ocaml-protoc-plugin icon indicating copy to clipboard operation
ocaml-protoc-plugin copied to clipboard

Remove dependency on unix libraries

Open ansiwen opened this issue 8 months ago • 11 comments

First: thanks for maintaining this library!

I'm using this library in an embedded/unikernel context (MirageOS/Solo5), and the new version does not compile in the cross-compiler environment:

make build               
dune build --profile release --root . ./dist
File "duniverse/dune_/otherlibs/configurator/src/dune", line 7, characters 12-16:
7 |  (libraries unix csexp)
                ^^^^
Error: Library "unix" in /home/opam/.opam/4.14/solo5-sysroot/lib/ocaml is
hidden (unsatisfied 'exists_if').
-> required by library "dune-configurator" in
   _build/solo5/duniverse/dune_/otherlibs/configurator/src
-> required by executable discover in duniverse/jst-config/discover/dune:1
-> required by _build/solo5/duniverse/jst-config/discover/discover.exe
-> required by _build/solo5/duniverse/jst-config/src/config.h
-> required by _build/install/solo5/lib/jst-config/config.h
-> required by %{lib:jst-config:config.h} at duniverse/time_now/src/dune:7
-> required by _build/solo5/duniverse/time_now/src/config.h
-> required by _build/solo5/manifest.o
-> required by _build/solo5/main.exe
-> required by _build/solo5/keyfender.muen
-> required by _build/solo5/dist/keyfender.muen
-> required by alias dist/all (context solo5)
-> required by alias dist/default (context solo5)
make: *** [Makefile:59: build] Error 1              

The problem seems to be the ppx_expect and ppx_inline_test dependencies, that unconditionally pull in unix dependencies, which are not available in this scenario. Is it possible to remove this dependency for the release profile, that is when tests are not compiled? I would expect that the PPX have a kind of "no-op-mode" for this case?

ansiwen avatar Apr 16 '25 23:04 ansiwen

I honestly don't know how to do this, but I welcome PRs to reduce the list of dependencies when/if the unix module is not available. Maybe with zealous use if exists_if to exclude tests ppx'es that depends on the unix library would work in this case.

andersfugmann avatar Apr 25 '25 07:04 andersfugmann

Unfortunately I also have no idea how to solve it. I would naively expect, that if you compile it without the tests, that there is no unix dependency, at least at runtime. I will try to get some know-how from somewhere...

ansiwen avatar Apr 25 '25 14:04 ansiwen

Related: https://github.com/ocaml/dune/issues/897

ansiwen avatar Apr 25 '25 16:04 ansiwen

It seems to be the library base which pulls dune-configurator. I see there exists libraries ppx_expect_nobase and ppx_inline_test_nobase which 🤞 would not pull in unix.

I've created a branch andersfugmann/nobase. Could you test to see if that one compiles in your setup, before I figure out how to seamlessly switch between the two variants of the ppx packages?

andersfugmann avatar Apr 26 '25 21:04 andersfugmann

I pushed some updates to the branch, which would be nice to know if works. It should seamlessly prioritize ppx_expect_nobase over ppx_expect, but it's somewhat untested.

If HEAD of the branch does not work, could you try only the first commit (i.e. git checkout 095b35cf4f4fe5f4969940d36bfc834a3db66cc9 and let me know if that works? Btw. remember to install ppx_expect_nobase first (opam install ppx_expect_nobase)

andersfugmann avatar Apr 28 '25 21:04 andersfugmann

Thanks a lot! I will try it out in next few days. I also kindly asked @firobe to give his input, when he finds some time, who knows a lot more about MirageOS and Ocaml than me.

BTW: there is another issue, that when google_types are created, the cross-compiled protoc-gen-ocaml is called, which of course doesn’t work. A host compiled version must be used. But I have no idea if this is a flaw un MirageOS/Solo5 or duniverse or the dune files in this repo. Unfortunately it’s a very complex build mechanism and costs a lot of time to debug. 😵‍💫

ansiwen avatar Apr 29 '25 11:04 ansiwen

~~I understand the challenge when doing x-compilation using dune. I really dont know how it's supposed to work, but there may be a problem with the dune rules referencing build artifacts and not dependencies correctly. Could you open a new issue for this?~~

andersfugmann avatar Apr 29 '25 17:04 andersfugmann

I've pushed an additional commit to try and use the host implementation when building the google types. Let me know if that works. It could be extended to tests as well if you want to run tests while using cross compilation.

andersfugmann avatar Apr 30 '25 21:04 andersfugmann

Come think of it - Of course tests that are executed on the host, should use the host binaries :-) I've amended the commit, and as far as I can see, the build process will pick up host binaries when doing cross compilation. The fix actually also solved some other problems I had with tracking dependencies correctly so thanks for reporting.

andersfugmann avatar Apr 30 '25 22:04 andersfugmann

@andersfugmann Thank a lot for your efforts! So I finally had time to try it out. opam monorepo apparently pulls the first of alternative dependencies in the opam file, so had to move the *_nobase variants before the normal versions. But unfortunately it still doesn't work. The ppx_expect_nobase doesn't compile with the cross-compiler:

File "duniverse/ppx_expect_nobase/collector/dune", line 9, characters 10-37:
9 |  (c_names expect_test_collector_stubs)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Context: solo5
expect_test_collector_stubs.c: In function 'expect_test_collector_before_test':
expect_test_collector_stubs.c:60:8: error: implicit declaration of function 'dup' [-Wimplicit-function-declaration]
   60 |   fd = dup(cstdout->fd);
      |        ^~~
expect_test_collector_stubs.c:68:9: error: implicit declaration of function 'dup2' [-Wimplicit-function-declaration]
   68 |   ret = dup2(output->fd, cstdout->fd);
      |         ^~~~
expect_test_collector_stubs.c: In function 'caml_out_channel_pos_fd':
expect_test_collector_stubs.c:100:28: error: 'SEEK_CUR' undeclared (first use in this function)
  100 |   ret = lseek(chan->fd, 0, SEEK_CUR);
      |                            ^~~~~~~~
expect_test_collector_stubs.c:100:28: note: each undeclared identifier is reported only once for each function it appears in
mtime_clock_stubs.c:209:2: warning: #warning OCaml Mtime_clock module: unsupported platform [-Wcpp]
  209 | #warning OCaml Mtime_clock module: unsupported platform
      |  ^~~~~~~
ocamlfind -toolchain solo5 ocamlopt    -c -o clock_stubs.o clock_stubs.c
ocamlfind -toolchain solo5 ocamlopt    -c -o mm_stubs.o mm_stubs.c
ocamlfind -toolchain solo5 ocamlopt    -c -o main.o main.c
ar r libmirage-solo5_bindings.a clock_stubs.o mm_stubs.o main.o
ar: creating libmirage-solo5_bindings.a
make[1]: *** [Makefile:59: build] Error 1           
make: *** [Makefile:11: all] Error 2

My toolchain doesn't have a normal C library, that's why this probably fails. And anyway it seems like it still tries to build the tests, just with another implementation. What would be actually be required would be a "noop" variant of the ppx_expect package, that just removes the ppx expressions from the Ocaml source code. Probably very easy for someone familiar with it. I will check if I can wrap my head around it.

In case you still have ideas that you want to try out, I made a reproducer for you, so you can easily try it out with the MirageOS toolchain.

$ docker run -ti --rm -v $SRC_PATH:/src:Z,ro docker.io/ansiwen/ocaml-protoc-plugin-repro

where $SRC_PATH is the absolute path to your ocaml-protoc-plugin directory, builds a test dummy unikernel including ocaml-protoc-plugin.

ansiwen avatar May 02 '25 21:05 ansiwen

Ok, I created a dummy ppx_expect, and if I pin to it, everything works. So your fix of the google_types generation definitely works. It also worked when I put the dummy implementation into a subdirectory of ocaml-protoc-plugin as ppx_expect_nobase. However, then I could not make it use the original ppx_expect anymore, it always used the dummy, but probably I did something wrong.

So as I see it there would be two paths forward: either people like me simply have to pin the dummy package, or you integrate the dummy into your repo somehow and use it if not in test mode (is there an inverse of {with-test}?). Anyway, it's as good as solved. :-)

ansiwen avatar May 03 '25 01:05 ansiwen