Cycles reported by dune are cryptic

[rgrinberg]

Dune reports some module cycles in a way that has no relationship with what the code the user wrote. I suspect this is caused by the fact that we over-approximate the dependency graph since we started including the transitive closure of the cmi's.

I'm not exactly sure how to fix this issue, but we should start by adding some test cases reproducing the issue.

cc @emillon and @diml

[rbjorklin]

Just want to add that I find dependency cycles in code I wrote to be somewhat cryptic. Would it be possible for the dependency cycle check to provide a line number in the offending file(s)?

[rgrinberg]

Would it be possible for the dependency cycle check to provide a line number in the offending file(s)?

It is not possible until ocamldep writes the locations of the dependencies it discovers.

[Alizter]

I guess this is something that will be improved once https://github.com/ocaml/dune/pull/6587 comes along.

[shonfeder]

Here is a good example: https://discuss.ocaml.org/t/help-with-cryptic-dependency-cycle-compile-errors-using-dune-build/14864

And the kind of error message discussed there (for searchability):

Error: dependency cycle between modules in _build/default/lib:
   Storage
-> Array_to_bytes
-> Array_to_bytes
-> required by _build/default/lib/zarr.a
-> required by alias lib/all
-> required by alias default

[edwintorok]

Here is another one, we modified 'storage_migrate.ml' to introduce a new call to 'storage_access.ml', which shows up nowhere in the trace (and a lot of other modules that are part of the cycle are missing):

Error: dependency cycle between modules in _build/default/ocaml/xapi:
   Xapi_xenops_queue
-> Xapi_host_helpers
-> Storage_access
-> Storage_access
-> required by alias ocaml/xapi/check

vs with codept:

[Fatal error]: Solver failure
   −Circular dependencies:
      Storage_access −(storage_access.ml:l327.2−24)⟶
      Storage_mux −(storage_mux.ml:l254.25−68)⟶
      Storage_smapiv1_wrapper −(storage_smapiv1_wrapper.ml:l423.18−53)⟶
      Storage_migrate −(storage_migrate.ml:l19.8−44)⟶ Storage_access

Code: https://github.com/xapi-project/xen-api/commit/07aeb7685b27cc04dd636c92b4d5a1d7fe4b49ea Running codept: cd ocaml/xapi && codept *.ml

I guess this is something that will be improved once #6587 comes along.

The linked PR got closed (even though it was approved), so I'm not sure what the plans are for codept integration, or whether dune itself could be improved to report better errors.