dune-site: use string list instead of Location.t

[rizo]

Desired Behavior

Currently the site module generated with generate_sites_module produces code that represents paths as Dune_site.Private_.Helpers.Location.t list, which is simply an alias for string list.

This means that if such a module is added to a standalone library and consumed as a dependency library the build fails with:

Error: This expression has type Dune_site.Private_.Helpers.Location.t
       but an expression was expected of type string
       Dune_site.Private_.Helpers.Location.t is abstract because no corresponding cmi file was found in path.

Of course, this can be fixed by depending on dune-site in both the sites "producer" library and the sites "consumer" library. I think only the producer should directly depend on dune-site instead.

A simple workaround for this is to add an interface file that constrains the Dune_site.Private_.Helpers.Location.t list type to string list, but ideally this shouldn't be necessary.

I understand that there may be a need to future-proof locations, but would argue that providing a dependency-free interface is an important requirement for users of sites too.

[rgrinberg]

cc @bobot

[bobot]

So the producer depends on dune-site, but the consumer is not. Are you using no-transitive-deps? If it is the case whenno-transitive-deps will be fixed (-H will arrive in 5.2) it will be fixed.

The alias Location is mainly just more informative that the type string, so if it is too bothersome it would not be a problem to add a signature in the generated implementation. We could also remove the type alias in Dune_site.Private_.Helpers.Location.t .

[rizo]

Yes, I should have mentioned that I have (implicit_transitive_deps false). Just checked enabling transitive deps and the error went away. I guess, as expected.

In a way, this relates to #9661.

I understand the motivation for the alias, but, in my opinion, this one alone doesn't justify enforcing the dependency on dune-site on the consumer. Maybe the alias could be included in the generated module?

Interface before:

module Sites : sig
  val public : string list
end

Interface after:

type location := string

module Sites : sig
  val public : location list
end

Note, the non-exporting type definition as a bonus.

Otherwise, good to hear -H work will help with this.

[bobot]

I don't understand your proposition, no interface files '.mli' are generated. I'm not against your objective, and I see two possibilities for that as summarized before:

• we can do easily is adding a signature inside the implementation "*.ml": module Sites : sig val public string list end = struct ... end.
• But really the simplest is just to remove the alias in helpers.
• I haven't tried but locally you can also add your own interface file "*.mli" for the generated file in order to only show string list.

[rizo]

My bad, for (1) I wanted to suggest that the interface is also generated, or alternatively type location = string is added to the implementation.

But I agree that (2) is simpler and would be my preferred suggestion.

Re (3), as mentioned in the description, I did add the interface manually, but I consider this a workaround.