Dune: Dependency cycle with odoc

[aantron]

When installing and building with the {test} dependencies, the result is something like:

The actions to process have cyclic dependencies:
  - ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  odoc.1.2.0
  - ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  lwt.dev -> ∗  logs.0.6.2 -> ∗  bos.0.2.0 -> ∗  odoc.1.2.0
  - ∗  doc-ock-xml.1.2.0 -> ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  doc-ock-xml.1.2.0
  - ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  doc-ock.1.2.0 -> ∗  odoc.1.2.0
  - ∗  doc-ock-xml.1.2.0 -> ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  doc-ock.1.2.0 -> ∗  doc-ock-xml.1.2.0
  - ∗  jbuilder.1.0+beta17 -> ∗  camomile.0.8.7 -> ∗  lambda-term.1.12.0 -> ∗  utop.2.0.2 -> ∗  reason.3.0.0 -> ∗  jbuilder.1.0+beta17
  - ∗  jbuilder.1.0+beta17 -> ∗  camomile.0.8.7 -> ∗  lambda-term.1.12.0 -> ∗  utop.2.0.2 -> ∗  jbuilder.1.0+beta17
  - ∗  jbuilder.1.0+beta17 -> ∗  biniou.1.2.0 -> ∗  yojson.1.4.0 -> ∗  js_of_ocaml-compiler.3.0.2 -> ∗  jbuilder.1.0+beta17
  - ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  alcotest.0.8.1 -> ∗  tyxml.4.2.0 -> ∗  odoc.1.2.0
  - ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17 -> ∗  alcotest.0.8.1 -> ∗  tyxml.4.2.0 -> ∗  doc-ock-html.1.2.0 -> ∗  odoc.1.2.0

This is softly affecting CI of Lwt in AppVeyor, because we had to resort to --build-test for some reason that I can't clearly remember immediately.

[rgrinberg]

Hmm, but isn't this an issue with your CI? Why does it try to install dune's {test} deps?

[aantron]

IIRC when you install one package's {test} deps, it installs the {test} deps of all dependencies as well. This is a design flaw of opam, and is a major reason why I avoid using --build-test, because it greatly inflates the dependency graph by testing everything transitively. However, we used it in AppVeyor to work around some bug. It may no longer be necessary, but regardless the {test} dep in Jbuilder's .opam file will always lead to this if it is actually used.

[aantron]

To clarify, we install only Lwt packages' {test} deps, and that pulls in Jbuilder's {test} deps, which pulls in odoc, which has Jbuilder as a normal (actually, build) dep, causing this cycle.

[aantron]

Maybe @AltGr can comment on the status of this in development opam.

[AltGr]

IIRC when you install one package's {test} deps, it installs the {test} deps of all dependencies as well. This is a design flaw of opam

This is no longer true, opam will only bring the test dependencies of what was listed on the command-line. Also new in opam 2 are {post} dependencies that can be used to break cycles. It indicates that the package must be installed along with your package, but doesn't need to be installed before it.

[aantron]

Thanks, good to know about {post}.

Just to clarify @rginberg: that is for 2.0 (as I understand), there is still a dependency cycle in opam 1.2, however, because of the transitive {test} installation.

[Chris00]

A similar problem is present here:

The actions to process have cyclic dependencies:
  - ∗  yojson.1.4.0 -> ∗  js_of_ocaml-compiler.3.0.2 -> ∗  jbuilder.1.0+beta17 -> ∗  yojson.1.4.0
  - ∗  jbuilder.1.0+beta17 -> ∗  ocaml-migrate-parsetree.1.0.7 -> ∗  jbuilder.1.0+beta17
  - ∗  jbuilder.1.0+beta17 -> ∗  ocaml-compiler-libs.v0.10.0 -> ∗  ppx_ast.v0.10.0 -> ∗  ppx_core.v0.10.0 -> ∗  ppx_driver.v0.10.2 -> ∗  jbuilder.1.0+beta17
  - ∗  js_of_ocaml-compiler.3.0.2 -> ∗  jbuilder.1.0+beta17 -> ∗  js_of_ocaml-compiler.3.0.2
  - ∗  yojson.1.4.0 -> ∗  js_of_ocaml-compiler.3.0.2 -> ∗  jbuilder.1.0+beta17 -> ∗  easy-format.1.3.1 -> ∗  yojson.1.4.0
  - ∗  biniou.1.2.0 -> ∗  yojson.1.4.0 -> ∗  js_of_ocaml-compiler.3.0.2 -> ∗  jbuilder.1.0+beta17 -> ∗  easy-format.1.3.1 -> ∗  biniou.1.2.0
  - ∗  jbuilder.1.0+beta17 -> ∗  camomile.0.8.7 -> ∗  lambda-term.1.12.0 -> ∗  utop.2.0.2 -> ∗  reason.3.0.0 -> ∗  jbuilder.1.0+beta17
  - ∗  jbuilder.1.0+beta17 -> ∗  camomile.0.8.7 -> ∗  lambda-term.1.12.0 -> ∗  utop.2.0.2 -> ∗  jbuilder.1.0+beta17
  - ∗  biniou.1.2.0 -> ∗  yojson.1.4.0 -> ∗  js_of_ocaml-compiler.3.0.2 -> ∗  jbuilder.1.0+beta17 -> ∗  biniou.1.2.0
  - ∗  jbuilder.1.0+beta17 -> ∗  alcotest.0.8.1 -> ∗  tyxml.4.2.0 -> ∗  doc-ock-html.1.2.0 -> ∗  odoc.1.2.0 -> ∗  jbuilder.1.0+beta17

[AltGr]

Any (test-)dependencies that lead to cycles should really be removed from the repo: as long as build and test actions are encoded in the same vertex of our action graph, there is no way they can work (even with {post}).

Once moved to their own node, and with {post}, we could improve so that opam is able to schedule e.g. compile jbuilder with tests → install jbuilder → install alcotest → run jbuilder tests. Note that since build-test was deprecated in favor of build: instructions with a filter on {with-test}, even in this case dependencies with flags {post & with-test} would be available to _run_ the tests (according to run-test: instructions), but not at test _build_-time.

Changing the action graph in such a way would be a tricky change, and won't be in 2.0.0. Meanwhile, I should probably add a check for cycles to @Camelus (and maybe separate that into a whole-repo linting module ?).

[jeremiedimino]

In the short term, maybe we should just move the tests to a jbuilder-tests package

[AltGr]

I'm afraid that's the only proper solution at the moment, yes

[aantron]

In the immediate term, I think it would be very helpful to remove the {test} dependencies from the .opam file now published in the opam repo.

[yomimono]

There is a PR doing as @aantron suggests at https://github.com/ocaml/opam-repository/pull/11376 .

[AltGr]

See https://github.com/ocaml/opam/pull/3215 that might help detect similar issues

[rgrinberg]

This is fixed in opam-repository and this repo.