dune icon indicating copy to clipboard operation
dune copied to clipboard

utop: warning when modules collide with utop deps (will become error in 5.1)

Open jchavarri opened this issue 2 years ago • 7 comments

It seems dune loads the dependencies of utop when running dune utop, which can lead to conflics for users trying to run utop sessions in projects with modules that can collision with utop deps.

In 4.14, this situation triggered a warning, like this PR shows. But in 5.1 it becomes a hard error. From OCaml compiler changelog:

  • #11635, #5461, #10564: turn warning 31 (Module_linked_twice) into a hard error for ocamlc — this was already an error with ocamlopt. (Hugo Heuzard, review by Valentin Gatien-Baron and Gabriel Scherer)

jchavarri avatar Sep 22 '23 10:09 jchavarri

@rgrinberg I asked you about this issue in person but I couldn't remember the solution you had nor could I find anything on it. Could you mention it here?

FTR @jchavarri IIRC there is a mode for utop where it doesn't load the libraries that were used to compile it, that you have to use. I think its pretty peculiar that this is even a thing. Can't remember how to switch the mode however.

Alizter avatar Sep 22 '23 12:09 Alizter

I was talking about expunging. I don't think it would help here.

I'm not sure what's the solution nor am I even certain this is a dune problem. I don't know of a way to produce a utop toplevel without linking its dependencies.

@nojb any ideas perhaps?

rgrinberg avatar Sep 23 '23 10:09 rgrinberg

dune utop works fine if the code being loaded into the toplevel depends on one or more of the libraries utop itself links against (eg lwt, react). On the other hand, it cannot link modules that have a name identical to one the modules that are already linked into it (eg React, Lwt, etc). This is a basic limitation of the OCaml toolchain.

@jchavarri is it right that in your example in https://github.com/ocaml-community/utop/issues/463, the react library is being vendored in server-reason-react? That would explain the name clash.

In the toplevel, there is a way to remove module names from the global scope (expunging). utop (like the standard toplevel) already expunges compiler-libs, and one could imagine expunging more names (eg React in this case), which would technically allow you to link modules with those names. However, note that expunging means that utop will not recognize that the React module being linked is the same as the (expunged) React module that it linked with originally, and that may lead to some puzzling behaviours.

In any case, note that dune utop does not expunge the custom utop toplevel at all (we don't want to link against compiler-libs).

In summary:

  • If you are vendoring react, then it will clash with the react that is linked into utop (this has nothing to do with Dune). The simplest way to fix this problem is not to vendor react, but link with it "normally".
  • You may experiment with expunging more things out of utop, but results may be unpredictable.
  • Dune does not do any expunging when building the custom utop executable, so it is unlikely that it will be possible to address this issue at the level of Dune. But, if you solve it at the level of utop you may still leverage Dune by using #use_output "dune ocaml top".

nojb avatar Sep 25 '23 08:09 nojb

Thanks for the thorough response @nojb !

@jchavarri is it right that in your example in https://github.com/ocaml-community/utop/issues/463, the react library is being vendored in server-reason-react? That would explain the name clash.

This issue was opened by @davesnx who is working on server-reason-react. This library does not vendor the OCaml react library. Rather, this package provides reimplementations in OCaml of the Melange bindings defined in reason-react. The goal is to compile the same code to either the client / JS (with Melange and reason-react) or the server (OCaml and server-reason-react), so the native implementation has to follow what the Melange one is doing, which in this case means naming the library react.

So the server-reason-react code is fully unrelated to the react library used by utop. It's just a case of name collision between unrelated libraries.

In the toplevel, there is a way to remove module names from the global scope (expunging). utop (like the standard toplevel) already expunges compiler-libs, and one could imagine expunging more names (eg React in this case), which would technically allow you to link modules with those names. However, note that expunging means that utop will not recognize that the React module being linked is the same as the (expunged) React module that it linked with originally, and that may lead to some puzzling behaviours.

This sounds exactly like what we need. And I understand that because the server-reason-react is its own thing, the puzzling behavior mentioned would not apply in our case?

I found some related code in the utop repo, I understand the suggested approach would be to add React to this list?

  • Dune does not do any expunging when building the custom utop executable,

Why doesn't Dune do any expunging? I understood expunging prevents some issues with compiler-libs and other libraries, so it looked useful to have it in all cases.

jchavarri avatar Sep 25 '23 13:09 jchavarri

I found some related code in the utop repo, I understand the suggested approach would be to add React to this list?

Yes. But to be clear, I am not recommending expunging as a solution (it is a hack and may have unexpected consequences). There may be other ways to work around the issue. For example, you could have a compilation unit called Server_react that does module React = struct ... end inside and use -open Server_react when compiling on the server. Or simply doing module React = Server_react in the source code somehow.

Why doesn't Dune do any expunging? I understood expunging prevents some issues with compiler-libs and other libraries, so it looked useful to have it in all cases.

I guess no one complained about it before. One downside of expunging is that it is a hack, and can sometimes produce unexpected results. For example utop has special support for lwt (it resolves top-level values of type 'a Lwt.t). I suspect that this will stop working if Lwt is expunged (but haven't tried it).

nojb avatar Sep 25 '23 14:09 nojb

For example, you could have a compilation unit called Server_react that does module React = struct ... end inside and use -open Server_react when compiling on the server. Or simply doing module React = Server_react in the source code somehow.

@davesnx is something like this possible?

jchavarri avatar Sep 27 '23 08:09 jchavarri

It's possible, but I wonder if changing the interface of the package is a good solution to have it working just with utop.

I can suggest to vendor React (and any other dependency) and rename it to a hard-to-collide name, maybe it's a proper solution but I raised the issue in order to have more ideas on how to fix it.

davesnx avatar Sep 27 '23 09:09 davesnx