Work around various opam and dune issues on non-linux systems#416
Work around various opam and dune issues on non-linux systems#416
Conversation
cc279d2 to
97ec5f0
Compare
dune build on non-linux systems...| @@ -1,35 +1,37 @@ | |||
| (* -*- tuareg -*- *) | |||
|
|
|||
| let linux = List.mem ("system", "linux") Jbuild_plugin.V1.ocamlc_config | |||
There was a problem hiding this comment.
Note dune issue: The documentation/value of %{system} is not consistent
I assume this test is not more broken than (enabled_if (= %{system} "linux")) which, unfortunately, doesn't seem to prevent dune from failing dune build.
There was a problem hiding this comment.
Related issue in dune: enabled_if doesn't work in (test)
| dune exec -- ./bench/bench_semaphore.exe | ||
| dune exec -- ./bench/bench_cancel.exe | ||
| dune exec -- ./lib_eio_linux/tests/bench_noop.exe | ||
| if ocamlc -config | grep -q '^system: linux'; then dune exec -- ./lib_eio_linux/tests/bench_noop.exe; fi |
There was a problem hiding this comment.
Probably not the best solution for Windows, but there are other Windows unfriendly commands in other rules.
talex5
left a comment
There was a problem hiding this comment.
We may have to do this, but let's wait a little and see if the dune devs come up with a work-around.
| (name eio_linux) | ||
| (synopsis "Eio implementation for Linux using io-uring") | ||
| (description "An eio implementation for Linux using io-uring.") | ||
| (allow_empty) |
There was a problem hiding this comment.
What does this do? I can't see it in the dune manual! https://dune.readthedocs.io/en/latest/dune-files.html#dune-project
There was a problem hiding this comment.
Good question! It was suggested by the dune command:
➜ eio git:(main) ✗ dune build
Error: The package eio_linux does not have any user defined stanzas attached
to it. If this is intentional, add (allow_empty) to the package definition in
the dune-project file
-> required by _build/default/eio_linux.install
-> required by alias all
-> required by alias default
Also. I sense another dune issue coming.
9360ce9 to
49329ff
Compare
I suppose we could change the commit that adds the Would that make it more tempting to just merge this and let it be there until dune is fixed? |
➜ eio git:(main) ✗ dune build
File "lib_eio_linux/tests/dune", line 30, characters 21-30:
30 | (libraries alcotest eio_linux))
^^^^^^^^^
Error: Library "eio_linux" in _build/default/lib_eio_linux is hidden
(unsatisfied 'enabled_if').
-> required by _build/default/lib_eio_linux/tests/test.exe
-> required by alias lib_eio_linux/tests/all
-> required by alias default
The use of undocumented `(allow_empty)` was suggested by dune:
➜ eio git:(main) ✗ dune build
Error: The package eio_linux does not have any user defined stanzas attached
to it. If this is intentional, add (allow_empty) to the package definition in
the dune-project file
-> required by _build/default/eio_linux.install
-> required by alias all
-> required by alias default
Otherwise `opam pin .` complains:
[ERROR] Package conflict!
* Missing dependency:
- uring >= 0.4
unmet availability conditions: 'os = "linux"'
Interestingly, `opam unpin .` still complains as above, but at least `opam pin
.` does not.
Otherwise `make bench` fails:
dune exec -- ./lib_eio_linux/tests/bench_noop.exe
Error: Program "./lib_eio_linux/tests/bench_noop.exe" not found!
make: *** [bench] Error 1
49329ff to
ac094e0
Compare
|
I moved the |
|
After adding the condition However, interestingly, I just noticed that I wonder if that is caused by opam installing the 0.7 version that doesn't have the additional condition? |
|
The change to the Makefile is fine, but I'm not very keen on the rest of it. The installation instructions say to do Changing the metadata to allow eio_linux to be empty so you can install it on macos seems like a bad idea. The dune docs say for the tuareg thing:
But maybe it's the only way for now. |
Unfortunately that does not help with Those errors are then shown after every change. I find that unacceptable. Build should be clean so that any actual errors are then trivial to spot. Additionally, I think it is a fair point that there are instruction for installation. However, I find that disappointing. It means that one has to separately look for instructions on how to do that for every project. I'd very much prefer that opam (and dune) would just support a workflow using BTW, I also tried to add to
Yes, I think it is very problematic to allow unrestricted(?) code execution in a build system. OTOH, like I said, it should be easy to remove the hack as soon as someone comes up with a better solution. |
|
I want to elaborate a little more on why I think it is important that generic incantations / workflows like Those incantations are basically the versions of Having simple standard incantations for building, testing, installing, etc... is not important just for being able to install one library, like Eio. Having such standard incantations also makes is possible to deal with libraries generically in automated scripts and other tooling. For example, opam has this feature that you can pin a git URL that may also include a branch name. For example, you can say ➜ opam pin -y https://github.com/ocaml-multicore/eio.git\#polytypic/fix-dune-build-on-non-linux-systems
This will pin the following packages: eio, eio_linux, eio_luv, eio_main. Continue? [Y/n] y
eio is now pinned to git+https://github.com/ocaml-multicore/eio.git#polytypic/fix-dune-build-on-non-linux-systems (version 0.7)
eio_linux is now pinned to git+https://github.com/ocaml-multicore/eio.git#polytypic/fix-dune-build-on-non-linux-systems (version 0.7)
eio_luv is now pinned to git+https://github.com/ocaml-multicore/eio.git#polytypic/fix-dune-build-on-non-linux-systems (version 0.7)
eio_main is now pinned to git+https://github.com/ocaml-multicore/eio.git#polytypic/fix-dune-build-on-non-linux-systems (version 0.7)
The following actions will be performed:
- install eio 0.7*
- install eio_luv 0.7*
- install eio_linux 0.7*
- install eio_main 0.7*
===== 4 to install =====
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> retrieved eio.0.7 (no changes)
-> retrieved eio_luv.0.7 (no changes)
-> retrieved eio_linux.0.7 (no changes)
-> retrieved eio_main.0.7 (no changes)
-> installed eio.0.7
-> installed eio_linux.0.7
-> installed eio_luv.0.7
-> installed eio_main.0.7
Done.to pin this PR branch on macos and the same should basically work also on windows. This sort of pinning can be used with libraries that are not published on opam and it is also useful for e.g. depending on specific branches/versions of a library that are being developed. Unfortunately, that doesn't work on the ➜ opam pin -y https://github.com/ocaml-multicore/eio.git\#main
This will pin the following packages: eio, eio_linux, eio_luv, eio_main. Continue? [Y/n] y
eio is now pinned to git+https://github.com/ocaml-multicore/eio.git#main (version 0.7)
eio_linux is now pinned to git+https://github.com/ocaml-multicore/eio.git#main (version 0.7)
eio_luv is now pinned to git+https://github.com/ocaml-multicore/eio.git#main (version 0.7)
eio_main is now pinned to git+https://github.com/ocaml-multicore/eio.git#main (version 0.7)
[ERROR] Package conflict!
* Missing dependency:
- uring >= 0.4
unmet availability conditions: 'os = "linux"'
[NOTE] Pinning command successful, but your installed packages may be out of sync.So, with Eio, one now has to have some other means of pinning to a branch and any generic tooling one might have built for this purpose (I actually have) may need to be complicated to take this into account. Imagine doing that for every library. Well, that is actually how things work on Windows at the moment with opam. There are several forks of the opam package repository that include work arounds for building packages on Windows. It is my understanding that there is a goal for opam 2.2 to not need to have such forks. The current situation is a huge impediment to working on Windows with OCaml. I guess opam and dune are still immature and suffer from growing pains. I hope that in the future those will actually offer generic incantations for building, testing, and installing software. Having such incantations is important not just for people to be able do those things without always looking for library specific instructions, but also for any tooling that wants to automate working on libraries in general. |
|
But what does it mean to "install Eio"? Even if opam is fixed to install only compatible packages, it would still install not only the macOS backend and an (empty!) Linux one, but also e.g. download js_of_ocaml and install the browser and Node backends (which may be compatible, but aren't necessarily what you want). If a user wants to use Eio for something, they're going to have to say which package they want, at which point opam can pull in the appropriate dependencies. e.g. if a user wants to install "app-using-eio" with the dev version of Eio they'd do: Some things that would make this smoother:
Anyway, the above comments are about the changes to the opam metadata. The workaround to get |
|
The dune devs don't have time to fix this themselves (having rejected the existing PR), but if someone else wants to create a new non-broken |
|
I merged most of this in #475. I think this can be closed. |
See individual commits.