Armael / marracheck

Gotta check them all!
10 stars 3 forks source link

Dependency cycle due to dune -> ocamlfind -> graphics -> dune #6

Open art-w opened 2 years ago

art-w commented 2 years ago

I'm not sure what marracheck should do here (see this stale issue https://github.com/ocaml/opam-repository/issues/16909), but others might encounter the same problem at:

https://github.com/Armael/marracheck/blob/eb333b8754bf4e9a8aef93dcae29b92fdadc614c/lib/lib.ml#L205-L206

When using the standard opam-repository, dune depends on ocamlfind, which has an optional dependency on graphics, which itself depends on dune.

A quickndirty fix is to remove the optional dependency from ocamlfind in the opam repo:

$ cd opam-repository/packages/ocamlfind
$ find . -name 'opam' -exec sed -i '/graphics/d' '{}' ';'
$ git commit -m 'Remove ocamlfind depopt on graphics'
Armael commented 2 years ago

Indeed, I remember having issues with cycles in the past; either because of optional dependencies, or because the custom solver would find actually possible cycles but which do not seem to happen in practice (or are a temporary "bug" of a specific commit of the opam-repo). I remember trying various approaches, but I don't remember the details, so I would have to consult notes.

For the record, could you write down the git hash of the opam-repository where you've observed this? And the hash of the commit of marracheck?

kit-ty-kate commented 2 years ago

this is off-topic but out of curiousity, what are you using marracheck for?

art-w commented 2 years ago

I was a bit quick in my reporting: dune has a conditional dependency on ocamlfind-secondary, which then depends on ocamlfind etc.

depends     ("ocaml" {>= "4.08"} | ("ocaml" {< "4.08~~"} & "ocamlfind-secondary"))

So the issue is that the conditional dependency is ignored when opam checks for cycles? (I don't personally care about the compiler variants... in my case, removing them is a better fix than patching ocamlfind depopt) For posterity, this happened with https://github.com/ocaml/opam-repository/commit/0c25d08c7a03345f94e794b6cda1392192da63d6 and marracheck https://github.com/Armael/marracheck/commit/0464876fae27b27930184e146e9fb9af390f4c5e

@kit-ty-kate > I'm searching for a small set of packages that the ocaml compiler finds "interesting" according to -dtimings and other metrics in order to benchmark trunk and PRs for regressions. A recent example is your irmin report https://github.com/ocaml/ocaml/issues/10779 for functor heavy code! We currently use a hand-picked selection of packages, but it's probably not a good coverage of all the ocaml features. Unrelatedly, I also want to run some experiments with the typed trees produced by opam packages :)

kit-ty-kate commented 2 years ago

See https://github.com/ocaml/opam/issues/4541 for the bug in the cycle detection system in opam.

Very interesting! ^_^

Armael commented 2 years ago

I submitted a fix upstream that should solve this specific issue: https://github.com/ocaml/opam/pull/5066 . I've also included the corresponding patch in the marracheck branch of opam that's used by the git submodule, so this issue should be fixed right now in marracheck.

Armael commented 2 years ago

Reopening since the approach of ocaml/opam#5066 has a fundamental flaw. I'll not remove the patch to the marracheck opam branch yet, but I plan to work on a better fix for the issue (fixing the computation of cycles in a repo to detect the cycle).