Allow for libraries depending on base to enable dune's (implicit_transitive_deps false)

[mbarbin]

Recently I started experimenting with dune's (implicit_transitive_deps false) setting.

https://dune.readthedocs.io/en/stable/dune-files.html#implicit-transitive-deps

We recommend users experiment with this mode and report any problems.

Build error

Currently if you enable this option in a library that depends on base and that uses ppx_hash this won't build. The errors look like this:

Error: Signature mismatch:
       ...
       Values do not match:
         val hash : t -> Hash.hash_value
       is not included in
         val hash : t -> int/2
       The type t -> Hash.hash_value is not compatible with the type
         t -> int/2
       Type Hash.hash_value is not compatible with type int/2

In the rest I'm documenting my understanding of the issue in the hope that can help.

Investigating

I believe this is due to the fact that the equality between int and Hash.hash_value is known by base.base_internalhash_types, which cmi is by default included in the compilation path when implicit transitive deps is true, but is not exported solely by base.

Local workaround

As an immediate and local workaround, I report that it is enough to add base.base_internalhash_types alongside base in the lib's dependency to fix the build.

Possible improvement (require bumping lang dune >= 2.0)

I think that the workaround stated above is not desirable in general, since an internal name of base leaks into a library's dune file in the user land. It's probably better to set this internal library to be re_exported as documented here.

I tried to offer this change as a pull request, but as it turns out this will require more careful thoughts:

File "src/dune", line 23, characters 2-37:
23 |   (re_export base_internalhash_types)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: 're_export' is only available since version 2.0 of the dune language.
Please update your dune-project file to have (lang dune 2.0).
Had 1 error, waiting for filesystem changes...

I'm opening this issue as documentation and for your consideration. Thank you!

[dkalinichenko-js]

Hi! We're unlikely to support (implicit_transitive_deps false) for now, as we enable implicit transitive dependencies internally and are unlikely to disable them. Thank you for reporting the issue in any case.

[dkalinichenko-js]

Actually, we're working on a similar rule update internally, but it'll depend on the recently implemented -H compiler flag (see https://github.com/ocaml/ocaml/pull/12246). I'm not sure the resulting semantics will match (implicit_transitive_deps false), but I'll keep this ticket open for now.