regression (3.7): chamelon-unix.0.0.7 - dependency cycle between modules

[emillon]

Hi,

chamelon-unix.0.0.7 does not build with 3.7.0~alpha1, though it does with 3.6.2.

The error is:

Error: dependency cycle between modules in _build/default/lib:
   Entry
-> Entry
-> required by _build/default/lib/chamelon.a
-> required by _build/install/default/lib/chamelon/chamelon.a
-> required by _build/default/chamelon.install
-> required by alias install

I bisected that to fb085c0 (#6594).
The dune file in lib does not seem to have any gotchas:

(library
  (public_name chamelon)
  (libraries checkseum cstruct ptime)
  (preprocess (pps ppx_cstruct))
)

This might be related to #7015 since it comes from the same feature but it's hard to tell.
@rgrinberg if you could have a look at this one too (or if you fix #7015 first I'm happy to check if that fixes this too).

Thanks!

[rgrinberg]

I had a look and there's indeed a cycle:

$ ocamldep -modules dir.mli
dir.mli: Cstruct Entry Tag

$ ocamldep -modules entry.ml
entry.ml: Cstruct Dir File Int Int32 List Tag

Now I need to understand why we weren't detecting it before.

[rgrinberg]

I guess the above isn't really a cycle. If we expand the dependency chain we get:

dir.mli -> entry.mli
entry.ml -> dir.mli -> entry.mli

[rgrinberg]

I thought about this again and I'm no longer sure we need to fix a bug here. Our dependency algorithm was specified to be:

transitive_deps_of(M, Kind) =
  let deps = deps_of(M, Kind) in
  deps U transitive_deps_of(X, Intf) for X in deps

And chamelon violates this specification. We have:

dep(impl, entry, [dir]).
dep(intf, entry, []).

dep(impl, dir, []).
dep(intf, dir, [entry]).

So when we compute transitive_deps_of(impl, entry), we clearly get a cycle.

The only reason why this works is that dir.mli is only using a type alias defined in entry.mli. But I don't see how can we detect this in general.

[emillon]

OK. I think that it makes sense. Since the latest version of chamelon is affected, I sent a PR to fix this: yomimono/chamelon#18.
Semgrep is also affected (in the commons library), I'll try to make a fix too.

[emillon]

I did the same for commons which was similarly affected: semgrep/semgrep#7173. And I'll add upper bounds in opam-repository.

[emillon]

If there's nothing to fix, let's close this one then. Thanks